[webkit-reviews] review granted: [Bug 234131] [Cocoa] -[AVPlayerItem liveUpdateThread] can hang the main thread for ~60ms : [Attachment 446682] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 234131: [Cocoa] -[AVPlayerItem liveUpdateThread] can hang the main thread
for ~60ms
https://bugs.webkit.org/show_bug.cgi?id=234131

Attachment 446682: Patch

https://bugs.webkit.org/attachment.cgi?id=446682&action=review




--- Comment #3 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 446682
  --> https://bugs.webkit.org/attachment.cgi?id=446682
Patch

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

> Source/WebCore/ChangeLog:3
> +	   [Cocoa] -[AVPlayerItem liveUpdateThread] can hang the main thread
for ~60ms

Do you mean -[AVPlayerItem liveUpdateInterval]?

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:3839
> +    auto queueTaskOnEventLoopWithPlayer = [self, strongSelf =
retainPtr(self)] (Function<void(MediaPlayerPrivateAVFoundationObjC&)>&&
function) mutable {

s/strongSelf = retainPtr(self)/strongSelf = RetainPtr { self }/

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:3856
> +	   auto seekableTimeRanges = retainPtr((NSArray*)newValue);

s/ = retainPtr( ... )/ = RetainPtr { ... }/

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:3858
> +	   auto playerItem = retainPtr((AVPlayerItem*)object);

The player doesn't need to be kept alive so you could avoid the refcount churn
by just casting to an AVPlayerItem

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:3866
> +    queueTaskOnEventLoopWithPlayer([keyPath = retainPtr(keyPath), change =
retainPtr(change), object = retainPtr(object), context] (auto& player) mutable
{

s/ = retainPtr( ... )/ = RetainPtr { ... }/