[PATCH] winegstreamer: Fix YUV->RGB conversion by copying colorimetry information.
This patch adds a new GStreamer element called yuvfixup, which copies colorimetry information from its super upstream source to its sink. By using yuvfixup element, the colorimetry informaiton match on videoconvert's sink pad and its source pad. Thus, we can avoid expensive YUV matrix conversions and do YUV->RGB conversion with matrix. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49535 Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> --- dlls/winegstreamer/Makefile.in | 3 +- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/gstdemux.c | 28 +++- dlls/winegstreamer/main.c | 2 + dlls/winegstreamer/yuvfixup.c | 248 +++++++++++++++++++++++++++++++ 5 files changed, 274 insertions(+), 8 deletions(-) create mode 100644 dlls/winegstreamer/yuvfixup.c
Something like this is probably fine if we can't get a better solution upstream, but it would really be nice to be able to have videoconvert propagate colorimetry information—either as the default when converting YUV <-> YUV or RGB <-> RGB, or as a configurable option. We'd want to look at gst_video_convert_fixate_caps(), I believe.
On Sun, 15 Nov 2020 19:52:31 -0600, Zebediah Figura (she/her) wrote:
Something like this is probably fine if we can't get a better solution upstream, but it would really be nice to be able to have videoconvert propagate colorimetry information—either as the default when converting YUV <-> YUV or RGB <-> RGB, or as a configurable option. We'd want to look at gst_video_convert_fixate_caps(), I believe.
Regarding videoconvert element, I found gst-base-plugin issue #614[1] and its merge request !263[2]. I'll try the patch set in the merge request. It might also solve the our issue though they aren't updated for several months. What do you think about implementing our own videoconvert element based on the above patch? [1] https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/issues/614 [2] https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/2...
On 11/16/20 8:03 AM, Akihiro Sagawa wrote:
On Sun, 15 Nov 2020 19:52:31 -0600, Zebediah Figura (she/her) wrote:
Something like this is probably fine if we can't get a better solution upstream, but it would really be nice to be able to have videoconvert propagate colorimetry information—either as the default when converting YUV <-> YUV or RGB <-> RGB, or as a configurable option. We'd want to look at gst_video_convert_fixate_caps(), I believe.
Regarding videoconvert element, I found gst-base-plugin issue #614[1] and its merge request !263[2]. I'll try the patch set in the merge request. It might also solve the our issue though they aren't updated for several months.
That does look like it's trying to achieve about the same thing, yeah. That said, I'm not a GStreamer reviewer, but I'd think the code path should look more similar to gst_video_convert_fixate_format() above (maybe even using the same function for both format and colorimetry?) Of course any attempt to upstream that or a similar patch should be accompanied by an explanation of why we want it for wine.
What do you think about implementing our own videoconvert element based on the above patch?
It's another option. Arguably it's architecturally a little nicer than "yuvfixup", but at the same time it may mean copying a lot more code.
[1] https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/issues/614 [2] https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/2...
participants (2)
-
Akihiro Sagawa -
Zebediah Figura (she/her)