[PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Ville Syrjälä]

On Fri, Jul 20, 2018 at 12:39:30PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jan 01, 2018 at 12:17:35PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 13, 2017 at 06:22:14PM +0200, Ville Syrjälä wrote:
> > > On Wed, Dec 13, 2017 at 11:12:18AM -0500, Ilia Mirkin wrote:
> > > > On Wed, Dec 13, 2017 at 10:41 AM, Daniel Stone <daniel at fooishbar.org> wrote:
> > > > > Hi Russell,
> > > > >
> > > > > On 8 December 2017 at 12:31, Russell King <rmk+kernel at armlinux.org.uk> wrote:
> > > > >> Add the defacto-standard "iturbt_709" property to the overlay plane to
> > > > >> control the YUV to RGB colorspace conversion.  This is mutually
> > > > >> exclusive with the CSC_YUV CRTC property - the last property to be set
> > > > >> determines the resulting colorspace conversion.
> > > > >
> > > > > I haven't seen this in other drivers - is it a 'defacto standard'? I
> > > > 
> > > > xf86-video-nv supported it, and I added it to nouveau as well when I
> > > > ported YUV plane support. Some video players use the Xv property when
> > > > available.
> > > > 
> > > > https://cgit.freedesktop.org/xorg/driver/xf86-video-nv/tree/src/nv_video.c#n128
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/dispnv04/overlay.c?h=v4.15-rc3#n316
> > > 
> > >    {XvSettable | XvGettable, 0, 1, "XV_ITURBT_709"}
> > > 
> > > Who came up with that and when? XV_COLORSPACE was the one semi-standard
> > > I know of.
> > 
> > I've no idea, and I was hoping that someone else would know - my use of
> > it comes from research into what will make userspace work, not what
> > standards may say.
> > 
> > XV_ITURBT_709 is already in-use in distro standard userspace programs:
> > 
> > # grep XV_ITURBT_709 /usr/lib/vlc/plugins/ -r
> > Binary file /usr/lib/vlc/plugins/video_output/libxcb_xv_plugin.so matches
> > # grep XV_ITURBT_709 /usr/lib/gstreamer-1.0/ -r
> > Binary file /usr/lib/gstreamer-1.0/libgstxvimagesink.so matches
> > 
> > but not XV_COLORSPACE:
> > 
> > # grep XV_COLORSPACE /usr/lib/vlc/plugins/ -r
> > # grep XV_COLORSPACE /usr/lib/gstreamer-1.0/ -r
> > 
> > So while XV_COLORSPACE may be some kind of standard, it seems that
> > userspace has decided otherwise to go with a different name for this
> > control.
> 
> Re-opening this discussion, since the above point was never replied to.
> 
> I can find no video players that make use of the "XV_COLORSPACE"
> property, but two that make use of the "XV_ITURBT_709" property.

mpv supports both, and xvattr is of course one way to change it but
requires manual user intervention.

I have a patch locally for gst xvimagesink to use XV_COLORSPACE as
well. I wrote it when I posted the proposed XV_COLOR_RANGE attribute
(and I wrote patches for that one too, for gst and mpv). Didn't
bother posting any of these until I got some feedback on
XV_COLOR_RANGE. No feedback was received though so I suppose
everyone is OK with it.

> 
> It was added to gstreamer in 2014:
> https://github.com/GStreamer/gst-plugins-base/commit/d99e270fc83278c309ec7cad20d75181d90b8722
> 
> and is present in VLC since 2016:
> https://github.com/videolan/vlc/commit/8172a5470964550a1e5d6e2b7082650f932e6ce6
> 
> We seem to have the situation where some Xv backends implement
> "XV_COLORSPACE", others "XV_ITURBT_709" but players implement only
> "XV_ITURBT_709".
> 
> A DDX /could/ consider implementing both, but there is no way to
> notify Xv event listeners from a DDX's Xv backend that "the other"
> property has been changed - XvdiSendPortNotify() needs an XvPortPtr
> but the DDX has no access to that due to the xf86 layer on top
> hiding that, and there's nothing at xf86 level to allow that.
> 
> This doesn't seem to be a very productive situation, and certainly
> not useful for the user.
> 
> I think that the conclusion I'd come to given this is that 99.9% of
> people don't care about correct Xv colorimetry.

Also Xv usage is probably on the decline seeing as it's not used
by browsers which is probably how many people watch videos these days.

-- 
Ville Syrjälä
Intel