Bug 57982 - AVF: MediaPlayerPrivateAVFoundationObjC should not use -[AVPlayerItem isPlaybackBufferEmpty]
Summary: AVF: MediaPlayerPrivateAVFoundationObjC should not use -[AVPlayerItem isPlayb...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-04-06 14:11 PDT by Jer Noble
Modified: 2011-04-06 20:34 PDT (History)
1 user (show)

See Also:

Attachments
Patch (2.09 KB, patch)
2011-04-06 14:24 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch (2.09 KB, patch)
2011-04-06 14:53 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2011-04-06 14:11:38 PDT 
-[AVPlayerItem isPlaybackBufferEmpty] often returns incorrect results.  Instead, call -[AVPlayerItem loadedTimeRanges] and determine if the current time resides within those ranges.
Comment 1 Jer Noble 2011-04-06 14:12:07 PDT 
<rdar://problem/9245087>
Comment 2 Jer Noble 2011-04-06 14:22:57 PDT 
This might result in the platformBufferedTimeRanges() being called more often.  The tradeoff here is performance vs. correctness.  As it stands, the current implementation, due to bugs in AVFoundation, is incorrect.
Comment 3 Jer Noble 2011-04-06 14:24:39 PDT 
Created attachment 88512 [details]
Patch
Comment 4 mitz 2011-04-06 14:43:10 PDT 
Comment on attachment 88512 [details]
Patch

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

r!=me

> Source/WebCore/ChangeLog:8
> +        Query our cached loaded time array instead of asking AVPlayerItem if it's buffer is empty.

it's -> its
Comment 5 Jer Noble 2011-04-06 14:53:28 PDT 
Created attachment 88519 [details]
Patch

Fixed some inverted conditionals and extraneous apostrophes.
Comment 6 Darin Adler 2011-04-06 16:49:22 PDT 
Comment on attachment 88519 [details]
Patch

That function name, contain, is really strange. It’s a verb.
Comment 7 Jer Noble 2011-04-06 16:58:38 PDT 
(In reply to comment #6)
> (From update of attachment 88519 [details])
> That function name, contain, is really strange. It’s a verb.

I know, I intensely dislike it.

Thanks for the review!
Comment 8 WebKit Commit Bot 2011-04-06 20:34:24 PDT 
Comment on attachment 88519 [details]
Patch

Clearing flags on attachment: 88519

Committed r83135: <http://trac.webkit.org/changeset/83135>
Comment 9 WebKit Commit Bot 2011-04-06 20:34:27 PDT 
All reviewed patches have been landed.  Closing bug.