| Message ID | 20240911135011.161217-2-hugues.fruchet@foss.st.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add WebP support to hantro decoder | expand |
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))
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.
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))
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(+)