Bug 69468 - media/video-loop.html is failing on Lion because of negative currentTime()
Summary: media/video-loop.html is failing on Lion because of negative currentTime()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks: 67509
  Show dependency treegraph
 
Reported: 2011-10-05 14:57 PDT by Jer Noble
Modified: 2011-10-10 15:00 PDT (History)
0 users

See Also:


Attachments
Patch (1.88 KB, patch)
2011-10-06 14:10 PDT, Jer Noble
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2011-10-05 14:57:56 PDT
Both the following tests fail because the AVFoundation back-end on Lion can report a negative currentTime.

media/video-loop.html
media/video-pause-immediately.html
Comment 1 Jer Noble 2011-10-06 14:07:21 PDT
Actually, after digging into this, only video-loop.html is being affected by the negative-time problem.
Comment 2 Jer Noble 2011-10-06 14:10:19 PDT
Created attachment 110017 [details]
Patch
Comment 3 Darin Adler 2011-10-06 16:07:39 PDT
Comment on attachment 110017 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:435
> +        float seconds = narrowPrecisionToFloat(CMTimeGetSeconds(itemTime));
> +        return max(seconds, 0.0f);

Should not need a local variable for this. Probably reads better without it.
Comment 4 Jer Noble 2011-10-06 16:37:25 PDT
(In reply to comment #3)
> (From update of attachment 110017 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110017&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:435
> > +        float seconds = narrowPrecisionToFloat(CMTimeGetSeconds(itemTime));
> > +        return max(seconds, 0.0f);
> 
> Should not need a local variable for this. Probably reads better without it.

You're right; I was thinking of the MAX macro.  I'll change this.

Thanks!
Comment 5 Jer Noble 2011-10-10 15:00:40 PDT
Committed r97093: <http://trac.webkit.org/changeset/97093>