Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VP8 WebCoreDecompressionSession should attribute its IOSurfaces to the media player resource owner #26032

Conversation

youennf
Copy link
Contributor

@youennf youennf commented Mar 18, 2024

fa9c6d7

VP8 WebCoreDecompressionSession should attribute its IOSurfaces to the media player resource owner
https://bugs.webkit.org/show_bug.cgi?id=271144
rdar://123795173

Reviewed by Jean-Yves Avenard.

When WebCoreDecompressionSession uses a VideoDecoder to do its decoding, it uses an IOSurface pixel buffer pool.
We then need to attribute these buffers to the corresponding resource owner.
To do so, we are setting a resourceOwner in WebCoreDecompressionSession from VideoMediaSampleRenderer, which gets it from MediaPlayerPrivateWebM.
The WebCoreDecompressionSession is creating a VideoDecoder that is given the resourceOwner so that,
everytime we have a pixel buffer coming from the buffer pool, we then do the attribution.

We also do this for MediaPlayerPrivateMediaSourceAVFObjC's session.
This means piping the resource owner to MediaSourcePrivate -> SourceBufferPrivate.

* Source/WebCore/platform/VideoDecoder.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::load):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::ensureDecompressionSession):
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
(WebCore::MediaSourcePrivateAVFObjC::addSourceBuffer):
* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::setVideoRenderer):
* Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm:
(WebCore::MediaPlayerPrivateWebM::ensureLayer):
(WebCore::MediaPlayerPrivateWebM::ensureDecompressionSession):
* Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.h:
(WebCore::VideoMediaSampleRenderer::setResourceOwner):
* Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.mm:
(WebCore::VideoMediaSampleRenderer::initializeDecompressionSession):
* Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.h:
* Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:
(WebCore::WebCoreDecompressionSession::initializeVideoDecoder):
* Source/WebCore/platform/libwebrtc/LibWebRTCVPXVideoDecoder.cpp:
(WebCore::LibWebRTCVPXInternalVideoDecoder::LibWebRTCVPXInternalVideoDecoder):
(WebCore::LibWebRTCVPXInternalVideoDecoder::Decoded):

Canonical link: https://commits.webkit.org/276270@main

0984310

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-skia
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2 ❌ πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim   πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@youennf youennf self-assigned this Mar 18, 2024
@youennf youennf added the WebRTC For bugs in WebRTC label Mar 18, 2024
@youennf youennf requested a review from jyavenard March 18, 2024 09:34
Copy link
Member

@jyavenard jyavenard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this WebCoreDecompressionSession is only used if we are painting on a canvas ; and the buffer allocation is already handled in updateLastPixelBuffer()

The WebCoreDecompressionSession that needs change is the one managed by VideoMediaSampleRenderer

@youennf youennf force-pushed the eng/VP8-WebCoreDecompressionSession-should-attribute-its-IOSurfaces-to-the-media-player-resource-owner branch from 3bafd19 to f6f9b6a Compare March 18, 2024 09:58
@youennf youennf force-pushed the eng/VP8-WebCoreDecompressionSession-should-attribute-its-IOSurfaces-to-the-media-player-resource-owner branch from f6f9b6a to 0b966ec Compare March 18, 2024 10:18
@youennf youennf marked this pull request as ready for review March 18, 2024 10:19
@youennf youennf requested a review from jyavenard March 18, 2024 10:19
@youennf youennf force-pushed the eng/VP8-WebCoreDecompressionSession-should-attribute-its-IOSurfaces-to-the-media-player-resource-owner branch from 0b966ec to 0984310 Compare March 18, 2024 11:59
@youennf youennf added the merge-queue Applied to send a pull request to merge-queue label Mar 18, 2024
@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Mar 18, 2024
@youennf youennf removed the merging-blocked Applied to prevent a change from being merged label Mar 18, 2024
@youennf
Copy link
Contributor Author

youennf commented Mar 18, 2024

gtk test failure unrelated.

@youennf youennf added the merge-queue Applied to send a pull request to merge-queue label Mar 18, 2024
…e media player resource owner

https://bugs.webkit.org/show_bug.cgi?id=271144
rdar://123795173

Reviewed by Jean-Yves Avenard.

When WebCoreDecompressionSession uses a VideoDecoder to do its decoding, it uses an IOSurface pixel buffer pool.
We then need to attribute these buffers to the corresponding resource owner.
To do so, we are setting a resourceOwner in WebCoreDecompressionSession from VideoMediaSampleRenderer, which gets it from MediaPlayerPrivateWebM.
The WebCoreDecompressionSession is creating a VideoDecoder that is given the resourceOwner so that,
everytime we have a pixel buffer coming from the buffer pool, we then do the attribution.

We also do this for MediaPlayerPrivateMediaSourceAVFObjC's session.
This means piping the resource owner to MediaSourcePrivate -> SourceBufferPrivate.

* Source/WebCore/platform/VideoDecoder.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::load):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::ensureDecompressionSession):
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
(WebCore::MediaSourcePrivateAVFObjC::addSourceBuffer):
* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::setVideoRenderer):
* Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm:
(WebCore::MediaPlayerPrivateWebM::ensureLayer):
(WebCore::MediaPlayerPrivateWebM::ensureDecompressionSession):
* Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.h:
(WebCore::VideoMediaSampleRenderer::setResourceOwner):
* Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.mm:
(WebCore::VideoMediaSampleRenderer::initializeDecompressionSession):
* Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.h:
* Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:
(WebCore::WebCoreDecompressionSession::initializeVideoDecoder):
* Source/WebCore/platform/libwebrtc/LibWebRTCVPXVideoDecoder.cpp:
(WebCore::LibWebRTCVPXInternalVideoDecoder::LibWebRTCVPXInternalVideoDecoder):
(WebCore::LibWebRTCVPXInternalVideoDecoder::Decoded):

Canonical link: https://commits.webkit.org/276270@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/VP8-WebCoreDecompressionSession-should-attribute-its-IOSurfaces-to-the-media-player-resource-owner branch from 0984310 to fa9c6d7 Compare March 18, 2024 15:12
@webkit-commit-queue
Copy link
Collaborator

Committed 276270@main (fa9c6d7): https://commits.webkit.org/276270@main

Reviewed commits have been landed. Closing PR #26032 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit fa9c6d7 into WebKit:main Mar 18, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebRTC For bugs in WebRTC
Projects
None yet
5 participants