LayoutTests/media/video-durationchange-on-ended.html - Issue 1074633002: Adjust video-durationchange unit test for new behavior

Side by Side Diff: LayoutTests/media/video-durationchange-on-ended.html

Issue 1074633002: Adjust video-durationchange unit test for new behavior (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Created 5 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <html>	2 <html>

3 <head>	3 <head>

4 <script src=video-test.js></script>	4 <script src=video-test.js></script>

5 </head>	5 </head>

6 <body>	6 <body>

7 <p>Verify there is a 'durationchange' event just before 'ended' event if the initially reported duration exceeds the actual data.</p>	7 <p>Verify there is a 'durationchange' event just before 'ended' event if the initially reported duration exceeds the actual data.</p>

8 <script>	8 <script>

9 var initialReportedDuration = -1;	9 var initialReportedDuration = -1;

10	10

11 video = document.createElement('video');	11 video = document.createElement('video');

12 document.body.appendChild(video);	12 document.body.appendChild(video);

13 video.src = "content/truncated.webm";	13 video.src = "content/truncated.webm";

14 video.play();	14 video.play();

15 waitForEvent('durationchange', function() {	15 waitForEvent('durationchange', function() {

16 // Note the initial reported duration	16 // Note the initial reported duration

17 if (initialReportedDuration == -1)	17 if (initialReportedDuration == -1)

18 initialReportedDuration = video.duration;	18 initialReportedDuration = video.duration;

19 });	19 });

20	20

21 waitForEventAndEnd('ended', function() {	21 waitForEventAndEnd('ended', function() {

	22 consoleWrite("initialReportedDuration=" + initialReportedDuratio n + " video.duration=" + video.duration);

22 // Verify that the actual duration is less than	23 // Verify that the actual duration is less than

23 // the initial reported duration	24 // the initial reported duration

24 testExpected("video.duration < initialReportedDuration", true);	25 testExpected("video.duration <= initialReportedDuration", true);
	wolenetz 2015/04/09 17:29:06 Though I understand how this causes the test to pa Though I understand how this causes the test to pass, it contradicts the goal of this layout test, which is to verify that durationchange fires on truncated duration files, along with confirming the situation of an actually-truncated duration. IIUC, the Chromium CL (https://codereview.chromium.org/1069253004/) fixes the ended firing behavior when the truncation is minimal, but regresses reporting the actual duration in that case. Ideally, the right fix would be to retain both correct ended firing behavior (for both MSE and src=) as well as reporting the actual truncated duration (even if the truncation is tiny). Any ideas how to fix this? Do I misunderstand? If I don't, please at least file a crbug to fix the minimal truncation not being reported correctly, and adjust this layout test's "content/truncated.webm" to have > 250ms duration truncation and retain the strict "<" check here. servolk 2015/04/29 21:31:01 Sorry for the delay on this. Yes, your understandi Show quoted text On 2015/04/09 17:29:06, wolenetz wrote: > Though I understand how this causes the test to pass, it contradicts the goal of > this layout test, which is to verify that durationchange fires on truncated > duration files, along with confirming the situation of an actually-truncated > duration. > > IIUC, the Chromium CL (https://codereview.chromium.org/1069253004/) fixes the > ended firing behavior when the truncation is minimal, but regresses reporting > the actual duration in that case. > > Ideally, the right fix would be to retain both correct ended firing behavior > (for both MSE and src=) as well as reporting the actual truncated duration (even > if the truncation is tiny). Any ideas how to fix this? Do I misunderstand? If I > don't, please at least file a crbug to fix the minimal truncation not being > reported correctly, and adjust this layout test's "content/truncated.webm" to > have > 250ms duration truncation and retain the strict "<" check here. Sorry for the delay on this. Yes, your understanding is correct. I think in order to fix this properly we need to: a. Have an explicit 'pipeline ended' signal from media pipeline to blink, instead of having blink rely on currentTime/duration comparison to detect eos (crbug.com/438581) b. When blink receives that 'pipeline ended' signal, it should re-check the stream duration and fire 'durationchanged' event if the actual duration turns out to be less than expected.
25 });	26 });

26 </script>	27 </script>

27 </body>	28 </body>

28 </html>	29 </html>

OLD	NEW

« no previous file with comments | « no previous file | LayoutTests/media/video-durationchange-on-ended-expected.txt » ('j') | no next file with comments »