Bug 236883

Summary: [GStreamer] Add WebKitDMABufVideoSink
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: clord, pnormand, vjaquez, webkit-bug-importer, zdobersek
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=237758
Bug Depends on:    
Bug Blocks: 237649    
Attachments:
Description Flags
Patch
none
Patch none

Zan Dobersek
Reported 2022-02-19 08:37:59 PST
[GStreamer] Add WebKitDMABufVideoSink
Attachments
Patch (20.41 KB, patch)
2022-02-19 08:38 PST, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Patch (30.00 KB, patch)
2022-02-21 00:18 PST, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Zan Dobersek
Comment 1 2022-02-19 08:38:31 PST
Created attachment 452639 [details] Patch
Víctor M. Jáquez L.
Comment 2 2022-02-19 10:01:43 PST
So just to say it explicitly, this approach will mean that color balance, color conversion, and perhaps others filters, will be processed outside of the gstreamer pipeline.
Víctor M. Jáquez L.
Comment 3 2022-02-19 10:06:13 PST
The patch looks simple and correct. Does it work as-is? (without modifying the player side)
Zan Dobersek
Comment 4 2022-02-19 11:22:04 PST
(In reply to Víctor M. Jáquez L. from comment #2) > So just to say it explicitly, this approach will mean that color balance, > color conversion, and perhaps others filters, will be processed outside of > the gstreamer pipeline. Yes, but in the current sinks AFAIU there's no color balance changes applied, and the color conversion is only used when necessary. If color balance or some other filter would be required with the dmabuf-based sink, it would be done inside the pipeline. But for any color conversion, with a dmabuf-capable decoder, we would need to cover composition of the given color format. Any other color conversion technique, if avoidable, would just add overhead. For a raw-data decoder, if it had some unorthodox color format output, we're covered by the videoconvert element in the playbin.
Zan Dobersek
Comment 5 2022-02-19 11:24:21 PST
(In reply to Víctor M. Jáquez L. from comment #3) > The patch looks simple and correct. Does it work as-is? (without modifying > the player side) Well it works, but it requires separate changes in MediaPlayerPrivateGStreamer to properly handle these samples as well as actually use the sink. The patch is split from the larger body of work to keep it easy to review.
Philippe Normand
Comment 6 2022-02-20 11:27:35 PST
Comment on attachment 452639 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452639&action=review Can you make the style bubble green? The new file should be listed somewhere in a style-checker allowlist for GObject modules. > Source/WebCore/platform/graphics/gstreamer/DMABufVideoSinkGStreamer.cpp:164 > +void webKitDMABufVideoSinkSetMediaPlayerPrivate(WebKitDMABufVideoSink* sink, MediaPlayerPrivateGStreamer* player) maybe factor this out to a GStreamerVideoSinkCommon or Utilities module?
Zan Dobersek
Comment 7 2022-02-21 00:18:12 PST
Created attachment 452715 [details] Patch
Philippe Normand
Comment 8 2022-02-22 10:41:48 PST
Let's land this then?
Zan Dobersek
Comment 9 2022-02-23 03:15:25 PST
Waiting for the branch to happen.
Zan Dobersek (Reviews)
Comment 10 2022-02-23 03:42:25 PST
Comment on attachment 452715 [details] Patch It happened.
EWS
Comment 11 2022-02-23 04:28:38 PST
Committed r290366 (247682@main): <https://commits.webkit.org/247682@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452715 [details].
Radar WebKit Bug Importer
Comment 12 2022-02-23 04:29:19 PST
<rdar://problem/89349168>
Note You need to log in before you can comment on or make changes to this bug.