Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6674abe5681c0f4a4e4466e034393359346cfda7.camel@ndufresne.ca>
Subject: Re: [PATCH v7 1/9] media: v4l2: Extend pixel formats to unify
 single/multi-planar handling (and more)
From:   Nicolas Dufresne <nicolas@ndufresne.ca>
To:     Hsia-Jun Li <Randy.Li@synaptics.com>, ayaka <ayaka@soulik.info>
Cc:     linux-media@vger.kernel.org, Brian.Starkey@arm.com,
        boris.brezillon@collabora.com, frkoenig@chromium.org,
        hans.verkuil@cisco.com, hiroh@chromium.org, hverkuil@xs4all.nl,
        kernel@collabora.com, laurent.pinchart@ideasonboard.com,
        linux-kernel@vger.kernel.org, mchehab@kernel.org,
        narmstrong@baylibre.com, sakari.ailus@iki.fi,
        stanimir.varbanov@linaro.org, tfiga@chromium.org,
        Helen Koike <helen.koike@collabora.com>
Date:   Wed, 28 Jun 2023 12:27:22 -0400
In-Reply-To: <1e3a583f-f9ef-d2d3-6571-ac3b79d800ce@synaptics.com>
References: <20230206043308.28365-1-ayaka@soulik.info>
         <20230206043308.28365-2-ayaka@soulik.info>
         <1e3a583f-f9ef-d2d3-6571-ac3b79d800ce@synaptics.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.3 (3.48.3-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

Hi,

please avoid HTML replies on the mailing list.

Le mardi 27 juin 2023 =C3=A0 14:42 +0800, Hsia-Jun Li a =C3=A9crit=C2=A0:
> > +/**
> > + * struct v4l2_ext_pix_format - extended multiplanar format definition
> > + * @type:		enum v4l2_buf_type; type of the data stream
> > + * @width:		image width in pixels
> > + * @height:		image height in pixels
> > + * @pixelformat:	little endian four character code (fourcc)
> > + * @modifier:		modifier applied to the format (used for tiled formats
> > + *			and other kind of HW-specific formats, like compressed
> > + *			formats) as defined in drm_fourcc.h
> > + * @field:		enum v4l2_field; field order (for interlaced video)
> > + * @colorspace:		enum v4l2_colorspace; supplemental to pixelformat
> > + * @plane_fmt:		per-plane information
> > + * @flags:		format flags (V4L2_PIX_FMT_FLAG_*)
> > + * @ycbcr_enc:		enum v4l2_ycbcr_encoding, Y'CbCr encoding
> > + * @hsv_enc:		enum v4l2_hsv_encoding, HSV encoding
> > + * @quantization:	enum v4l2_quantization, colorspace quantization
> > + * @xfer_func:		enum v4l2_xfer_func, colorspace transfer function
> > + * @reserved:		drivers and applications must zero this array
> > + */
> > +struct v4l2_ext_pix_format {
> > +	__u32				type;
> > +	__u32				width;
> > +	__u32				height;
> > +	__u32				pixelformat;
> > +	__u64				modifier;
> > +	__u32				field;
> > +	__u32				colorspace;
> > +
> > +	struct v4l2_plane_pix_format	plane_fmt[VIDEO_MAX_PLANES];
> > +	__u8				flags;
> > +	 union {
> > +		__u8				ycbcr_enc;
> > +		__u8				hsv_enc;
> > +	};
> > +	__u8				quantization;
> > +	__u8				xfer_func;
> >=20
> > =C2=A0=C2=A0=C2=A0=C2=A0
> >=20
> >=20
> >=20
>=20
> =C2=A0=C2=A0=C2=A0=C2=A0I heard that a suggestion that we could remove co=
lorimetry fields
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0here.
> =C2=A0=C2=A0=C2=A0=C2=A0Although those are useless for codec M2M drivers =
if no pixel
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0formats translation invoked.
> =C2=A0=C2=A0=C2=A0=C2=A0Even HDMI(DRM) cares about colorspace.
> =C2=A0=C2=A0=C2=A0=C2=A0For example if a downsink(TV) shows RGB formats,=
=C2=A0 with an YUV
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0input frame buffer, colorimetry would=
 be important or the wrong
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0EOTF would be used. If YUV is MPEG ra=
nge(linear EOTF) while a
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0non-linear EOFT (full range) is used,=
 you would found the black is
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0not black enough while the white look=
s a gray. Also color bias
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0would happen.
> =C2=A0=C2=A0=C2=A0=C2=A0This problem may not happen to a ultra high resol=
ution TV while
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0only YUV type color formats are suppo=
rted due to HDMI bandwidth
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0limitation.
> =C2=A0=C2=A0=C2=A0=C2=A0The problem I want to raise is the time cost for =
enumeration.
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0Each pixel format with a colorimetry =
setting would invoke a
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ioctl(). For the application likes Gs=
treamer would enum all the
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0possible colorimetries.
> =C2=A0=C2=A0=C2=A0=C2=A0It would be better we could have something like D=
RM blob id that
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0application could copy the data from =
a non-DMA buffer from the
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0kernel.

This is a good topic. Colorimetry could indeed be moved away from the forma=
t,
considering they cannot be enumerated. It remains that this information nee=
ds to
be passed around, and the format of a blob in media space is not has restri=
cted
as with display HW. I think keeping an "exploded version" of the colorimetr=
y
remains needed.

Remember though that for stateful decoder, were the information could be st=
ored
in the bitstream, the decoder is responsible for returning that information=
.
Currently its passed through the G_FMT call, it would need to be replaced w=
ith a
control, similar to the HDR10 static metadata. If the colorimetry is no lon=
ger
static in the future, and may change while streaming, one option would be R=
O
request. This was foreseen for HDR10+ and Dolby Vision metadata notably, th=
ough
other options exists.

There exist known decoders that can do YUV to RGB conversion using an inlin=
e
post procesor (VC8000D and newer is an example), and for these to return co=
rrect
colors, the colorimetry information needs to be passed. So its not strictly
useless.

In short, if we drop colorimetry from format, we also need to go ahead and
design a replacement for it, that allow for the application to detect chang=
es.

regards,
Nicolas