From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: David Airlie <airlied@linux.ie>,
dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace
Date: Fri, 20 Jul 2018 15:26:46 +0300 [thread overview]
Message-ID: <20180720122646.GM5565@intel.com> (raw)
In-Reply-To: <20180720113930.GA1000@n2100.armlinux.org.uk>
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@fooishbar.org> wrote:
> > > > > Hi Russell,
> > > > >
> > > > > On 8 December 2017 at 12:31, Russell King <rmk+kernel@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
prev parent reply other threads:[~2018-07-20 12:27 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-08 12:13 [PATCH v2 0/5] Armada DRM fixes Russell King - ARM Linux
2017-12-08 12:14 ` [PATCH 1/5] drm/armada: fix leak of crtc structure Russell King
2017-12-08 12:14 ` [PATCH 2/5] drm/armada: fix SRAM powerdown Russell King
2017-12-08 12:14 ` [PATCH 3/5] drm/armada: fix UV swap code Russell King
2017-12-08 12:14 ` [PATCH 4/5] drm/armada: improve efficiency of armada_drm_plane_calc_addrs() Russell King
2017-12-08 12:14 ` [PATCH 5/5] drm/armada: fix YUV planar format framebuffer offsets Russell King
2017-12-08 12:27 ` [PATCH 00/25] Armada DRM development for 4.16 Russell King - ARM Linux
2017-12-08 12:29 ` [PATCH 01/25] drm/armada: remove armada_drm_plane_work_cancel() return value Russell King
2017-12-08 12:29 ` [PATCH 02/25] drm/armada: add a common frame work allocator Russell King
2017-12-08 12:29 ` [PATCH 03/25] drm/armada: store plane in armada_plane_work Russell King
2017-12-08 12:29 ` [PATCH 04/25] drm/armada: add work cancel callback Russell King
2017-12-08 12:29 ` [PATCH 05/25] drm/armada: wait and cancel any pending frame work at disable Russell King
2017-12-08 12:29 ` [PATCH 06/25] drm/armada: allow the primary plane to be disabled Russell King
2017-12-08 12:29 ` [PATCH 07/25] drm/armada: clean up armada_drm_crtc_plane_disable() Russell King
2017-12-08 12:29 ` [PATCH 08/25] drm/armada: clear plane enable bit when disabling Russell King
2017-12-08 12:29 ` [PATCH 09/25] drm/armada: move overlay plane work out from under spinlock Russell King
2017-12-08 12:29 ` [PATCH 10/25] drm/armada: move fb retirement into armada_plane_work Russell King
2017-12-08 12:29 ` [PATCH 11/25] drm/armada: move event sending " Russell King
2017-12-08 12:29 ` [PATCH 12/25] drm/armada: move regs " Russell King
2017-12-08 12:30 ` [PATCH 13/25] drm/armada: move writes of LCD_SPU_SRAM_PARA1 under lock Russell King
2017-12-08 12:30 ` [PATCH 14/25] drm/armada: only enable HSMOOTH if scaling horizontally Russell King
2017-12-08 12:30 ` [PATCH 15/25] drm/armada: use drm_plane_helper_check_state() Russell King
2017-12-08 12:30 ` [PATCH 16/25] drm/armada: allow armada_drm_plane_work_queue() to silently fail Russell King
2017-12-08 12:30 ` [PATCH 17/25] drm/armada: avoid work allocation Russell King
2017-12-08 12:30 ` [PATCH 18/25] drm/armada: disable planes at next blanking period Russell King
2017-12-08 12:30 ` [PATCH 19/25] drm/armada: re-organise overlay register update generation Russell King
2017-12-08 12:30 ` [PATCH 20/25] drm/armada: move overlay plane " Russell King
2017-12-08 12:30 ` [PATCH 21/25] drm/armada: wait for previous work when moving overlay window Russell King
2017-12-08 12:30 ` [PATCH 22/25] drm/armada: extract register generation from armada_drm_primary_set() Russell King
2017-12-08 12:30 ` [PATCH 23/25] drm/armada: implement primary plane update Russell King
2017-12-08 12:31 ` [PATCH 24/25] drm/armada: expand overlay trace entry Russell King
2017-12-08 12:31 ` [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace Russell King
2017-12-11 20:54 ` Daniel Vetter
2017-12-11 22:17 ` Russell King - ARM Linux
2017-12-13 15:02 ` Ville Syrjälä
2017-12-13 15:41 ` Daniel Stone
2017-12-13 16:07 ` Russell King - ARM Linux
2017-12-13 16:12 ` Ilia Mirkin
2017-12-13 16:22 ` Ville Syrjälä
2018-01-01 12:17 ` Russell King - ARM Linux
2018-07-20 11:39 ` Russell King - ARM Linux
2018-07-20 12:26 ` Ville Syrjälä [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180720122646.GM5565@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=airlied@linux.ie \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux@armlinux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.