Re: [PATCH 1/2] media: uapi: add WebP VP8 frame flag

From: Nicolas Dufresne
Date: Wed Sep 11 2024 - 13:13:10 EST


Hi Hugues,

Le mercredi 11 septembre 2024 à 15:50 +0200, Hugues Fruchet a écrit :
> Add a flag indicating that VP8 bitstream is a WebP picture.

Sounds like there should be some code changes in GStreamer that you haven't
disclosed. Mind sharing how this new uAPI is used ? I would also expect this
commit message to give more insight on what is special about WebP that makes
this flag required.

I would also need some more API or documentation that explain how we can
differentiate a upstream decoder that is capable of WebP decoding from one that
does not. I wonder if it would not have been better to define a new format ?
That being said, I haven't looked at all in the specification and only rely on
your cover letter and patch series.

Nicolas

>
> Signed-off-by: Hugues Fruchet <hugues.fruchet@xxxxxxxxxxx>
> ---
> .../userspace-api/media/v4l/ext-ctrls-codec-stateless.rst | 3 +++
> include/uapi/linux/v4l2-controls.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
> index 0da635691fdc..bb08aacddc9c 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
> @@ -1062,6 +1062,9 @@ FWHT Flags
> * - ``V4L2_VP8_FRAME_FLAG_SIGN_BIAS_ALT``
> - 0x20
> - Sign of motion vectors when the alt frame is referenced.
> + * - ``V4L2_VP8_FRAME_FLAG_WEBP``
> + - 0x40
> + - Indicates that this frame is a WebP picture.
>
> .. c:type:: v4l2_vp8_entropy_coder_state
>
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 974fd254e573..e41b62f2cb2b 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1897,6 +1897,7 @@ struct v4l2_vp8_entropy_coder_state {
> #define V4L2_VP8_FRAME_FLAG_MB_NO_SKIP_COEFF 0x08
> #define V4L2_VP8_FRAME_FLAG_SIGN_BIAS_GOLDEN 0x10
> #define V4L2_VP8_FRAME_FLAG_SIGN_BIAS_ALT 0x20
> +#define V4L2_VP8_FRAME_FLAG_WEBP 0x40
>
> #define V4L2_VP8_FRAME_IS_KEY_FRAME(hdr) \
> (!!((hdr)->flags & V4L2_VP8_FRAME_FLAG_KEY_FRAME))