Bug 36165 – [GStreamer] media/video-played-collapse.html

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Gustavo Noronha (kov)]

Like I said in https://bugs.webkit.org/show_bug.cgi?id=34372, I think fixing the timeout priorities to avoid starvation is exposing a problem in this test. It was failing consistently for me before the patches, but passing on the bots, and now the situation has somewhat reversed, so I think there's some racy behaviour we have to investigate making this test unreliable. I'll skip it.

[Maciej Stachowiak]

Created attachment 50879 [details]
Patch

[Maciej Stachowiak]

Comment on attachment 50879 [details]
Patch

Oops, wrong bug (curse you webkit-patch)

[Philippe Normand]

Created attachment 51737 [details]
test fix proposal

[Philippe Normand]

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.

[Philippe Normand]

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?

[Philippe Normand]

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?

[Gustavo Noronha (kov)]

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

[Philippe Normand]

(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!

[Philippe Normand]

(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 ;)

[Philippe Normand]

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?

[Gustavo Noronha (kov)]

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.

[Martin Robinson]

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.

[Philippe Normand]

Eric,

Could you test this patch too?
Do you know why the "looping" part of the test was deactivated?

[Eric Carlson]

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?

[Philippe Normand]

(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.

[Philippe Normand]

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.

[Philippe Normand]

Created attachment 130333 [details]
Patch

[Philippe Normand]

Committed r109921: <http://trac.webkit.org/changeset/109921>