| Summary: | Add LayoutTests that verify MediaSource.duration behavior. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aaron Colwell <acolwell> | ||||||
| Component: | New Bugs | Assignee: | Aaron Colwell <acolwell> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | eric.carlson, feature-media-reviews, jer.noble, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Aaron Colwell
2013-03-27 13:40:13 PDT
Created attachment 195388 [details]
Patch
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. 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. 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. Created attachment 195796 [details]
Patch
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. (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. 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? (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. Comment on attachment 195796 [details] Patch Clearing flags on attachment: 195796 Committed r147255: <http://trac.webkit.org/changeset/147255> All reviewed patches have been landed. Closing bug. |