[PATCH] winegstreamer: Fix YUV->RGB conversion by copying colorimetry information.

[Akihiro Sagawa]

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

[Zebediah Figura (she/her)]

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.

[Akihiro Sagawa]

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...

[Zebediah Figura (she/her)]

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...