Bug 171668

Summary: MediaSource duration attribute should not be equal to Infinity when set to a value greater than 2^64
Product: WebKit Reporter: Nael Ouedraogo <nael.ouedp>
Component: MediaAssignee: Nael Ouedraogo <nael.ouedp>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, eric.carlson, jer.noble, rniwa, youennf
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews100 for mac-elcapitan
none
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews114 for mac-elcapitan
none
Patch
none
Patch none

Description Nael Ouedraogo 2017-05-04 09:34:46 PDT
MediaSource duration attribute should not be equal to Infinity when set to a value greater than 2^64.
Comment 1 Nael Ouedraogo 2017-05-04 09:50:01 PDT
Created attachment 309056 [details]
Patch
Comment 2 Jer Noble 2017-05-09 10:35:54 PDT
Comment on attachment 309056 [details]
Patch

You can't just change the behavior of MediaTime (such as allowing it to represent double values > 2^64) without defining what happens with math operations between a rational MediaTime and a double outside 2^64.  What does it mean to do `MediaTime(1,2) * MediaTime(1e303)`?   

Also, please add API tests to verify this new behavior.
Comment 3 Nael Ouedraogo 2017-05-29 09:16:42 PDT
Created attachment 311475 [details]
Patch
Comment 4 Nael Ouedraogo 2017-05-29 09:33:32 PDT
Thanks for the comments. In uploaded patch, I modified MediaTime so that rational MediaTime is converted first to double for math operations between a rational and double MediaTime. The precision of  `MediaTime(1,2) * MediaTime(1e303)` math operation is thus the same as for the double operation (`0.5f * 1e303`). 

I added new API tests to verify the new behavior.
Comment 5 Build Bot 2017-05-29 10:14:48 PDT
Comment on attachment 311475 [details]
Patch

Attachment 311475 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3837746

New failing tests:
media/media-source/media-source-timeoffset.html
Comment 6 Build Bot 2017-05-29 10:14:50 PDT
Created attachment 311477 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 7 Build Bot 2017-05-29 10:19:59 PDT
Comment on attachment 311475 [details]
Patch

Attachment 311475 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3837748

New failing tests:
media/media-source/media-source-timeoffset.html
Comment 8 Build Bot 2017-05-29 10:20:00 PDT
Created attachment 311478 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 9 Build Bot 2017-05-29 10:34:57 PDT
Comment on attachment 311475 [details]
Patch

Attachment 311475 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3837753

New failing tests:
media/media-source/media-source-timeoffset.html
Comment 10 Build Bot 2017-05-29 10:34:58 PDT
Created attachment 311479 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 11 Nael Ouedraogo 2017-05-30 06:38:10 PDT
Created attachment 311502 [details]
Patch
Comment 12 Nael Ouedraogo 2017-06-13 10:08:49 PDT
Created attachment 312783 [details]
Patch
Comment 13 Nael Ouedraogo 2017-06-14 01:14:24 PDT
Thanks for the review.
Comment 14 WebKit Commit Bot 2017-06-14 01:44:33 PDT
Comment on attachment 312783 [details]
Patch

Clearing flags on attachment: 312783

Committed r218247: <http://trac.webkit.org/changeset/218247>
Comment 15 WebKit Commit Bot 2017-06-14 01:44:35 PDT
All reviewed patches have been landed.  Closing bug.