Bug 139248

Summary: [WTF] MediaTime should support round-tripping from and to doubles.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cmarcelo, commit-queue, eric.carlson, glenn, philipj, rniwa, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
none
Patch
none
Patch
eric.carlson: review+
Patch for landing
none
Patch for landing
none
Patch for landing none

Description Jer Noble 2014-12-03 18:01:01 PST
[WTF] MediaTime should support round-tripping from and to doubles.
Comment 1 Jer Noble 2014-12-03 18:12:34 PST
Created attachment 242546 [details]
Patch
Comment 2 Build Bot 2014-12-03 19:51:36 PST
Comment on attachment 242546 [details]
Patch

Attachment 242546 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6483620982161408

Number of test failures exceeded the failure limit.
Comment 3 Build Bot 2014-12-03 19:51:39 PST
Created attachment 242551 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Build Bot 2014-12-03 20:32:53 PST
Comment on attachment 242546 [details]
Patch

Attachment 242546 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5155735742709760

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2014-12-03 20:32:56 PST
Created attachment 242552 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2014-12-03 22:34:07 PST
Comment on attachment 242546 [details]
Patch

Attachment 242546 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6348355819012096

Number of test failures exceeded the failure limit.
Comment 7 Build Bot 2014-12-03 22:34:09 PST
Created attachment 242553 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Jer Noble 2014-12-04 12:09:09 PST
Created attachment 242579 [details]
Patch
Comment 9 Jer Noble 2014-12-04 14:58:59 PST
Created attachment 242590 [details]
Patch
Comment 10 Eric Carlson 2014-12-05 05:46:10 PST
Comment on attachment 242590 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/MediaTimeAVFoundation.cpp:73
> +    if (mediaTime.hasDoubleValue())
> +        time = CMTimeMakeWithSeconds(mediaTime.toDouble(), MediaTime::DefaultTimeScale);

This overwrites the flags you just set, can it be done as an early return?
Comment 11 Jer Noble 2014-12-05 08:47:22 PST
(In reply to comment #10)
> Comment on attachment 242590 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242590&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/MediaTimeAVFoundation.cpp:73
> > +    if (mediaTime.hasDoubleValue())
> > +        time = CMTimeMakeWithSeconds(mediaTime.toDouble(), MediaTime::DefaultTimeScale);
> 
> This overwrites the flags you just set, can it be done as an early return?

Sure, I'll just move this if() statement to the first line. (So that further NaN checks, infinity checks, etc, can still set the correct flags.)
Comment 12 Jer Noble 2014-12-05 09:55:30 PST
Created attachment 242637 [details]
Patch for landing
Comment 13 Jer Noble 2014-12-05 10:04:10 PST
Created attachment 242639 [details]
Patch for landing
Comment 14 Jer Noble 2014-12-05 11:13:50 PST
Created attachment 242645 [details]
Patch for landing
Comment 15 WebKit Commit Bot 2014-12-05 11:29:30 PST
Attachment 242645 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/MediaTimeAVFoundation.cpp:33:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 Jer Noble 2014-12-05 12:50:33 PST
Committed r176863: <http://trac.webkit.org/changeset/176863>