Summary: | [GStreamer] media/video-played-collapse.html | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> | ||||||||||||||
Component: | Media | Assignee: | Philippe Normand <pnormand> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | chinmaya, eric.carlson, mrobinson, pnormand | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | Linux | ||||||||||||||||
Attachments: |
|
Description
Gustavo Noronha (kov)
2010-03-16 05:06:20 PDT
Created attachment 50879 [details]
Patch
Comment on attachment 50879 [details]
Patch
Oops, wrong bug (curse you webkit-patch)
Created attachment 51737 [details]
test fix proposal
Comment on attachment 51737 [details]
test fix proposal
Executed alone the test passes fine locally but not if i run the whole media test suite... Will investigate further. Removing review flag for now.
If media/video-pause-immediately.html is executed before played-collapste.html, the later fails. In the first test a played range is created and the second test fails because there are 3 ranges created instead of 2. If I make the first test not start any playback the video-played-collapse test passes. So... it looks like the second test reuses the media element of the first one, how is that possible? :( What do you think Gustavo? Comment on attachment 51737 [details] test fix proposal Marking again for review as I can't reproduce the behavior observed in comment 5 anymore. Can you please test this Gustavo? The test fails for me with your patch applied, when running all the media tests =(: -Test 13 OK +EXPECTED (video.played.length == '2'), OBSERVED '3' FAIL Test 14 OK -Test 15 OK -Test 16 OK -Test 17 OK +EXPECTED (video.played.end(0).toFixed(2) == '1.30'), OBSERVED '0.28' FAIL +EXPECTED (video.played.start(1).toFixed(2) == '5.98'), OBSERVED '0.40' FAIL +EXPECTED (video.played.end(1).toFixed(2) == '6.03'), OBSERVED '1.30' FAIL (In reply to comment #7) > The test fails for me with your patch applied, when running all the media tests =(: Can you please uncomment the disableFullTestDetailsPrinting() line in the test, run it with DRT and give the results? Thanks! (In reply to comment #7) > The test fails for me with your patch applied, when running all the media tests =(: > > -Test 13 OK > +EXPECTED (video.played.length == '2'), OBSERVED '3' FAIL > Test 14 OK > -Test 15 OK > -Test 16 OK > -Test 17 OK > +EXPECTED (video.played.end(0).toFixed(2) == '1.30'), OBSERVED '0.28' FAIL > +EXPECTED (video.played.start(1).toFixed(2) == '5.98'), OBSERVED '0.40' FAIL > +EXPECTED (video.played.end(1).toFixed(2) == '6.03'), OBSERVED '1.30' FAIL Ok I can reproduce that failure on my laptop but not on my other build machine, will have a look ;) Created attachment 64228 [details]
test fix proposal
Found out why it failed :) During the last part of the test (looping),
we check that the first 2 timeranges are merged but in some cases the
player had no time to reach past the merge position. Setting the
second range start position earlier in the timeline fixes the issue in
my case. Can you test Gustavo?
It seems to fail once in a while with this diff (I ran it 8 times and it failed 3 times here): --- /tmp/layout-test-results/media/video-played-collapse-expected.txt 2010-08-16 10:51:36.000000000 -0300 +++ /tmp/layout-test-results/media/video-played-collapse-actual.txt 2010-08-16 10:51:36.000000000 -0300 @@ -36,10 +36,10 @@ RUN(video.play()) RUN(video.pause()) EVENT(pause) -Test 13 OK +EXPECTED (video.played.length == '2'), OBSERVED '3' FAIL Test 14 OK -Test 15 OK -Test 16 OK -Test 17 OK +EXPECTED (video.played.end(0).toFixed(2) == '1.60'), OBSERVED '0.19' FAIL +EXPECTED (video.played.start(1).toFixed(2) == '5.98'), OBSERVED '0.25' FAIL +EXPECTED (video.played.end(1).toFixed(2) == '6.03'), OBSERVED '1.60' FAIL END OF TEST I also had a GStreamer crash on media/video-layer-crash.html in one of my runs heh. The diff seems to always be mostly the same, though it had "1.56" in the last FAIL the last time I ran it. Phillippe is it possible for you to ping Eric Carlson for this bug? He's not the original author, but he'd probably be the best person to review this patch. Eric, Could you test this patch too? Do you know why the "looping" part of the test was deactivated? Created attachment 67701 [details] Updated patch (In reply to comment #13) > > Could you test this patch too? > Do you know why the "looping" part of the test was deactivated? > It looks like the problem is that the movie was supposed to play for duration-0.05 seconds, loop, and play at least 0.4 seconds before pausing, but the test was only configured to run for 100ms. I have changed the test to make the first range start closer to 0 and to calculate the time required to loop and play. Can you if this work for you? (In reply to comment #14) > Created an attachment (id=67701) [details] > Updated patch > > (In reply to comment #13) > > > > Could you test this patch too? > > Do you know why the "looping" part of the test was deactivated? > > > It looks like the problem is that the movie was supposed to play for duration-0.05 seconds, loop, and play at least 0.4 seconds before pausing, but the test was only configured to run for 100ms. > > I have changed the test to make the first range start closer to 0 and to calculate the time required to loop and play. Can you if this work for you? It fails here: --- /tmp/layout-test-results/media/video-played-collapse-expected.txt 2010-11-17 15:45:44.000000000 +0100 +++ /tmp/layout-test-results/media/video-played-collapse-actual.txt 2010-11-17 15:45:44.000000000 +0100 @@ -34,12 +34,6 @@ Test looping RUN(video.loop = true) RUN(video.play()) -RUN(video.pause()) -EVENT(pause) -Test 13 OK -Test 14 OK -Test 15 OK -Test 16 OK -Test 17 OK +ERROR: test stalled, waited 2.3510000705718994 seconds for movie to play 2.2808964252471924 seconds FAIL END OF TEST Haven't had time yet to debug this. Created attachment 76214 [details]
updated test
I had some success with this one on GTK. The expected result didn't
change from Eric's version.
Created attachment 130333 [details]
Patch
Committed r109921: <http://trac.webkit.org/changeset/109921> |