⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 280723 in webkit


Ignore:
Timestamp:
Aug 6, 2021, 8:09:20 AM (5 years ago)
Author:
jer.noble@apple.com
Message:

[Cocoa] Remove support for AVAssetImageGenerator
​https://bugs.webkit.org/show_bug.cgi?id=228560
<rdar://problem/81336280>

Reviewed by Eric Carlson.

Source/WebCore:

A much more minimal approach to removing support for AVAssetImageGenerator.

The only time we use an AVAssetImageGenerator (as opposed to an AVPlayerItemVideoOutput)
is when the latter does not currently have an available image enqueued. Because painting
is a synchronous operation, we use a synchronous API (the generator) to create an image
for that operation. However, this can create deadlocks if (for example) the resource needs
to load data on the main thread in order to complete the painting operation.

Instead, allow the main runloop to spin while waiting (up to 1_s) for the video output
to receive a decoded frame.

Drive-by fixes:

  • Don't create an AVPlayerLayer at AVPlayer-creation; this causes the AVPlayerItemVideoOutput to never receive a decoded frambe (as the layer is not in a CALayer-heirarchy).
  • preferredRenderingMode() shouldn't be "none" when the page isn't visible. We already just mark the layer as "hidden" in that case.
  • Don't tear down the AVPlayerItemVideoOutput when creating an AVPlayerLayer; it'll just get re-created anyway.
  • platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:

(WebCore::MediaPlayerPrivateAVFoundation::preferredRenderingMode const):
(WebCore::MediaPlayerPrivateAVFoundation::setUpVideoRendering):

  • platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
  • platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:

(WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayer):
(WebCore::MediaPlayerPrivateAVFoundationObjC::paintCurrentFrameInContext):
(WebCore::MediaPlayerPrivateAVFoundationObjC::createVideoOutput):
(WebCore::MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput):
(WebCore::MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange):
(WebCore::MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange):
(-[WebCoreAVFPullDelegate outputMediaDataWillChange:]):
(-[WebCoreAVFPullDelegate setParent:]):

Source/WebKit:

Drive-by fix: we're passing the wrong value into acceleratedRenderingStateChanged(), and
we're not setting the correct initial value on MediaPlayerPrivateRemote creation.

  • GPUProcess/media/RemoteMediaPlayerProxy.h:
  • WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:

(WebKit::MediaPlayerPrivateRemote::MediaPlayerPrivateRemote):
(WebKit::MediaPlayerPrivateRemote::acceleratedRenderingStateChanged):

Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r280721 r280723  
     12021-08-06  Jer Noble  <jer.noble@apple.com>
     2
     3        [Cocoa] Remove support for AVAssetImageGenerator
     4        https://bugs.webkit.org/show_bug.cgi?id=228560
     5        <rdar://problem/81336280>
     6
     7        Reviewed by Eric Carlson.
     8
     9        A much more minimal approach to removing support for AVAssetImageGenerator.
     10
     11        The only time we use an AVAssetImageGenerator (as opposed to an AVPlayerItemVideoOutput)
     12        is when the latter does not currently have an available image enqueued. Because painting
     13        is a synchronous operation, we use a synchronous API (the generator) to create an image
     14        for that operation. However, this can create deadlocks if (for example) the resource needs
     15        to load data on the main thread in order to complete the painting operation.
     16
     17        Instead, allow the main runloop to spin while waiting (up to 1_s) for the video output
     18        to receive a decoded frame.
     19
     20        Drive-by fixes:
     21        - Don't create an AVPlayerLayer at AVPlayer-creation; this causes the AVPlayerItemVideoOutput
     22          to never receive a decoded frambe (as the layer is not in a CALayer-heirarchy).
     23        - preferredRenderingMode() shouldn't be "none" when the page isn't visible. We already
     24          just mark the layer as "hidden" in that case.
     25        - Don't tear down the AVPlayerItemVideoOutput when creating an AVPlayerLayer; it'll just
     26          get re-created anyway.
     27
     28        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
     29        (WebCore::MediaPlayerPrivateAVFoundation::preferredRenderingMode const):
     30        (WebCore::MediaPlayerPrivateAVFoundation::setUpVideoRendering):
     31        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
     32        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
     33        (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayer):
     34        (WebCore::MediaPlayerPrivateAVFoundationObjC::paintCurrentFrameInContext):
     35        (WebCore::MediaPlayerPrivateAVFoundationObjC::createVideoOutput):
     36        (WebCore::MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput):
     37        (WebCore::MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange):
     38        (WebCore::MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange):
     39        (-[WebCoreAVFPullDelegate outputMediaDataWillChange:]):
     40        (-[WebCoreAVFPullDelegate setParent:]):
     41
    1422021-08-06  Antti Koivisto  <antti@apple.com>
    243

  • trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp

    r280624 r280723  
    106106MediaPlayerPrivateAVFoundation::MediaRenderingMode MediaPlayerPrivateAVFoundation::preferredRenderingMode() const
    107107{
    108     if (!m_visible || assetStatus() == MediaPlayerAVAssetStatusUnknown)
     108    if (assetStatus() == MediaPlayerAVAssetStatusUnknown)
    109109        return MediaRenderingNone;
    110110
    … …  
    123123    MediaRenderingMode preferredMode = preferredRenderingMode();
    124124
    125     if (preferredMode == MediaRenderingNone)
    126         preferredMode = MediaRenderingToContext;
    127 
    128125    if (currentMode == preferredMode && currentMode != MediaRenderingNone)
    129126        return;
    130 
    131     if (currentMode != MediaRenderingNone)
    132         tearDownVideoRendering();
    133127
    134128    switch (preferredMode) {
    135129    case MediaRenderingNone:
     130        tearDownVideoRendering();
     131        break;
     132
    136133    case MediaRenderingToContext:
     134        destroyVideoLayer();
    137135        createContextVideoRenderer();
    138136        break;

  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h

    r280624 r280723  
    118118
    119119    MediaTime currentMediaTime() const final;
     120    void outputMediaDataWillChange();
    120121
    121122private:
    … …  
    436437    bool m_shouldPlayToPlaybackTarget { false };
    437438#endif
     439    bool m_runningModalPaint { false };
    438440};
    439441

  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm

    r280624 r280723  
    197197    BinarySemaphore m_semaphore;
    198198}
     199- (id)initWithPlayer:(WeakPtr<MediaPlayerPrivateAVFoundationObjC>&&)player;
    199200- (void)outputMediaDataWillChange:(AVPlayerItemOutput *)sender;
    200201- (void)outputSequenceWasFlushed:(AVPlayerItemOutput *)output;
    … …  
    10711072    }
    10721073
    1073     if (player()->isVideoPlayer())
    1074         createAVPlayerLayer();
    1075 
    10761074    if (m_avPlayerItem)
    10771075        setAVPlayerItem(m_avPlayerItem.get());
    … …  
    18031801    BEGIN_BLOCK_OBJC_EXCEPTIONS
    18041802
    1805     // Callers of this will often call copyVideoTextureToPlatformTexture first,
    1806     // which calls updateLastPixelBuffer, which clears m_lastImage whenever the
    1807     // video delivers a new frame. This breaks videoOutputHasAvailableFrame's
    1808     // short-circuiting when m_lastImage is non-null, but the video often
    1809     // doesn't have a new frame to deliver since the last time
    1810     // hasNewPixelBufferForItemTime was called against m_videoOutput. To avoid
    1811     // changing the semantics of videoOutputHasAvailableFrame in ways that might
    1812     // break other callers, look for production of a recent pixel buffer from
    1813     // the video output, too.
    1814     if (videoOutputHasAvailableFrame() || (m_videoOutput && m_lastPixelBuffer))
    1815         paintWithVideoOutput(context, rect);
    1816     else
    1817         paintWithImageGenerator(context, rect);
     1803    paintWithVideoOutput(context, rect);
    18181804
    18191805    END_BLOCK_OBJC_EXCEPTIONS
    … …  
    24402426    }
    24412427
    2442     m_videoOutputDelegate = adoptNS([[WebCoreAVFPullDelegate alloc] init]);
     2428    m_videoOutputDelegate = adoptNS([[WebCoreAVFPullDelegate alloc] initWithPlayer:makeWeakPtr(*this)]);
    24432429    [m_videoOutput setDelegate:m_videoOutputDelegate.get() queue:globalPullDelegateQueue()];
    24442430
    … …  
    25242510void MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput(GraphicsContext& context, const FloatRect& outputRect)
    25252511{
    2526     // It's crucial to not wait synchronously for the next image. Videos that
    2527     // come down this path are performing slow-case software uploads, and such
    2528     // videos may not return metadata in a timely fashion. Use the most recently
    2529     // available pixel buffer, if any.
    2530     updateLastImage();
     2512    updateLastImage(UpdateType::UpdateSynchronously);
    25312513    if (!m_lastImage)
    25322514        return;
    … …  
    25682550
    25692551    // Wait for 1 second.
    2570     bool satisfied = [m_videoOutputDelegate semaphore].waitFor(1_s);
     2552    MonotonicTime start = MonotonicTime::now();
     2553
     2554    RunLoop::Timer<MediaPlayerPrivateAVFoundationObjC> timeoutTimer { RunLoop::main(), [] {
     2555        RunLoop::main().stop();
     2556    } };
     2557    timeoutTimer.startOneShot(1_s);
     2558
     2559    m_runningModalPaint = true;
     2560    RunLoop::run();
     2561    m_runningModalPaint = false;
     2562
     2563    bool satisfied = timeoutTimer.isActive();
    25712564    if (!satisfied)
    25722565        ERROR_LOG(LOGIDENTIFIER, "timed out");
     2566    else
     2567        INFO_LOG(LOGIDENTIFIER, "waiting for videoOutput took ", (MonotonicTime::now() - start).seconds());
     2568}
     2569
     2570void MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange()
     2571{
     2572    if (m_runningModalPaint)
     2573        RunLoop::main().stop();
    25732574}
    25742575
    … …  
    39003901@end
    39013902
    3902 @implementation WebCoreAVFPullDelegate
     3903@implementation WebCoreAVFPullDelegate {
     3904    WeakPtr<WebCore::MediaPlayerPrivateAVFoundationObjC> _player;
     3905}
    39033906
    39043907@synthesize semaphore = m_semaphore;
     3908
     3909- (id)initWithPlayer:(WeakPtr<MediaPlayerPrivateAVFoundationObjC>&&)player
     3910{
     3911    self = [super init];
     3912    if (!self)
     3913        return nil;
     3914    _player = WTFMove(player);
     3915    return self;
     3916}
    39053917
    39063918- (void)outputMediaDataWillChange:(AVPlayerItemVideoOutput *)output
    … …  
    39083920    UNUSED_PARAM(output);
    39093921    m_semaphore.signal();
     3922    RunLoop::main().dispatch([player = _player] {
     3923        if (player)
     3924            player->outputMediaDataWillChange();
     3925    });
    39103926}
    39113927

  • trunk/Source/WebKit/ChangeLog

    r280722 r280723  
     12021-08-06  Jer Noble  <jer.noble@apple.com>
     2
     3        [Cocoa] Remove support for AVAssetImageGenerator
     4        https://bugs.webkit.org/show_bug.cgi?id=228560
     5        <rdar://problem/81336280>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Drive-by fix: we're passing the wrong value into acceleratedRenderingStateChanged(), and
     10        we're not setting the correct initial value on MediaPlayerPrivateRemote creation.
     11
     12        * GPUProcess/media/RemoteMediaPlayerProxy.h:
     13        * WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
     14        (WebKit::MediaPlayerPrivateRemote::MediaPlayerPrivateRemote):
     15        (WebKit::MediaPlayerPrivateRemote::acceleratedRenderingStateChanged):
     16
    1172021-08-06  Eric Carlson  <eric.carlson@apple.com>
    218

  • trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h

    r279786 r280723  
    354354
    355355    bool m_bufferedChanged { true };
    356     bool m_renderingCanBeAccelerated { true };
     356    bool m_renderingCanBeAccelerated { false };
    357357
    358358#if ENABLE(LEGACY_ENCRYPTED_MEDIA) && ENABLE(ENCRYPTED_MEDIA)

  • trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp

    r280624 r280723  
    115115{
    116116    INFO_LOG(LOGIDENTIFIER);
     117
     118    acceleratedRenderingStateChanged();
    117119}
    118120#endif
    … …  
    456458{
    457459    if (auto player = makeRefPtr(m_player.get()))
    458         connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(player->supportsAcceleratedRendering()), m_id);
     460        connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(player->renderingCanBeAccelerated()), m_id);
    459461}
    460462
Note: See TracChangeset for help on using the changeset viewer.