From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from bombadil.infradead.org ([198.137.202.133]:38712 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727534AbeIMTix (ORCPT ); Thu, 13 Sep 2018 15:38:53 -0400 Date: Thu, 13 Sep 2018 11:29:03 -0300 From: Mauro Carvalho Chehab To: Hans Verkuil Cc: linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, Hans Verkuil Subject: Re: [PATCH 1/5] media: replace ADOBERGB by OPRGB Message-ID: <20180913112903.7275b126@coco.lan> In-Reply-To: <20180913114731.16500-2-hverkuil@xs4all.nl> References: <20180913114731.16500-1-hverkuil@xs4all.nl> <20180913114731.16500-2-hverkuil@xs4all.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Em Thu, 13 Sep 2018 13:47:27 +0200 Hans Verkuil escreveu: > From: Hans Verkuil > > The CTA-861 standards have been updated to refer to opRGB instead > of AdobeRGB. The official standard is in fact named opRGB, so > switch to that. > > The two old defines referring to ADOBERGB in the public API are > put under #ifndef __KERNEL__ and a comment mentions that they are > deprecated. > > Signed-off-by: Hans Verkuil > index 184e4dbe8f9c..c1e14a3b476e 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -225,8 +225,12 @@ enum v4l2_colorspace { > /* For RGB colorspaces such as produces by most webcams. */ > V4L2_COLORSPACE_SRGB = 8, > > - /* AdobeRGB colorspace */ > + /* opRGB colorspace */ > + V4L2_COLORSPACE_OPRGB = 9, > +#ifndef __KERNEL__ > + /* Deprecated alias for V4L2_COLORSPACE_OPRGB */ > V4L2_COLORSPACE_ADOBERGB = 9, > +#endif > > /* BT.2020 colorspace, used for UHDTV. */ > V4L2_COLORSPACE_BT2020 = 10, > @@ -258,7 +262,7 @@ enum v4l2_xfer_func { > * > * V4L2_COLORSPACE_SRGB, V4L2_COLORSPACE_JPEG: V4L2_XFER_FUNC_SRGB > * > - * V4L2_COLORSPACE_ADOBERGB: V4L2_XFER_FUNC_ADOBERGB > + * V4L2_COLORSPACE_OPRGB: V4L2_XFER_FUNC_OPRGB > * > * V4L2_COLORSPACE_SMPTE240M: V4L2_XFER_FUNC_SMPTE240M > * > @@ -269,7 +273,11 @@ enum v4l2_xfer_func { > V4L2_XFER_FUNC_DEFAULT = 0, > V4L2_XFER_FUNC_709 = 1, > V4L2_XFER_FUNC_SRGB = 2, > + V4L2_XFER_FUNC_OPRGB = 3, > +#ifndef __KERNEL__ > + /* Deprecated alias for V4L2_XFER_FUNC_OPRGB */ > V4L2_XFER_FUNC_ADOBERGB = 3, > +#endif > V4L2_XFER_FUNC_SMPTE240M = 4, > V4L2_XFER_FUNC_NONE = 5, > V4L2_XFER_FUNC_DCI_P3 = 6, Nitpick: instead of having #ifndef inside the enum, I would instead place both V4L2_COLORSPACE_ADOBERGB and V4L2_XFER_FUNC_ADOBERGB on a separate #define, e. g: /* * Deprecated names for Optional RGB colorspace (IEC 61966-2) * * WARNING: Please don't use it on your code, as those can be removed * from Kernelspace in the future. */ #ifndef __KERNEL__ # define V4L2_COLORSPACE_ADOBERGB V4L2_COLORSPACE_OPRGB # define V4L2_XFER_FUNC_ADOBERGB V4L2_XFER_FUNC_OPRGB #endif There are two reasons for that: 1) by adding them inside enums and not documenting, you may end by having warnings; 2) as you mentioned on patch 0/5, one of the goals is to "avoid possible future trademark complaints." So, better to add a clear warning at the Kernel that we may need to remove it in the future. Thanks, Mauro