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

Hugues Fruchet posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH 1/2] media: uapi: add WebP VP8 frame flag
Posted by Hugues Fruchet 2 months, 2 weeks ago
Add a flag indicating that VP8 bitstream is a WebP picture.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
---
 .../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))
-- 
2.25.1
Re: [PATCH 1/2] media: uapi: add WebP VP8 frame flag
Posted by Nicolas Dufresne 2 months, 2 weeks ago
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@foss.st.com>
> ---
>  .../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))

Re: [PATCH 1/2] media: uapi: add WebP VP8 frame flag
Posted by Hugues FRUCHET 2 months, 2 weeks ago
Hi Nicolas,

Thanks for reviewing.

On 9/11/24 19:12, Nicolas Dufresne wrote:
> 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.


GStreamer changes here:
https://gitlab.freedesktop.org/gstreamer/gstreamer/-/commit/138ecfac54ce85b273a26ff6f0fefe3998f8d436?merge_request_iid=7505

Verisilicon datasheet is not explicit on why WebP must be signaled to 
hardware but WebP decoding fails if not.
Seems to me that such a simple addition on an already existing flag is 
something acceptable and preferable to the development of a new complete 
uAPI for WebP decoding.

> 
> 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@foss.st.com>
>> ---
>>   .../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))
> 

BR,
Hugues.