469595 - test_wav_seek* intermittent test failure

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

test_wav_seek6.html fails intermittently on WinXP. Appears to be a timing issue, often when the "First seek" test runs, currentTime has passed the range of the test.

We're also seeking int nsHTMLMediaElement::Load(), which is firing seeking and seeked events, which can confuse things.

You may want to apply patch from bug 465921 when trying to reproduce this, as it makes mochitest http server more reliable.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Looks like test_wav_seek3.html is doing this as well.

Comment 2

[Matthew Gregan [:kinetik]]

The problem is, these tests are playing the media and trying to query currentTime, but it can be outside the expected range in some cases due to timing on a particular machine.  The tests are cloned from the Ogg ones, but for some reason they don't seem to be as sensitive to failing.

It's not safe to call play() and then check currentTime in a later event and expect it to be a particular value.

Filed bug 469598 (with a patch) for the seek in nsHTMLMediaElement::MetadataLoaded (which is what cpearce meant, not Load).

Comment 3

[Mats Palmgren (inactive)]

test_wav_8bit.html failed on MacOSX:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1229338384.1229343048.20754.gz

Comment 4

[cajbir (:cajbir)]

Bug 469628 is possibly the cause of the issue from comment 3.

Comment 5

[Matthew Gregan [:kinetik]]

Attachment: patch v0 [Checkin: Comment 10 & 13]

Improve test robustness.  Avoid checking currentTime when we're playing, since (depending on system load, etc.) the currentTime may already have moved outside the threshold.  For seek1, I changed it to pause in the seeking event and resume playing after checking currentTime in the seeked event.  Increased the truncated file size and tweaked the currentTime/duration thresholds in the wav_trunc tests.

Comment 6

[Matthew Gregan [:kinetik]]

Note that the patch includes a binary file and a rename, so needs to be applied with import/qimport.

Comment 7

[Matthew Gregan [:kinetik]]

Comment on attachment 353153 [details] [diff] [review]
patch v0
[Checkin: Comment 10 & 13]

These pass on cpearce's XP machine, which seems to be good at finding test
problems.  Also pass on my OS X machine.

The changed tests are intended to still check the same assertions, so
requesting review to make sure I didn't screw up.

Comment 8

[Matthew Gregan [:kinetik]]

Just test changes, so doesn't need blocking/approval.  Be good to get this in to fix some of the spurious oranges on Tinderbox.

Comment 9

[Matthew Gregan [:kinetik]]

Attachment: new file

Mercurial can't import the patch properly: r11025_s16_c1_trunc.wav had the contents replaced and was renamed to r11025_u8_c1_trunc.wav, but Hg claims the file exists when trying to import.

Comment 10

[cajbir (:cajbir)]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/0a945ba5d2eb

Comment 11

[Mats Palmgren (inactive)]

Looks like we need this on 1.9.1 too?
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.1/1229416834.1229420300.15840.gz

Comment 12

[Mats Palmgren (inactive)]

Yeah, right, I see the Whiteboard now :-)

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 353153 [details] [diff] [review]
patch v0
[Checkin: Comment 10 & 13]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/66d6428ddf43

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to comment #13)
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/66d6428ddf43

Ftr, that was after fixing (context for)
{
patching file content/media/video/test/Makefile.in
Hunk #1 FAILED at 92
1 out of 1 hunks FAILED
}

Comment 15

[Daniel Holbert [:dholbert]]

See also bug 475369 for a recent failure in test_seek1.html

Comment 16

[Aakash Desai [:aakashd]]

Seeing as there hasn't been any discussions about this bug for 2 1/2 months and the fix has been pushed out for a bit longer than that, I'm assuming there aren't any intermittent failures with test_wav_seek6.html. I'm moving this to verified as a result. If anyone has any qualms, feel free to bring them up.