Bug 113438 – Add LayoutTests that verify MediaSource.duration behavior.

• Component: New Bugs
• Status: RESOLVED FIXED

[Aaron Colwell]

Add LayoutTests that verify MediaSource.duration behavior.

[Aaron Colwell]

Created attachment 195388 [details]
Patch

[Aaron Colwell]

Comment on attachment 195388 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195388&action=review

> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions-expected.txt:29
> +EXPECTED (mediaSource.duration == '1.7976931348623157e+308'), OBSERVED '3.4028234663852886e+38' FAIL

This and the two other faiures below are expected right now because HTMLMediaElement.duration is a float instead of a double like the spec state. Eventhough MediaSource.duration is declared as a double, the return value is truncated to the float range so that the mediaSource.duration == video.duration condition required by the Media Source spec is always true. When HTMLMediaElement.duration is changed to a double, then these expectations will be updated.

[Eric Carlson]

Comment on attachment 195388 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195388&action=review

> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions-expected.txt:6
> +onDurationChange : video.duration is 6.041999816894531

I don't think it makes sense to log time values with full precision, or every port may need to have unique results for the same test.  Comparing the value is fine, but use reportExpected(() so you just log success or failure. See video-loop.html for an example.

> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions.html:24
> +          consoleWrite('EXPECTED (mediaSource.duration == video.duration) OK');

Can you use reportExpected( instead of hard coding this?

> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions.html:27
> +        consoleWrite('EXPECTED (mediaSource.duration == video.duration), OBSERVED ' + mediaSource.duration + ' vs ' + video.duration + ' FAIL');

Ditto.

[Aaron Colwell]

Comment on attachment 195388 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195388&action=review

>> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions-expected.txt:6
>> +onDurationChange : video.duration is 6.041999816894531
> 
> I don't think it makes sense to log time values with full precision, or every port may need to have unique results for the same test.  Comparing the value is fine, but use reportExpected(() so you just log success or failure. See video-loop.html for an example.

Done

>> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions.html:24
>> +          consoleWrite('EXPECTED (mediaSource.duration == video.duration) OK');
> 
> Can you use reportExpected( instead of hard coding this?

Yes. Done.

>> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions.html:27
>> +        consoleWrite('EXPECTED (mediaSource.duration == video.duration), OBSERVED ' + mediaSource.duration + ' vs ' + video.duration + ' FAIL');
> 
> Ditto.

Done.

[Aaron Colwell]

Created attachment 195796 [details]
Patch

[Eric Carlson]

Comment on attachment 195796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195796&action=review

> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions-expected.txt:29
> +EXPECTED (mediaSource.duration == 'testDurationValue'), OBSERVED '3.4028234663852886e+38' FAIL

Are these failures expected? If so, this suffers from the same problem as the previous patch: not all ports will have exactly the same duration so each will need custom results.

[Aaron Colwell]

(In reply to comment #6)
> (From update of attachment 195796 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195796&action=review
> 
> > LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions-expected.txt:29
> > +EXPECTED (mediaSource.duration == 'testDurationValue'), OBSERVED '3.4028234663852886e+38' FAIL
> 
> Are these failures expected? If so, this suffers from the same problem as the previous patch: not all ports will have exactly the same duration so each will need custom results.

Yes these are expected because HTMLMediaElement.duration is a float and MediaSource.duration is a double. Until HTMLMediaElement.duration is updated to a double to match the HTML5 spec, these test cases will fail. :/ These expectations were just to capture the fact that MediaSource.duration truncates itself to the float range so that MediaSource.duration always equals HTMLMediaElement.duration.

[Eric Carlson]

Comment on attachment 195796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195796&action=review

>>> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions-expected.txt:29
>>> +EXPECTED (mediaSource.duration == 'testDurationValue'), OBSERVED '3.4028234663852886e+38' FAIL
>> 
>> Are these failures expected? If so, this suffers from the same problem as the previous patch: not all ports will have exactly the same duration so each will need custom results.
> 
> Yes these are expected because HTMLMediaElement.duration is a float and MediaSource.duration is a double. Until HTMLMediaElement.duration is updated to a double to match the HTML5 spec, these test cases will fail. :/ These expectations were just to capture the fact that MediaSource.duration truncates itself to the float range so that MediaSource.duration always equals HTMLMediaElement.duration.

Why not just fix HTMLMediaElement? Have you filed a bug about the issue?

[Aaron Colwell]

(In reply to comment #8)
> (From update of attachment 195796 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195796&action=review
> 
> >>> LayoutTests/http/tests/media/media-source/video-media-source-duration-boundaryconditions-expected.txt:29
> >>> +EXPECTED (mediaSource.duration == 'testDurationValue'), OBSERVED '3.4028234663852886e+38' FAIL
> >> 
> >> Are these failures expected? If so, this suffers from the same problem as the previous patch: not all ports will have exactly the same duration so each will need custom results.
> > 
> > Yes these are expected because HTMLMediaElement.duration is a float and MediaSource.duration is a double. Until HTMLMediaElement.duration is updated to a double to match the HTML5 spec, these test cases will fail. :/ These expectations were just to capture the fact that MediaSource.duration truncates itself to the float range so that MediaSource.duration always equals HTMLMediaElement.duration.
> 
> Why not just fix HTMLMediaElement? Have you filed a bug about the issue?

I plan on doing so. My initial survey of the code made me think this was going to be a large disruptive yak shave. I'll file a bug and start working on a patch to see how bad it really ends up being.

[WebKit Review Bot]

Comment on attachment 195796 [details]
Patch

Clearing flags on attachment: 195796

Committed r147255: <http://trac.webkit.org/changeset/147255>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.