public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add WebP support to hantro decoder
@ 2024-11-20 11:01 Hugues Fruchet
  2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Hugues Fruchet @ 2024-11-20 11:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Ezequiel Garcia, Philipp Zabel,
	Hans Verkuil, Fritz Koenig, Sebastian Fricke, Daniel Almeida,
	Andrzej Pietrasiewicz, Nicolas Dufresne, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32
  Cc: Hugues Fruchet

Add WebP image decoding support to stateless V4L2 VP8 decoder.

This have been tested on STM32MP257-EV board using GStreamer.

Simple basic test:
$> wget https://www.gstatic.com/webp/gallery/1.webp
$> gst-launch-1.0 filesrc location= 1.webp ! typefind ! v4l2slwebpdec ! imagefreeze num-buffers=20 ! waylandsink fullscreen=true

Slideshow of a set of WebP pictures and WebM video files:
$> wget https://www.gstatic.com/webp/gallery/2.webp
$> wget https://www.gstatic.com/webp/gallery/3.webp
$> wget https://www.gstatic.com/webp/gallery/4.webp
$> wget https://www.gstatic.com/webp/gallery/5.webp
$> wget https://samplemedia.linaro.org/VP8/big_buck_bunny_480p_VP8_VORBIS_25fps_1900K_short.WebM
$> gst-play-1.0 *.webp *.webm *.WebM --wait-on-eos
<hit key ">" to display next file >

Large WebP image > 16777215 (size > 2^24)
$> gst-launch-1.0 fakesrc num-buffers=1 format=4 do-timestamp=true filltype=3 sizetype=2 sizemax=25165824 blocksize=25165824 ! video/x-raw, format=I420, width=4096, height=3072, framerate=1/1 ! webpenc quality=100 ! filesink location=4096x3072_HQ_random.webp
$> ls -l 4096x3072_HQ_random.webp
[...] 16877404 Nov 20 11:40 4096x3072_HQ_random.webp
$> gst-launch-1.0 filesrc location= 4096x3072_HQ_random.webp blocksize=16876610 ! image/webp, width=1, height=1, framerate=0/1 ! v4l2slwebpdec ! imagefreeze num-buffers=20 ! waylandsink fullscreen=true

Large WebP image decoding using post-processor is untested because of lack
of hardware support on this platform, nevertheless support is provided in
this serie for further testing on another platform having post-processor
support.


Hugues Fruchet (3):
  media: uapi: add WebP uAPI
  media: verisilicon: add WebP decoding support
  media: verisilicon: postproc: 4K support

 .../userspace-api/media/v4l/biblio.rst          |  9 +++++++++
 .../media/v4l/pixfmt-compressed.rst             | 15 +++++++++++++++
 drivers/media/platform/verisilicon/hantro.h     |  2 ++
 .../media/platform/verisilicon/hantro_g1_regs.h |  3 ++-
 .../platform/verisilicon/hantro_g1_vp8_dec.c    | 14 ++++++++++++++
 .../platform/verisilicon/hantro_postproc.c      |  6 +++++-
 .../media/platform/verisilicon/hantro_v4l2.c    |  2 ++
 .../platform/verisilicon/stm32mp25_vpu_hw.c     | 17 +++++++++++++++--
 drivers/media/v4l2-core/v4l2-ioctl.c            |  1 +
 include/uapi/linux/videodev2.h                  |  1 +
 10 files changed, 66 insertions(+), 4 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 11:01 [PATCH v2 0/3] Add WebP support to hantro decoder Hugues Fruchet
@ 2024-11-20 11:01 ` Hugues Fruchet
  2024-11-20 13:42   ` Link Mauve
                     ` (3 more replies)
  2024-11-20 11:01 ` [PATCH v2 2/3] media: verisilicon: add WebP decoding support Hugues Fruchet
  2024-11-20 11:01 ` [PATCH v2 3/3] media: verisilicon: postproc: 4K support Hugues Fruchet
  2 siblings, 4 replies; 16+ messages in thread
From: Hugues Fruchet @ 2024-11-20 11:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Ezequiel Garcia, Philipp Zabel,
	Hans Verkuil, Fritz Koenig, Sebastian Fricke, Daniel Almeida,
	Andrzej Pietrasiewicz, Nicolas Dufresne, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32
  Cc: Hugues Fruchet

This patch adds the WebP picture decoding kernel uAPI.

This design is based on currently available VP8 API implementation and
aims to support the development of WebP stateless video codecs
on Linux.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
---
 Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
 .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
 drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
 include/uapi/linux/videodev2.h                    |  1 +
 4 files changed, 26 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
index 35674eeae20d..df3e963fc54f 100644
--- a/Documentation/userspace-api/media/v4l/biblio.rst
+++ b/Documentation/userspace-api/media/v4l/biblio.rst
@@ -447,3 +447,12 @@ AV1
 :title:     AV1 Bitstream & Decoding Process Specification
 
 :author:    Peter de Rivaz, Argon Design Ltd, Jack Haughton, Argon Design Ltd
+
+.. _webp:
+
+WEBP
+====
+
+:title:     WEBP picture Bitstream & Decoding Process Specification
+
+:author:    Google (https://developers.google.com/speed/webp)
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
index 806ed73ac474..e664e70b0619 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
@@ -169,6 +169,21 @@ Compressed Formats
 	this pixel format. The output buffer must contain the appropriate number
 	of macroblocks to decode a full corresponding frame to the matching
 	capture buffer.
+    * .. _V4L2-PIX-FMT-WEBP-FRAME:
+
+      - ``V4L2_PIX_FMT_WEBP_FRAME``
+      - 'WEBP'
+      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
+	bistream including the frame header, as extracted from the container.
+	This format is adapted for stateless video decoders that implement a
+	WEBP pipeline with the :ref:`stateless_decoder`.
+	Metadata associated with the frame to decode is required to be passed
+	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
+	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
+	Exactly one output and one capture buffer must be provided for use with
+	this pixel format. The output buffer must contain the appropriate number
+	of macroblocks to decode a full corresponding frame to the matching
+	capture buffer.
 
     * .. _V4L2-PIX-FMT-VP9:
 
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 0304daa8471d..e2ff03d0d773 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1501,6 +1501,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
 		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
 		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 Frame"; break;
+		case V4L2_PIX_FMT_WEBP_FRAME:    descr = "WEBP VP8 Frame"; break;
 		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
 		case V4L2_PIX_FMT_VP9_FRAME:    descr = "VP9 Frame"; break;
 		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index e7c4dce39007..09fff269e852 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -757,6 +757,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
 #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
 #define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frame */
+#define V4L2_PIX_FMT_WEBP_FRAME v4l2_fourcc('W', 'B', 'P', 'F') /* WEBP VP8 parsed frame */
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_VP9_FRAME v4l2_fourcc('V', 'P', '9', 'F') /* VP9 parsed frame */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] media: verisilicon: add WebP decoding support
  2024-11-20 11:01 [PATCH v2 0/3] Add WebP support to hantro decoder Hugues Fruchet
  2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
@ 2024-11-20 11:01 ` Hugues Fruchet
  2024-11-20 14:25   ` Nicolas Dufresne
  2024-11-20 11:01 ` [PATCH v2 3/3] media: verisilicon: postproc: 4K support Hugues Fruchet
  2 siblings, 1 reply; 16+ messages in thread
From: Hugues Fruchet @ 2024-11-20 11:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Ezequiel Garcia, Philipp Zabel,
	Hans Verkuil, Fritz Koenig, Sebastian Fricke, Daniel Almeida,
	Andrzej Pietrasiewicz, Nicolas Dufresne, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32
  Cc: Hugues Fruchet

Add WebP picture decoding support to VP8 stateless decoder.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
---
 .../media/platform/verisilicon/hantro_g1_regs.h |  1 +
 .../platform/verisilicon/hantro_g1_vp8_dec.c    | 14 ++++++++++++++
 .../media/platform/verisilicon/hantro_v4l2.c    |  2 ++
 .../platform/verisilicon/stm32mp25_vpu_hw.c     | 17 +++++++++++++++--
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
index c623b3b0be18..e7d4db788e57 100644
--- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
+++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
@@ -232,6 +232,7 @@
 #define     G1_REG_DEC_CTRL7_DCT7_START_BIT(x)		(((x) & 0x3f) << 0)
 #define G1_REG_ADDR_STR					0x030
 #define G1_REG_ADDR_DST					0x034
+#define G1_REG_ADDR_DST_CHROMA				0x038
 #define G1_REG_ADDR_REF(i)				(0x038 + ((i) * 0x4))
 #define     G1_REG_ADDR_REF_FIELD_E			BIT(1)
 #define     G1_REG_ADDR_REF_TOPC_E			BIT(0)
diff --git a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
index 851eb67f19f5..c83ee6f5edc8 100644
--- a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
+++ b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
@@ -307,6 +307,12 @@ static void cfg_parts(struct hantro_ctx *ctx,
 			   G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
 			   G1_REG_DEC_CTRL3);
 
+	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
+		vdpu_write_relaxed(vpu,
+				   G1_REG_DEC_CTRL3_STREAM_LEN_EXT
+					(dct_part_total_len >> 24),
+				   G1_REG_DEC_CTRL3);
+
 	/* DCT partitions base address */
 	for (i = 0; i < hdr->num_dct_parts; i++) {
 		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
@@ -427,6 +433,12 @@ static void cfg_buffers(struct hantro_ctx *ctx,
 
 	dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
 	vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
+
+	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
+		vdpu_write_relaxed(vpu, dst_dma +
+				   ctx->dst_fmt.plane_fmt[0].bytesperline *
+				   ctx->dst_fmt.height,
+				   G1_REG_ADDR_DST_CHROMA);
 }
 
 int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
@@ -471,6 +483,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
 		reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
 	if (hdr->lf.level == 0)
 		reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
+	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
+		reg |= G1_REG_DEC_CTRL0_WEBP_E;
 	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
 
 	/* Frame dimensions */
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 2513adfbd825..7075b2ba1ec2 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -470,6 +470,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
 		break;
 	case V4L2_PIX_FMT_MPEG2_SLICE:
 	case V4L2_PIX_FMT_VP8_FRAME:
+	case V4L2_PIX_FMT_WEBP_FRAME:
 	case V4L2_PIX_FMT_H264_SLICE:
 	case V4L2_PIX_FMT_HEVC_SLICE:
 	case V4L2_PIX_FMT_VP9_FRAME:
@@ -492,6 +493,7 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
 	case V4L2_PIX_FMT_JPEG:
 	case V4L2_PIX_FMT_MPEG2_SLICE:
 	case V4L2_PIX_FMT_VP8_FRAME:
+	case V4L2_PIX_FMT_WEBP_FRAME:
 	case V4L2_PIX_FMT_HEVC_SLICE:
 	case V4L2_PIX_FMT_VP9_FRAME:
 		vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
diff --git a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
index 833821120b20..48d6912c3bab 100644
--- a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
+++ b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
@@ -22,10 +22,10 @@ static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
 		.codec_mode = HANTRO_MODE_NONE,
 		.frmsize = {
 			.min_width = FMT_MIN_WIDTH,
-			.max_width = FMT_FHD_WIDTH,
+			.max_width = FMT_4K_WIDTH,
 			.step_width = MB_DIM,
 			.min_height = FMT_MIN_HEIGHT,
-			.max_height = FMT_FHD_HEIGHT,
+			.max_height = FMT_4K_HEIGHT,
 			.step_height = MB_DIM,
 		},
 	},
@@ -68,6 +68,19 @@ static const struct hantro_fmt stm32mp25_venc_fmts[] = {
 		.codec_mode = HANTRO_MODE_NONE,
 		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
 	},
+	{
+		.fourcc = V4L2_PIX_FMT_WEBP_FRAME,
+		.codec_mode = HANTRO_MODE_VP8_DEC,
+		.max_depth = 2,
+		.frmsize = {
+			.min_width = FMT_MIN_WIDTH,
+			.max_width = FMT_4K_WIDTH,
+			.step_width = MB_DIM,
+			.min_height = FMT_MIN_HEIGHT,
+			.max_height = FMT_4K_HEIGHT,
+			.step_height = MB_DIM,
+		},
+	},
 	{
 		.fourcc = V4L2_PIX_FMT_YUYV,
 		.codec_mode = HANTRO_MODE_NONE,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] media: verisilicon: postproc: 4K support
  2024-11-20 11:01 [PATCH v2 0/3] Add WebP support to hantro decoder Hugues Fruchet
  2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
  2024-11-20 11:01 ` [PATCH v2 2/3] media: verisilicon: add WebP decoding support Hugues Fruchet
@ 2024-11-20 11:01 ` Hugues Fruchet
  2024-12-12 14:26   ` Nicolas Dufresne
  2 siblings, 1 reply; 16+ messages in thread
From: Hugues Fruchet @ 2024-11-20 11:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Ezequiel Garcia, Philipp Zabel,
	Hans Verkuil, Fritz Koenig, Sebastian Fricke, Daniel Almeida,
	Andrzej Pietrasiewicz, Nicolas Dufresne, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32
  Cc: Hugues Fruchet

Support input larger than 4096x2048 using extended input width/height
fields of swreg92.
This is needed to decode large WebP or JPEG pictures.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
---
 drivers/media/platform/verisilicon/hantro.h          | 2 ++
 drivers/media/platform/verisilicon/hantro_g1_regs.h  | 2 +-
 drivers/media/platform/verisilicon/hantro_postproc.c | 6 +++++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index 811260dc3c77..d1337f7742e4 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -321,6 +321,8 @@ struct hantro_postproc_regs {
 	struct hantro_reg output_fmt;
 	struct hantro_reg orig_width;
 	struct hantro_reg display_width;
+	struct hantro_reg input_width_ext;
+	struct hantro_reg input_height_ext;
 };
 
 struct hantro_vp9_decoded_buffer_info {
diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
index e7d4db788e57..f6e5bbeb1914 100644
--- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
+++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
@@ -351,7 +351,7 @@
 #define     G1_REG_PP_CONTROL_OUT_WIDTH(v) (((v) << 4) & GENMASK(14, 4))
 #define G1_REG_PP_MASK1_ORIG_WIDTH	G1_SWREG(88)
 #define     G1_REG_PP_ORIG_WIDTH(v)	(((v) << 23) & GENMASK(31, 23))
-#define G1_REG_PP_DISPLAY_WIDTH		G1_SWREG(92)
+#define G1_REG_PP_DISPLAY_WIDTH_IN_EXT	G1_SWREG(92)
 #define G1_REG_PP_FUSE			G1_SWREG(99)
 
 #endif /* HANTRO_G1_REGS_H_ */
diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
index 232c93eea7ee..84c8e287470d 100644
--- a/drivers/media/platform/verisilicon/hantro_postproc.c
+++ b/drivers/media/platform/verisilicon/hantro_postproc.c
@@ -49,7 +49,9 @@ static const struct hantro_postproc_regs hantro_g1_postproc_regs = {
 	.input_fmt = {G1_REG_PP_CONTROL, 29, 0x7},
 	.output_fmt = {G1_REG_PP_CONTROL, 26, 0x7},
 	.orig_width = {G1_REG_PP_MASK1_ORIG_WIDTH, 23, 0x1ff},
-	.display_width = {G1_REG_PP_DISPLAY_WIDTH, 0, 0xfff},
+	.display_width = {G1_REG_PP_DISPLAY_WIDTH_IN_EXT, 0, 0xfff},
+	.input_width_ext = {G1_REG_PP_DISPLAY_WIDTH_IN_EXT, 26, 0x7},
+	.input_height_ext = {G1_REG_PP_DISPLAY_WIDTH_IN_EXT, 29, 0x7},
 };
 
 bool hantro_needs_postproc(const struct hantro_ctx *ctx,
@@ -103,6 +105,8 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
 	HANTRO_PP_REG_WRITE(vpu, output_height, ctx->dst_fmt.height);
 	HANTRO_PP_REG_WRITE(vpu, orig_width, MB_WIDTH(ctx->dst_fmt.width));
 	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
+	HANTRO_PP_REG_WRITE(vpu, input_width_ext, MB_WIDTH(ctx->dst_fmt.width) >> 9);
+	HANTRO_PP_REG_WRITE(vpu, input_height_ext, MB_HEIGHT(ctx->dst_fmt.height >> 8));
 }
 
 static int down_scale_factor(struct hantro_ctx *ctx)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
@ 2024-11-20 13:42   ` Link Mauve
  2024-11-20 13:57     ` Nicolas Dufresne
  2024-11-20 14:21   ` Nicolas Dufresne
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Link Mauve @ 2024-11-20 13:42 UTC (permalink / raw)
  To: Hugues Fruchet
  Cc: Mauro Carvalho Chehab, Ezequiel Garcia, Philipp Zabel,
	Hans Verkuil, Fritz Koenig, Sebastian Fricke, Daniel Almeida,
	Andrzej Pietrasiewicz, Nicolas Dufresne, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Hi,

On Wed, Nov 20, 2024 at 12:01:03PM +0100, Hugues Fruchet wrote:
> This patch adds the WebP picture decoding kernel uAPI.
> 
> This design is based on currently available VP8 API implementation and
> aims to support the development of WebP stateless video codecs
> on Linux.

Why do you need this new uAPI exactly?  The WebP format is more complex
than the simple 'VP8 ' format, the 'VP8X' fourcc for instance is an
animated format which may contain multiple VP8 keyframes, or an alpha
side channel, and just like any other video container we queue each
VP8 frame separately in V4L2 for decoding, not the whole file.

In Onix[1] I parse the WebP header and pass the raw VP8 frame to V4L2
without the RIFF around it.

So I’d rather NACK this patch, I don’t think it’s a good idea to
hardcode the simplest version of the WebP container in the uAPI, to the
detriment of all other possible WebP files.

[1] git clone https://git.linkmauve.fr/onix.git/

> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> ---
>  Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
>  .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
>  drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
>  include/uapi/linux/videodev2.h                    |  1 +
>  4 files changed, 26 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
> index 35674eeae20d..df3e963fc54f 100644
> --- a/Documentation/userspace-api/media/v4l/biblio.rst
> +++ b/Documentation/userspace-api/media/v4l/biblio.rst
> @@ -447,3 +447,12 @@ AV1
>  :title:     AV1 Bitstream & Decoding Process Specification
>  
>  :author:    Peter de Rivaz, Argon Design Ltd, Jack Haughton, Argon Design Ltd
> +
> +.. _webp:
> +
> +WEBP
> +====
> +
> +:title:     WEBP picture Bitstream & Decoding Process Specification
> +
> +:author:    Google (https://developers.google.com/speed/webp)
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> index 806ed73ac474..e664e70b0619 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> @@ -169,6 +169,21 @@ Compressed Formats
>  	this pixel format. The output buffer must contain the appropriate number
>  	of macroblocks to decode a full corresponding frame to the matching
>  	capture buffer.
> +    * .. _V4L2-PIX-FMT-WEBP-FRAME:
> +
> +      - ``V4L2_PIX_FMT_WEBP_FRAME``
> +      - 'WEBP'
> +      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
> +	bistream including the frame header, as extracted from the container.
> +	This format is adapted for stateless video decoders that implement a
> +	WEBP pipeline with the :ref:`stateless_decoder`.
> +	Metadata associated with the frame to decode is required to be passed
> +	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
> +	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
> +	Exactly one output and one capture buffer must be provided for use with
> +	this pixel format. The output buffer must contain the appropriate number
> +	of macroblocks to decode a full corresponding frame to the matching
> +	capture buffer.
>  
>      * .. _V4L2-PIX-FMT-VP9:
>  
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 0304daa8471d..e2ff03d0d773 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1501,6 +1501,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
>  		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
>  		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 Frame"; break;
> +		case V4L2_PIX_FMT_WEBP_FRAME:    descr = "WEBP VP8 Frame"; break;
>  		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
>  		case V4L2_PIX_FMT_VP9_FRAME:    descr = "VP9 Frame"; break;
>  		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index e7c4dce39007..09fff269e852 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -757,6 +757,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
>  #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
>  #define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frame */
> +#define V4L2_PIX_FMT_WEBP_FRAME v4l2_fourcc('W', 'B', 'P', 'F') /* WEBP VP8 parsed frame */
>  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
>  #define V4L2_PIX_FMT_VP9_FRAME v4l2_fourcc('V', 'P', '9', 'F') /* VP9 parsed frame */
>  #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

-- 
Link Mauve

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 13:42   ` Link Mauve
@ 2024-11-20 13:57     ` Nicolas Dufresne
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Dufresne @ 2024-11-20 13:57 UTC (permalink / raw)
  To: Link Mauve, Hugues Fruchet
  Cc: Mauro Carvalho Chehab, Ezequiel Garcia, Philipp Zabel,
	Hans Verkuil, Fritz Koenig, Sebastian Fricke, Daniel Almeida,
	Andrzej Pietrasiewicz, Benjamin Gaignard, linux-media,
	linux-kernel, linux-rockchip, linux-stm32

Hi,

Le mercredi 20 novembre 2024 à 14:42 +0100, Link Mauve a écrit :
> Hi,
> 
> On Wed, Nov 20, 2024 at 12:01:03PM +0100, Hugues Fruchet wrote:
> > This patch adds the WebP picture decoding kernel uAPI.
> > 
> > This design is based on currently available VP8 API implementation and
> > aims to support the development of WebP stateless video codecs
> > on Linux.
> 
> Why do you need this new uAPI exactly?  The WebP format is more complex
> than the simple 'VP8 ' format, the 'VP8X' fourcc for instance is an
> animated format which may contain multiple VP8 keyframes, or an alpha
> side channel, and just like any other video container we queue each
> VP8 frame separately in V4L2 for decoding, not the whole file.

This is a v2, you should have read v1 thread before commenting this in my
opinion. Let me resume it: While a VP8 HW decoder is compatible with WebP (lossy
only), a WebP (lossy) decoders are not always compatible wit VP8. The reason is
that some design may sacrifice support for references in order to allow larger
resolutions.

This is proven by Hantro implementation, which is the main implementation out
there since back when Google owned it this hardware design was given away for
free to anyone wanting to support VP8 in HW.

> 
> In Onix[1] I parse the WebP header and pass the raw VP8 frame to V4L2
> without the RIFF around it.
> 
> So I’d rather NACK this patch, I don’t think it’s a good idea to
> hardcode the simplest version of the WebP container in the uAPI, to the
> detriment of all other possible WebP files.

I don't expect the RIFF to be passed to the stateless decoder, this is a
stateless interface that is proposed here. Though, you need the RIFF to determin
if the stream is Lossy WebP (keyframe only), so if someone comes up with a
stateful HW design, it will face the same challenge. As said, what you do in
Onix is valid, but limiting. 

Let's first have a look at the code before taking out big guns, also make sure
you have the full context before proposing to nack something.

regards,
Nicolas

> 
> [1] git clone https://git.linkmauve.fr/onix.git/
> 
> > 
> > Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> > ---
> >  Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
> >  .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
> >  drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
> >  include/uapi/linux/videodev2.h                    |  1 +
> >  4 files changed, 26 insertions(+)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
> > index 35674eeae20d..df3e963fc54f 100644
> > --- a/Documentation/userspace-api/media/v4l/biblio.rst
> > +++ b/Documentation/userspace-api/media/v4l/biblio.rst
> > @@ -447,3 +447,12 @@ AV1
> >  :title:     AV1 Bitstream & Decoding Process Specification
> >  
> >  :author:    Peter de Rivaz, Argon Design Ltd, Jack Haughton, Argon Design Ltd
> > +
> > +.. _webp:
> > +
> > +WEBP
> > +====
> > +
> > +:title:     WEBP picture Bitstream & Decoding Process Specification
> > +
> > +:author:    Google (https://developers.google.com/speed/webp)
> > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> > index 806ed73ac474..e664e70b0619 100644
> > --- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> > +++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> > @@ -169,6 +169,21 @@ Compressed Formats
> >  	this pixel format. The output buffer must contain the appropriate number
> >  	of macroblocks to decode a full corresponding frame to the matching
> >  	capture buffer.
> > +    * .. _V4L2-PIX-FMT-WEBP-FRAME:
> > +
> > +      - ``V4L2_PIX_FMT_WEBP_FRAME``
> > +      - 'WEBP'
> > +      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
> > +	bistream including the frame header, as extracted from the container.
> > +	This format is adapted for stateless video decoders that implement a
> > +	WEBP pipeline with the :ref:`stateless_decoder`.
> > +	Metadata associated with the frame to decode is required to be passed
> > +	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
> > +	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
> > +	Exactly one output and one capture buffer must be provided for use with
> > +	this pixel format. The output buffer must contain the appropriate number
> > +	of macroblocks to decode a full corresponding frame to the matching
> > +	capture buffer.
> >  
> >      * .. _V4L2-PIX-FMT-VP9:
> >  
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 0304daa8471d..e2ff03d0d773 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1501,6 +1501,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> >  		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
> >  		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
> >  		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 Frame"; break;
> > +		case V4L2_PIX_FMT_WEBP_FRAME:    descr = "WEBP VP8 Frame"; break;
> >  		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
> >  		case V4L2_PIX_FMT_VP9_FRAME:    descr = "VP9 Frame"; break;
> >  		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index e7c4dce39007..09fff269e852 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -757,6 +757,7 @@ struct v4l2_pix_format {
> >  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
> >  #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
> >  #define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frame */
> > +#define V4L2_PIX_FMT_WEBP_FRAME v4l2_fourcc('W', 'B', 'P', 'F') /* WEBP VP8 parsed frame */
> >  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
> >  #define V4L2_PIX_FMT_VP9_FRAME v4l2_fourcc('V', 'P', '9', 'F') /* VP9 parsed frame */
> >  #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
> > -- 
> > 2.25.1
> > 
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
  2024-11-20 13:42   ` Link Mauve
@ 2024-11-20 14:21   ` Nicolas Dufresne
  2024-11-21  9:59     ` Hugues FRUCHET
  2024-11-20 15:43   ` Diederik de Haas
  2024-12-12 14:34   ` Nicolas Dufresne
  3 siblings, 1 reply; 16+ messages in thread
From: Nicolas Dufresne @ 2024-11-20 14:21 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Hi Hughe,

thanks for the update.

Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
> This patch adds the WebP picture decoding kernel uAPI.
> 
> This design is based on currently available VP8 API implementation and
> aims to support the development of WebP stateless video codecs
> on Linux.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> ---
>  Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
>  .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
>  drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
>  include/uapi/linux/videodev2.h                    |  1 +
>  4 files changed, 26 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
> index 35674eeae20d..df3e963fc54f 100644
> --- a/Documentation/userspace-api/media/v4l/biblio.rst
> +++ b/Documentation/userspace-api/media/v4l/biblio.rst
> @@ -447,3 +447,12 @@ AV1
>  :title:     AV1 Bitstream & Decoding Process Specification
>  
>  :author:    Peter de Rivaz, Argon Design Ltd, Jack Haughton, Argon Design Ltd
> +
> +.. _webp:
> +
> +WEBP
> +====
> +
> +:title:     WEBP picture Bitstream & Decoding Process Specification
> +
> +:author:    Google (https://developers.google.com/speed/webp)
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> index 806ed73ac474..e664e70b0619 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> @@ -169,6 +169,21 @@ Compressed Formats
>  	this pixel format. The output buffer must contain the appropriate number
>  	of macroblocks to decode a full corresponding frame to the matching
>  	capture buffer.
> +    * .. _V4L2-PIX-FMT-WEBP-FRAME:
> +
> +      - ``V4L2_PIX_FMT_WEBP_FRAME``
> +      - 'WEBP'
> +      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
> +	bistream including the frame header, as extracted from the container.
> +	This format is adapted for stateless video decoders that implement a
> +	WEBP pipeline with the :ref:`stateless_decoder`.
> +	Metadata associated with the frame to decode is required to be passed
> +	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
> +	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
> +	Exactly one output and one capture buffer must be provided for use with
> +	this pixel format. The output buffer must contain the appropriate number
> +	of macroblocks to decode a full corresponding frame to the matching
> +	capture buffer.

I wonder if we should document the constraints, I think
V4L2_VP8_FRAME_FLAG_KEY_FRAME must be set, which imply that last/golden/alt
timestamp are ignored.

With that clarified:

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> 
>  
>      * .. _V4L2-PIX-FMT-VP9:
>  
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 0304daa8471d..e2ff03d0d773 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1501,6 +1501,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
>  		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
>  		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 Frame"; break;
> +		case V4L2_PIX_FMT_WEBP_FRAME:    descr = "WEBP VP8 Frame"; break;
>  		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
>  		case V4L2_PIX_FMT_VP9_FRAME:    descr = "VP9 Frame"; break;
>  		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index e7c4dce39007..09fff269e852 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -757,6 +757,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
>  #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
>  #define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frame */
> +#define V4L2_PIX_FMT_WEBP_FRAME v4l2_fourcc('W', 'B', 'P', 'F') /* WEBP VP8 parsed frame */
>  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
>  #define V4L2_PIX_FMT_VP9_FRAME v4l2_fourcc('V', 'P', '9', 'F') /* VP9 parsed frame */
>  #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] media: verisilicon: add WebP decoding support
  2024-11-20 11:01 ` [PATCH v2 2/3] media: verisilicon: add WebP decoding support Hugues Fruchet
@ 2024-11-20 14:25   ` Nicolas Dufresne
  2024-11-21 10:07     ` Hugues FRUCHET
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas Dufresne @ 2024-11-20 14:25 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
> Add WebP picture decoding support to VP8 stateless decoder.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> ---
>  .../media/platform/verisilicon/hantro_g1_regs.h |  1 +
>  .../platform/verisilicon/hantro_g1_vp8_dec.c    | 14 ++++++++++++++
>  .../media/platform/verisilicon/hantro_v4l2.c    |  2 ++
>  .../platform/verisilicon/stm32mp25_vpu_hw.c     | 17 +++++++++++++++--
>  4 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
> index c623b3b0be18..e7d4db788e57 100644
> --- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
> +++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
> @@ -232,6 +232,7 @@
>  #define     G1_REG_DEC_CTRL7_DCT7_START_BIT(x)		(((x) & 0x3f) << 0)
>  #define G1_REG_ADDR_STR					0x030
>  #define G1_REG_ADDR_DST					0x034
> +#define G1_REG_ADDR_DST_CHROMA				0x038
>  #define G1_REG_ADDR_REF(i)				(0x038 + ((i) * 0x4))
>  #define     G1_REG_ADDR_REF_FIELD_E			BIT(1)
>  #define     G1_REG_ADDR_REF_TOPC_E			BIT(0)
> diff --git a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> index 851eb67f19f5..c83ee6f5edc8 100644
> --- a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> +++ b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> @@ -307,6 +307,12 @@ static void cfg_parts(struct hantro_ctx *ctx,
>  			   G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
>  			   G1_REG_DEC_CTRL3);
>  
> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
> +		vdpu_write_relaxed(vpu,
> +				   G1_REG_DEC_CTRL3_STREAM_LEN_EXT
> +					(dct_part_total_len >> 24),
> +				   G1_REG_DEC_CTRL3);
> +
>  	/* DCT partitions base address */
>  	for (i = 0; i < hdr->num_dct_parts; i++) {
>  		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
> @@ -427,6 +433,12 @@ static void cfg_buffers(struct hantro_ctx *ctx,
>  
>  	dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
>  	vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
> +
> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
> +		vdpu_write_relaxed(vpu, dst_dma +
> +				   ctx->dst_fmt.plane_fmt[0].bytesperline *
> +				   ctx->dst_fmt.height,
> +				   G1_REG_ADDR_DST_CHROMA);
>  }
>  
>  int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
> @@ -471,6 +483,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>  		reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
>  	if (hdr->lf.level == 0)
>  		reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
> +		reg |= G1_REG_DEC_CTRL0_WEBP_E;
>  	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
>  
>  	/* Frame dimensions */
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index 2513adfbd825..7075b2ba1ec2 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -470,6 +470,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
>  		break;
>  	case V4L2_PIX_FMT_MPEG2_SLICE:
>  	case V4L2_PIX_FMT_VP8_FRAME:
> +	case V4L2_PIX_FMT_WEBP_FRAME:
>  	case V4L2_PIX_FMT_H264_SLICE:
>  	case V4L2_PIX_FMT_HEVC_SLICE:
>  	case V4L2_PIX_FMT_VP9_FRAME:
> @@ -492,6 +493,7 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
>  	case V4L2_PIX_FMT_JPEG:
>  	case V4L2_PIX_FMT_MPEG2_SLICE:
>  	case V4L2_PIX_FMT_VP8_FRAME:
> +	case V4L2_PIX_FMT_WEBP_FRAME:
>  	case V4L2_PIX_FMT_HEVC_SLICE:
>  	case V4L2_PIX_FMT_VP9_FRAME:
>  		vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
> diff --git a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
> index 833821120b20..48d6912c3bab 100644
> --- a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
> @@ -22,10 +22,10 @@ static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
>  		.codec_mode = HANTRO_MODE_NONE,
>  		.frmsize = {
>  			.min_width = FMT_MIN_WIDTH,
> -			.max_width = FMT_FHD_WIDTH,
> +			.max_width = FMT_4K_WIDTH,
>  			.step_width = MB_DIM,
>  			.min_height = FMT_MIN_HEIGHT,
> -			.max_height = FMT_FHD_HEIGHT,
> +			.max_height = FMT_4K_HEIGHT,

I'm a little surprised of this change, since this is modifying VP8_FRAME, while
we should instead introduce WEBP_FRAME.

>  			.step_height = MB_DIM,
>  		},
>  	},
> @@ -68,6 +68,19 @@ static const struct hantro_fmt stm32mp25_venc_fmts[] = {
>  		.codec_mode = HANTRO_MODE_NONE,
>  		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>  	},
> +	{
> +		.fourcc = V4L2_PIX_FMT_WEBP_FRAME,
> +		.codec_mode = HANTRO_MODE_VP8_DEC,
> +		.max_depth = 2,
> +		.frmsize = {
> +			.min_width = FMT_MIN_WIDTH,
> +			.max_width = FMT_4K_WIDTH,
> +			.step_width = MB_DIM,
> +			.min_height = FMT_MIN_HEIGHT,
> +			.max_height = FMT_4K_HEIGHT,
> +			.step_height = MB_DIM,
> +		},
> +	},

This is venc_fmt (encoder), this shouldn't be there.

>  	{
>  		.fourcc = V4L2_PIX_FMT_YUYV,
>  		.codec_mode = HANTRO_MODE_NONE,


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
  2024-11-20 13:42   ` Link Mauve
  2024-11-20 14:21   ` Nicolas Dufresne
@ 2024-11-20 15:43   ` Diederik de Haas
  2024-11-21 10:08     ` Hugues FRUCHET
  2024-12-12 14:34   ` Nicolas Dufresne
  3 siblings, 1 reply; 16+ messages in thread
From: Diederik de Haas @ 2024-11-20 15:43 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Nicolas Dufresne,
	Benjamin Gaignard, linux-media, linux-kernel, linux-rockchip,
	linux-stm32

[-- Attachment #1: Type: text/plain, Size: 2118 bytes --]

On Wed Nov 20, 2024 at 12:01 PM CET, Hugues Fruchet wrote:
> This patch adds the WebP picture decoding kernel uAPI.
>
> This design is based on currently available VP8 API implementation and
> aims to support the development of WebP stateless video codecs
> on Linux.
>
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> ---
>  Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
>  .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
>  drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
>  include/uapi/linux/videodev2.h                    |  1 +
>  4 files changed, 26 insertions(+)
>
> ...
>
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> index 806ed73ac474..e664e70b0619 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> @@ -169,6 +169,21 @@ Compressed Formats
>  	this pixel format. The output buffer must contain the appropriate number
>  	of macroblocks to decode a full corresponding frame to the matching
>  	capture buffer.
> +    * .. _V4L2-PIX-FMT-WEBP-FRAME:
> +
> +      - ``V4L2_PIX_FMT_WEBP_FRAME``
> +      - 'WEBP'
> +      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
> +	bistream including the frame header, as extracted from the container.

s/bistream/bitstream/ ?

> +	This format is adapted for stateless video decoders that implement a
> +	WEBP pipeline with the :ref:`stateless_decoder`.
> +	Metadata associated with the frame to decode is required to be passed
> +	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
> +	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
> +	Exactly one output and one capture buffer must be provided for use with
> +	this pixel format. The output buffer must contain the appropriate number
> +	of macroblocks to decode a full corresponding frame to the matching
> +	capture buffer.
>  
>      * .. _V4L2-PIX-FMT-VP9:
>  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 14:21   ` Nicolas Dufresne
@ 2024-11-21  9:59     ` Hugues FRUCHET
  0 siblings, 0 replies; 16+ messages in thread
From: Hugues FRUCHET @ 2024-11-21  9:59 UTC (permalink / raw)
  To: Nicolas Dufresne, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Hi Nicolas,

thanks for review,

On 11/20/24 15:21, Nicolas Dufresne wrote:
> Hi Hughe,
> 
> thanks for the update.
> 
> Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
>> This patch adds the WebP picture decoding kernel uAPI.
>>
>> This design is based on currently available VP8 API implementation and
>> aims to support the development of WebP stateless video codecs
>> on Linux.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
>> ---
>>   Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
>>   .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
>>   drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
>>   include/uapi/linux/videodev2.h                    |  1 +
>>   4 files changed, 26 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
>> index 35674eeae20d..df3e963fc54f 100644
>> --- a/Documentation/userspace-api/media/v4l/biblio.rst
>> +++ b/Documentation/userspace-api/media/v4l/biblio.rst
>> @@ -447,3 +447,12 @@ AV1
>>   :title:     AV1 Bitstream & Decoding Process Specification
>>   
>>   :author:    Peter de Rivaz, Argon Design Ltd, Jack Haughton, Argon Design Ltd
>> +
>> +.. _webp:
>> +
>> +WEBP
>> +====
>> +
>> +:title:     WEBP picture Bitstream & Decoding Process Specification
>> +
>> +:author:    Google (https://developers.google.com/speed/webp)
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
>> index 806ed73ac474..e664e70b0619 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
>> @@ -169,6 +169,21 @@ Compressed Formats
>>   	this pixel format. The output buffer must contain the appropriate number
>>   	of macroblocks to decode a full corresponding frame to the matching
>>   	capture buffer.
>> +    * .. _V4L2-PIX-FMT-WEBP-FRAME:
>> +
>> +      - ``V4L2_PIX_FMT_WEBP_FRAME``
>> +      - 'WEBP'
>> +      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
>> +	bistream including the frame header, as extracted from the container.
>> +	This format is adapted for stateless video decoders that implement a
>> +	WEBP pipeline with the :ref:`stateless_decoder`.
>> +	Metadata associated with the frame to decode is required to be passed
>> +	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
>> +	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
>> +	Exactly one output and one capture buffer must be provided for use with
>> +	this pixel format. The output buffer must contain the appropriate number
>> +	of macroblocks to decode a full corresponding frame to the matching
>> +	capture buffer.
> 
> I wonder if we should document the constraints, I think
> V4L2_VP8_FRAME_FLAG_KEY_FRAME must be set, which imply that last/golden/alt
> timestamp are ignored.

I will add something about that.

> 
> With that clarified:
> 
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
>>
>>   
>>       * .. _V4L2-PIX-FMT-VP9:
>>   
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 0304daa8471d..e2ff03d0d773 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -1501,6 +1501,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>   		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
>>   		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
>>   		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 Frame"; break;
>> +		case V4L2_PIX_FMT_WEBP_FRAME:    descr = "WEBP VP8 Frame"; break;
>>   		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
>>   		case V4L2_PIX_FMT_VP9_FRAME:    descr = "VP9 Frame"; break;
>>   		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index e7c4dce39007..09fff269e852 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -757,6 +757,7 @@ struct v4l2_pix_format {
>>   #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
>>   #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
>>   #define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frame */
>> +#define V4L2_PIX_FMT_WEBP_FRAME v4l2_fourcc('W', 'B', 'P', 'F') /* WEBP VP8 parsed frame */
>>   #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
>>   #define V4L2_PIX_FMT_VP9_FRAME v4l2_fourcc('V', 'P', '9', 'F') /* VP9 parsed frame */
>>   #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
> 

BR,
Hugues.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] media: verisilicon: add WebP decoding support
  2024-11-20 14:25   ` Nicolas Dufresne
@ 2024-11-21 10:07     ` Hugues FRUCHET
  2024-11-21 14:20       ` Nicolas Dufresne
  0 siblings, 1 reply; 16+ messages in thread
From: Hugues FRUCHET @ 2024-11-21 10:07 UTC (permalink / raw)
  To: Nicolas Dufresne, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Hi Nicolas,

On 11/20/24 15:25, Nicolas Dufresne wrote:
> Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
>> Add WebP picture decoding support to VP8 stateless decoder.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
>> ---
>>   .../media/platform/verisilicon/hantro_g1_regs.h |  1 +
>>   .../platform/verisilicon/hantro_g1_vp8_dec.c    | 14 ++++++++++++++
>>   .../media/platform/verisilicon/hantro_v4l2.c    |  2 ++
>>   .../platform/verisilicon/stm32mp25_vpu_hw.c     | 17 +++++++++++++++--
>>   4 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>> index c623b3b0be18..e7d4db788e57 100644
>> --- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
>> +++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>> @@ -232,6 +232,7 @@
>>   #define     G1_REG_DEC_CTRL7_DCT7_START_BIT(x)		(((x) & 0x3f) << 0)
>>   #define G1_REG_ADDR_STR					0x030
>>   #define G1_REG_ADDR_DST					0x034
>> +#define G1_REG_ADDR_DST_CHROMA				0x038
>>   #define G1_REG_ADDR_REF(i)				(0x038 + ((i) * 0x4))
>>   #define     G1_REG_ADDR_REF_FIELD_E			BIT(1)
>>   #define     G1_REG_ADDR_REF_TOPC_E			BIT(0)
>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>> index 851eb67f19f5..c83ee6f5edc8 100644
>> --- a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>> +++ b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>> @@ -307,6 +307,12 @@ static void cfg_parts(struct hantro_ctx *ctx,
>>   			   G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
>>   			   G1_REG_DEC_CTRL3);
>>   
>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>> +		vdpu_write_relaxed(vpu,
>> +				   G1_REG_DEC_CTRL3_STREAM_LEN_EXT
>> +					(dct_part_total_len >> 24),
>> +				   G1_REG_DEC_CTRL3);
>> +
>>   	/* DCT partitions base address */
>>   	for (i = 0; i < hdr->num_dct_parts; i++) {
>>   		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
>> @@ -427,6 +433,12 @@ static void cfg_buffers(struct hantro_ctx *ctx,
>>   
>>   	dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
>>   	vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
>> +
>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>> +		vdpu_write_relaxed(vpu, dst_dma +
>> +				   ctx->dst_fmt.plane_fmt[0].bytesperline *
>> +				   ctx->dst_fmt.height,
>> +				   G1_REG_ADDR_DST_CHROMA);
>>   }
>>   
>>   int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>> @@ -471,6 +483,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>>   		reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
>>   	if (hdr->lf.level == 0)
>>   		reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>> +		reg |= G1_REG_DEC_CTRL0_WEBP_E;
>>   	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
>>   
>>   	/* Frame dimensions */
>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> index 2513adfbd825..7075b2ba1ec2 100644
>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> @@ -470,6 +470,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
>>   		break;
>>   	case V4L2_PIX_FMT_MPEG2_SLICE:
>>   	case V4L2_PIX_FMT_VP8_FRAME:
>> +	case V4L2_PIX_FMT_WEBP_FRAME:
>>   	case V4L2_PIX_FMT_H264_SLICE:
>>   	case V4L2_PIX_FMT_HEVC_SLICE:
>>   	case V4L2_PIX_FMT_VP9_FRAME:
>> @@ -492,6 +493,7 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
>>   	case V4L2_PIX_FMT_JPEG:
>>   	case V4L2_PIX_FMT_MPEG2_SLICE:
>>   	case V4L2_PIX_FMT_VP8_FRAME:
>> +	case V4L2_PIX_FMT_WEBP_FRAME:
>>   	case V4L2_PIX_FMT_HEVC_SLICE:
>>   	case V4L2_PIX_FMT_VP9_FRAME:
>>   		vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
>> diff --git a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>> index 833821120b20..48d6912c3bab 100644
>> --- a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>> +++ b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>> @@ -22,10 +22,10 @@ static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
>>   		.codec_mode = HANTRO_MODE_NONE,
>>   		.frmsize = {
>>   			.min_width = FMT_MIN_WIDTH,
>> -			.max_width = FMT_FHD_WIDTH,
>> +			.max_width = FMT_4K_WIDTH,
>>   			.step_width = MB_DIM,
>>   			.min_height = FMT_MIN_HEIGHT,
>> -			.max_height = FMT_FHD_HEIGHT,
>> +			.max_height = FMT_4K_HEIGHT,
> 
> I'm a little surprised of this change, since this is modifying VP8_FRAME, while
> we should instead introduce WEBP_FRAME.

This is the resolution of the YUV output of decoder, not the WebP input, 
and because of lack of post-processor, the output is not scaled, so can 
go up to 4K with WebP.
Before WebP introduction, the maximum output resolution was FHD for all 
codecs. Now WebP allows up to 4K but FHD constraint remains for 
H264/VP8. I don't see real problems because VP8/H264 compressed inputs 
are well limited to FHD and only WebP allows 4K...

> 
>>   			.step_height = MB_DIM,
>>   		},
>>   	},
>> @@ -68,6 +68,19 @@ static const struct hantro_fmt stm32mp25_venc_fmts[] = {
>>   		.codec_mode = HANTRO_MODE_NONE,
>>   		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>>   	},
>> +	{
>> +		.fourcc = V4L2_PIX_FMT_WEBP_FRAME,
>> +		.codec_mode = HANTRO_MODE_VP8_DEC,
>> +		.max_depth = 2,
>> +		.frmsize = {
>> +			.min_width = FMT_MIN_WIDTH,
>> +			.max_width = FMT_4K_WIDTH,
>> +			.step_width = MB_DIM,
>> +			.min_height = FMT_MIN_HEIGHT,
>> +			.max_height = FMT_4K_HEIGHT,
>> +			.step_height = MB_DIM,
>> +		},
>> +	},
> 
> This is venc_fmt (encoder), this shouldn't be there.

All apologizes for this rebase issue, it is of course part of 
stm32mp25_vdec_fmts.

> 
>>   	{
>>   		.fourcc = V4L2_PIX_FMT_YUYV,
>>   		.codec_mode = HANTRO_MODE_NONE,
> 

BR,
Hugues.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 15:43   ` Diederik de Haas
@ 2024-11-21 10:08     ` Hugues FRUCHET
  0 siblings, 0 replies; 16+ messages in thread
From: Hugues FRUCHET @ 2024-11-21 10:08 UTC (permalink / raw)
  To: Diederik de Haas, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Nicolas Dufresne,
	Benjamin Gaignard, linux-media, linux-kernel, linux-rockchip,
	linux-stm32

Hi Diederik,

On 11/20/24 16:43, Diederik de Haas wrote:
> On Wed Nov 20, 2024 at 12:01 PM CET, Hugues Fruchet wrote:
>> This patch adds the WebP picture decoding kernel uAPI.
>>
>> This design is based on currently available VP8 API implementation and
>> aims to support the development of WebP stateless video codecs
>> on Linux.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
>> ---
>>   Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
>>   .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
>>   drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
>>   include/uapi/linux/videodev2.h                    |  1 +
>>   4 files changed, 26 insertions(+)
>>
>> ...
>>
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
>> index 806ed73ac474..e664e70b0619 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
>> @@ -169,6 +169,21 @@ Compressed Formats
>>   	this pixel format. The output buffer must contain the appropriate number
>>   	of macroblocks to decode a full corresponding frame to the matching
>>   	capture buffer.
>> +    * .. _V4L2-PIX-FMT-WEBP-FRAME:
>> +
>> +      - ``V4L2_PIX_FMT_WEBP_FRAME``
>> +      - 'WEBP'
>> +      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
>> +	bistream including the frame header, as extracted from the container.
> 
> s/bistream/bitstream/ ?

Thanks for noticing this typo, will fix in v3...

> 
>> +	This format is adapted for stateless video decoders that implement a
>> +	WEBP pipeline with the :ref:`stateless_decoder`.
>> +	Metadata associated with the frame to decode is required to be passed
>> +	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
>> +	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
>> +	Exactly one output and one capture buffer must be provided for use with
>> +	this pixel format. The output buffer must contain the appropriate number
>> +	of macroblocks to decode a full corresponding frame to the matching
>> +	capture buffer.
>>   
>>       * .. _V4L2-PIX-FMT-VP9:
>>   

BR,
Hugues.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] media: verisilicon: add WebP decoding support
  2024-11-21 10:07     ` Hugues FRUCHET
@ 2024-11-21 14:20       ` Nicolas Dufresne
  2024-11-21 14:28         ` Hugues FRUCHET
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas Dufresne @ 2024-11-21 14:20 UTC (permalink / raw)
  To: Hugues FRUCHET, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Hi Hugues,

Le jeudi 21 novembre 2024 à 11:07 +0100, Hugues FRUCHET a écrit :
> Hi Nicolas,
> 
> On 11/20/24 15:25, Nicolas Dufresne wrote:
> > Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
> > > Add WebP picture decoding support to VP8 stateless decoder.
> > > 
> > > Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> > > ---
> > >   .../media/platform/verisilicon/hantro_g1_regs.h |  1 +
> > >   .../platform/verisilicon/hantro_g1_vp8_dec.c    | 14 ++++++++++++++
> > >   .../media/platform/verisilicon/hantro_v4l2.c    |  2 ++
> > >   .../platform/verisilicon/stm32mp25_vpu_hw.c     | 17 +++++++++++++++--
> > >   4 files changed, 32 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
> > > index c623b3b0be18..e7d4db788e57 100644
> > > --- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
> > > +++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
> > > @@ -232,6 +232,7 @@
> > >   #define     G1_REG_DEC_CTRL7_DCT7_START_BIT(x)		(((x) & 0x3f) << 0)
> > >   #define G1_REG_ADDR_STR					0x030
> > >   #define G1_REG_ADDR_DST					0x034
> > > +#define G1_REG_ADDR_DST_CHROMA				0x038
> > >   #define G1_REG_ADDR_REF(i)				(0x038 + ((i) * 0x4))
> > >   #define     G1_REG_ADDR_REF_FIELD_E			BIT(1)
> > >   #define     G1_REG_ADDR_REF_TOPC_E			BIT(0)
> > > diff --git a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> > > index 851eb67f19f5..c83ee6f5edc8 100644
> > > --- a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> > > +++ b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> > > @@ -307,6 +307,12 @@ static void cfg_parts(struct hantro_ctx *ctx,
> > >   			   G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
> > >   			   G1_REG_DEC_CTRL3);
> > >   
> > > +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
> > > +		vdpu_write_relaxed(vpu,
> > > +				   G1_REG_DEC_CTRL3_STREAM_LEN_EXT
> > > +					(dct_part_total_len >> 24),
> > > +				   G1_REG_DEC_CTRL3);
> > > +
> > >   	/* DCT partitions base address */
> > >   	for (i = 0; i < hdr->num_dct_parts; i++) {
> > >   		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
> > > @@ -427,6 +433,12 @@ static void cfg_buffers(struct hantro_ctx *ctx,
> > >   
> > >   	dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
> > >   	vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
> > > +
> > > +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
> > > +		vdpu_write_relaxed(vpu, dst_dma +
> > > +				   ctx->dst_fmt.plane_fmt[0].bytesperline *
> > > +				   ctx->dst_fmt.height,
> > > +				   G1_REG_ADDR_DST_CHROMA);
> > >   }
> > >   
> > >   int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
> > > @@ -471,6 +483,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
> > >   		reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
> > >   	if (hdr->lf.level == 0)
> > >   		reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
> > > +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
> > > +		reg |= G1_REG_DEC_CTRL0_WEBP_E;
> > >   	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
> > >   
> > >   	/* Frame dimensions */
> > > diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> > > index 2513adfbd825..7075b2ba1ec2 100644
> > > --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> > > +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> > > @@ -470,6 +470,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
> > >   		break;
> > >   	case V4L2_PIX_FMT_MPEG2_SLICE:
> > >   	case V4L2_PIX_FMT_VP8_FRAME:
> > > +	case V4L2_PIX_FMT_WEBP_FRAME:
> > >   	case V4L2_PIX_FMT_H264_SLICE:
> > >   	case V4L2_PIX_FMT_HEVC_SLICE:
> > >   	case V4L2_PIX_FMT_VP9_FRAME:
> > > @@ -492,6 +493,7 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
> > >   	case V4L2_PIX_FMT_JPEG:
> > >   	case V4L2_PIX_FMT_MPEG2_SLICE:
> > >   	case V4L2_PIX_FMT_VP8_FRAME:
> > > +	case V4L2_PIX_FMT_WEBP_FRAME:
> > >   	case V4L2_PIX_FMT_HEVC_SLICE:
> > >   	case V4L2_PIX_FMT_VP9_FRAME:
> > >   		vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
> > > diff --git a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
> > > index 833821120b20..48d6912c3bab 100644
> > > --- a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
> > > +++ b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
> > > @@ -22,10 +22,10 @@ static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
> > >   		.codec_mode = HANTRO_MODE_NONE,
> > >   		.frmsize = {
> > >   			.min_width = FMT_MIN_WIDTH,
> > > -			.max_width = FMT_FHD_WIDTH,
> > > +			.max_width = FMT_4K_WIDTH,
> > >   			.step_width = MB_DIM,
> > >   			.min_height = FMT_MIN_HEIGHT,
> > > -			.max_height = FMT_FHD_HEIGHT,
> > > +			.max_height = FMT_4K_HEIGHT,
> > 
> > I'm a little surprised of this change, since this is modifying VP8_FRAME, while
> > we should instead introduce WEBP_FRAME.
> 
> This is the resolution of the YUV output of decoder, not the WebP input, 
> and because of lack of post-processor, the output is not scaled, so can 
> go up to 4K with WebP.
> Before WebP introduction, the maximum output resolution was FHD for all 
> codecs. Now WebP allows up to 4K but FHD constraint remains for 
> H264/VP8. I don't see real problems because VP8/H264 compressed inputs 
> are well limited to FHD and only WebP allows 4K...

Good point. Would you mind adding a justification for this change within the
commit message in v3 ?

> 
> > 
> > >   			.step_height = MB_DIM,
> > >   		},
> > >   	},
> > > @@ -68,6 +68,19 @@ static const struct hantro_fmt stm32mp25_venc_fmts[] = {
> > >   		.codec_mode = HANTRO_MODE_NONE,
> > >   		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
> > >   	},
> > > +	{
> > > +		.fourcc = V4L2_PIX_FMT_WEBP_FRAME,
> > > +		.codec_mode = HANTRO_MODE_VP8_DEC,
> > > +		.max_depth = 2,
> > > +		.frmsize = {
> > > +			.min_width = FMT_MIN_WIDTH,
> > > +			.max_width = FMT_4K_WIDTH,
> > > +			.step_width = MB_DIM,
> > > +			.min_height = FMT_MIN_HEIGHT,
> > > +			.max_height = FMT_4K_HEIGHT,
> > > +			.step_height = MB_DIM,
> > > +		},
> > > +	},
> > 
> > This is venc_fmt (encoder), this shouldn't be there.
> 
> All apologizes for this rebase issue, it is of course part of 
> stm32mp25_vdec_fmts.

Ack, let's get this right in v3 :-D

> 
> > 
> > >   	{
> > >   		.fourcc = V4L2_PIX_FMT_YUYV,
> > >   		.codec_mode = HANTRO_MODE_NONE,
> > 
> 
> BR,
> Hugues.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] media: verisilicon: add WebP decoding support
  2024-11-21 14:20       ` Nicolas Dufresne
@ 2024-11-21 14:28         ` Hugues FRUCHET
  0 siblings, 0 replies; 16+ messages in thread
From: Hugues FRUCHET @ 2024-11-21 14:28 UTC (permalink / raw)
  To: Nicolas Dufresne, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Hi Nicolas,

On 11/21/24 15:20, Nicolas Dufresne wrote:
> Hi Hugues,
> 
> Le jeudi 21 novembre 2024 à 11:07 +0100, Hugues FRUCHET a écrit :
>> Hi Nicolas,
>>
>> On 11/20/24 15:25, Nicolas Dufresne wrote:
>>> Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
>>>> Add WebP picture decoding support to VP8 stateless decoder.
>>>>
>>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
>>>> ---
>>>>    .../media/platform/verisilicon/hantro_g1_regs.h |  1 +
>>>>    .../platform/verisilicon/hantro_g1_vp8_dec.c    | 14 ++++++++++++++
>>>>    .../media/platform/verisilicon/hantro_v4l2.c    |  2 ++
>>>>    .../platform/verisilicon/stm32mp25_vpu_hw.c     | 17 +++++++++++++++--
>>>>    4 files changed, 32 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>>>> index c623b3b0be18..e7d4db788e57 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
>>>> +++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
>>>> @@ -232,6 +232,7 @@
>>>>    #define     G1_REG_DEC_CTRL7_DCT7_START_BIT(x)		(((x) & 0x3f) << 0)
>>>>    #define G1_REG_ADDR_STR					0x030
>>>>    #define G1_REG_ADDR_DST					0x034
>>>> +#define G1_REG_ADDR_DST_CHROMA				0x038
>>>>    #define G1_REG_ADDR_REF(i)				(0x038 + ((i) * 0x4))
>>>>    #define     G1_REG_ADDR_REF_FIELD_E			BIT(1)
>>>>    #define     G1_REG_ADDR_REF_TOPC_E			BIT(0)
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>>>> index 851eb67f19f5..c83ee6f5edc8 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>>>> +++ b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
>>>> @@ -307,6 +307,12 @@ static void cfg_parts(struct hantro_ctx *ctx,
>>>>    			   G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
>>>>    			   G1_REG_DEC_CTRL3);
>>>>    
>>>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>>>> +		vdpu_write_relaxed(vpu,
>>>> +				   G1_REG_DEC_CTRL3_STREAM_LEN_EXT
>>>> +					(dct_part_total_len >> 24),
>>>> +				   G1_REG_DEC_CTRL3);
>>>> +
>>>>    	/* DCT partitions base address */
>>>>    	for (i = 0; i < hdr->num_dct_parts; i++) {
>>>>    		u32 byte_offset = dct_part_offset + dct_size_part_size + count;
>>>> @@ -427,6 +433,12 @@ static void cfg_buffers(struct hantro_ctx *ctx,
>>>>    
>>>>    	dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
>>>>    	vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
>>>> +
>>>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>>>> +		vdpu_write_relaxed(vpu, dst_dma +
>>>> +				   ctx->dst_fmt.plane_fmt[0].bytesperline *
>>>> +				   ctx->dst_fmt.height,
>>>> +				   G1_REG_ADDR_DST_CHROMA);
>>>>    }
>>>>    
>>>>    int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>>>> @@ -471,6 +483,8 @@ int hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
>>>>    		reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
>>>>    	if (hdr->lf.level == 0)
>>>>    		reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
>>>> +	if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_WEBP_FRAME)
>>>> +		reg |= G1_REG_DEC_CTRL0_WEBP_E;
>>>>    	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
>>>>    
>>>>    	/* Frame dimensions */
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>>> index 2513adfbd825..7075b2ba1ec2 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>>>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>>> @@ -470,6 +470,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
>>>>    		break;
>>>>    	case V4L2_PIX_FMT_MPEG2_SLICE:
>>>>    	case V4L2_PIX_FMT_VP8_FRAME:
>>>> +	case V4L2_PIX_FMT_WEBP_FRAME:
>>>>    	case V4L2_PIX_FMT_H264_SLICE:
>>>>    	case V4L2_PIX_FMT_HEVC_SLICE:
>>>>    	case V4L2_PIX_FMT_VP9_FRAME:
>>>> @@ -492,6 +493,7 @@ hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
>>>>    	case V4L2_PIX_FMT_JPEG:
>>>>    	case V4L2_PIX_FMT_MPEG2_SLICE:
>>>>    	case V4L2_PIX_FMT_VP8_FRAME:
>>>> +	case V4L2_PIX_FMT_WEBP_FRAME:
>>>>    	case V4L2_PIX_FMT_HEVC_SLICE:
>>>>    	case V4L2_PIX_FMT_VP9_FRAME:
>>>>    		vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
>>>> diff --git a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>>>> index 833821120b20..48d6912c3bab 100644
>>>> --- a/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>>>> +++ b/drivers/media/platform/verisilicon/stm32mp25_vpu_hw.c
>>>> @@ -22,10 +22,10 @@ static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
>>>>    		.codec_mode = HANTRO_MODE_NONE,
>>>>    		.frmsize = {
>>>>    			.min_width = FMT_MIN_WIDTH,
>>>> -			.max_width = FMT_FHD_WIDTH,
>>>> +			.max_width = FMT_4K_WIDTH,
>>>>    			.step_width = MB_DIM,
>>>>    			.min_height = FMT_MIN_HEIGHT,
>>>> -			.max_height = FMT_FHD_HEIGHT,
>>>> +			.max_height = FMT_4K_HEIGHT,
>>>
>>> I'm a little surprised of this change, since this is modifying VP8_FRAME, while
>>> we should instead introduce WEBP_FRAME.
>>
>> This is the resolution of the YUV output of decoder, not the WebP input,
>> and because of lack of post-processor, the output is not scaled, so can
>> go up to 4K with WebP.
>> Before WebP introduction, the maximum output resolution was FHD for all
>> codecs. Now WebP allows up to 4K but FHD constraint remains for
>> H264/VP8. I don't see real problems because VP8/H264 compressed inputs
>> are well limited to FHD and only WebP allows 4K...
> 
> Good point. Would you mind adding a justification for this change within the
> commit message in v3 ?

v3 already sent but I'll note that for the future v4.
Could you test & ack the 4K support on your platform having 
post-processor support ?

> 
>>
>>>
>>>>    			.step_height = MB_DIM,
>>>>    		},
>>>>    	},
>>>> @@ -68,6 +68,19 @@ static const struct hantro_fmt stm32mp25_venc_fmts[] = {
>>>>    		.codec_mode = HANTRO_MODE_NONE,
>>>>    		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>>>>    	},
>>>> +	{
>>>> +		.fourcc = V4L2_PIX_FMT_WEBP_FRAME,
>>>> +		.codec_mode = HANTRO_MODE_VP8_DEC,
>>>> +		.max_depth = 2,
>>>> +		.frmsize = {
>>>> +			.min_width = FMT_MIN_WIDTH,
>>>> +			.max_width = FMT_4K_WIDTH,
>>>> +			.step_width = MB_DIM,
>>>> +			.min_height = FMT_MIN_HEIGHT,
>>>> +			.max_height = FMT_4K_HEIGHT,
>>>> +			.step_height = MB_DIM,
>>>> +		},
>>>> +	},
>>>
>>> This is venc_fmt (encoder), this shouldn't be there.
>>
>> All apologizes for this rebase issue, it is of course part of
>> stm32mp25_vdec_fmts.
> 
> Ack, let's get this right in v3 :-D
> 
>>
>>>
>>>>    	{
>>>>    		.fourcc = V4L2_PIX_FMT_YUYV,
>>>>    		.codec_mode = HANTRO_MODE_NONE,
>>>
>>
>> BR,
>> Hugues.
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] media: verisilicon: postproc: 4K support
  2024-11-20 11:01 ` [PATCH v2 3/3] media: verisilicon: postproc: 4K support Hugues Fruchet
@ 2024-12-12 14:26   ` Nicolas Dufresne
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Dufresne @ 2024-12-12 14:26 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
> Support input larger than 4096x2048 using extended input width/height
> fields of swreg92.
> This is needed to decode large WebP or JPEG pictures.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

This can likely be picked independently as it already improve JPEG support.

Nicolas

> ---
>  drivers/media/platform/verisilicon/hantro.h          | 2 ++
>  drivers/media/platform/verisilicon/hantro_g1_regs.h  | 2 +-
>  drivers/media/platform/verisilicon/hantro_postproc.c | 6 +++++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index 811260dc3c77..d1337f7742e4 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
> @@ -321,6 +321,8 @@ struct hantro_postproc_regs {
>  	struct hantro_reg output_fmt;
>  	struct hantro_reg orig_width;
>  	struct hantro_reg display_width;
> +	struct hantro_reg input_width_ext;
> +	struct hantro_reg input_height_ext;
>  };
>  
>  struct hantro_vp9_decoded_buffer_info {
> diff --git a/drivers/media/platform/verisilicon/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
> index e7d4db788e57..f6e5bbeb1914 100644
> --- a/drivers/media/platform/verisilicon/hantro_g1_regs.h
> +++ b/drivers/media/platform/verisilicon/hantro_g1_regs.h
> @@ -351,7 +351,7 @@
>  #define     G1_REG_PP_CONTROL_OUT_WIDTH(v) (((v) << 4) & GENMASK(14, 4))
>  #define G1_REG_PP_MASK1_ORIG_WIDTH	G1_SWREG(88)
>  #define     G1_REG_PP_ORIG_WIDTH(v)	(((v) << 23) & GENMASK(31, 23))
> -#define G1_REG_PP_DISPLAY_WIDTH		G1_SWREG(92)
> +#define G1_REG_PP_DISPLAY_WIDTH_IN_EXT	G1_SWREG(92)
>  #define G1_REG_PP_FUSE			G1_SWREG(99)
>  
>  #endif /* HANTRO_G1_REGS_H_ */
> diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
> index 232c93eea7ee..84c8e287470d 100644
> --- a/drivers/media/platform/verisilicon/hantro_postproc.c
> +++ b/drivers/media/platform/verisilicon/hantro_postproc.c
> @@ -49,7 +49,9 @@ static const struct hantro_postproc_regs hantro_g1_postproc_regs = {
>  	.input_fmt = {G1_REG_PP_CONTROL, 29, 0x7},
>  	.output_fmt = {G1_REG_PP_CONTROL, 26, 0x7},
>  	.orig_width = {G1_REG_PP_MASK1_ORIG_WIDTH, 23, 0x1ff},
> -	.display_width = {G1_REG_PP_DISPLAY_WIDTH, 0, 0xfff},
> +	.display_width = {G1_REG_PP_DISPLAY_WIDTH_IN_EXT, 0, 0xfff},
> +	.input_width_ext = {G1_REG_PP_DISPLAY_WIDTH_IN_EXT, 26, 0x7},
> +	.input_height_ext = {G1_REG_PP_DISPLAY_WIDTH_IN_EXT, 29, 0x7},
>  };
>  
>  bool hantro_needs_postproc(const struct hantro_ctx *ctx,
> @@ -103,6 +105,8 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
>  	HANTRO_PP_REG_WRITE(vpu, output_height, ctx->dst_fmt.height);
>  	HANTRO_PP_REG_WRITE(vpu, orig_width, MB_WIDTH(ctx->dst_fmt.width));
>  	HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
> +	HANTRO_PP_REG_WRITE(vpu, input_width_ext, MB_WIDTH(ctx->dst_fmt.width) >> 9);
> +	HANTRO_PP_REG_WRITE(vpu, input_height_ext, MB_HEIGHT(ctx->dst_fmt.height >> 8));
>  }
>  
>  static int down_scale_factor(struct hantro_ctx *ctx)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: uapi: add WebP uAPI
  2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
                     ` (2 preceding siblings ...)
  2024-11-20 15:43   ` Diederik de Haas
@ 2024-12-12 14:34   ` Nicolas Dufresne
  3 siblings, 0 replies; 16+ messages in thread
From: Nicolas Dufresne @ 2024-12-12 14:34 UTC (permalink / raw)
  To: Hugues Fruchet, Mauro Carvalho Chehab, Ezequiel Garcia,
	Philipp Zabel, Hans Verkuil, Fritz Koenig, Sebastian Fricke,
	Daniel Almeida, Andrzej Pietrasiewicz, Benjamin Gaignard,
	linux-media, linux-kernel, linux-rockchip, linux-stm32

Hi Hugues,

as you know, naming is hard and I had to think about this one a little, see my
comment below.

Le mercredi 20 novembre 2024 à 12:01 +0100, Hugues Fruchet a écrit :
> This patch adds the WebP picture decoding kernel uAPI.
> 
> This design is based on currently available VP8 API implementation and
> aims to support the development of WebP stateless video codecs
> on Linux.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> ---
>  Documentation/userspace-api/media/v4l/biblio.rst  |  9 +++++++++
>  .../userspace-api/media/v4l/pixfmt-compressed.rst | 15 +++++++++++++++
>  drivers/media/v4l2-core/v4l2-ioctl.c              |  1 +
>  include/uapi/linux/videodev2.h                    |  1 +
>  4 files changed, 26 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
> index 35674eeae20d..df3e963fc54f 100644
> --- a/Documentation/userspace-api/media/v4l/biblio.rst
> +++ b/Documentation/userspace-api/media/v4l/biblio.rst
> @@ -447,3 +447,12 @@ AV1
>  :title:     AV1 Bitstream & Decoding Process Specification
>  
>  :author:    Peter de Rivaz, Argon Design Ltd, Jack Haughton, Argon Design Ltd
> +
> +.. _webp:
> +
> +WEBP
> +====
> +
> +:title:     WEBP picture Bitstream & Decoding Process Specification
> +
> +:author:    Google (https://developers.google.com/speed/webp)
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> index 806ed73ac474..e664e70b0619 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-compressed.rst
> @@ -169,6 +169,21 @@ Compressed Formats
>  	this pixel format. The output buffer must contain the appropriate number
>  	of macroblocks to decode a full corresponding frame to the matching
>  	capture buffer.
> +    * .. _V4L2-PIX-FMT-WEBP-FRAME:
> +
> +      - ``V4L2_PIX_FMT_WEBP_FRAME``
> +      - 'WEBP'
> +      - WEBP VP8 parsed frame, excluding WEBP RIFF header, keeping only the VP8
> +	bistream including the frame header, as extracted from the container.
> +	This format is adapted for stateless video decoders that implement a
> +	WEBP pipeline with the :ref:`stateless_decoder`.
> +	Metadata associated with the frame to decode is required to be passed
> +	through the ``V4L2_CID_STATELESS_VP8_FRAME`` control.
> +	See the :ref:`associated Codec Control IDs <v4l2-codec-stateless-vp8>`.
> +	Exactly one output and one capture buffer must be provided for use with
> +	this pixel format. The output buffer must contain the appropriate number
> +	of macroblocks to decode a full corresponding frame to the matching
> +	capture buffer.

So after reading more about it, I think we should avoid the usage of the WEBP
name, as WEBP is clearly a RIFF based container format and supports 2 totally
different codecs. I think we should instead call this
V4L2_PIX_FMT_VP8_INTRA_FRAME. We should still document that these are used by
WebP for lossy compression, so that  readers can correlate.

Some folks will still wonder why a separate format, and the I still hold that it
fits better for the case we have one driver, with one video node that can handle
both, since you can probe these by simply using VIDIOC_ENUM_FMT, its easily
discoverable without introducing a new control, etc.

regards,
Nicolas

>  
>      * .. _V4L2-PIX-FMT-VP9:
>  
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 0304daa8471d..e2ff03d0d773 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1501,6 +1501,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  		case V4L2_PIX_FMT_VC1_ANNEX_L:	descr = "VC-1 (SMPTE 412M Annex L)"; break;
>  		case V4L2_PIX_FMT_VP8:		descr = "VP8"; break;
>  		case V4L2_PIX_FMT_VP8_FRAME:    descr = "VP8 Frame"; break;
> +		case V4L2_PIX_FMT_WEBP_FRAME:    descr = "WEBP VP8 Frame"; break;
>  		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
>  		case V4L2_PIX_FMT_VP9_FRAME:    descr = "VP9 Frame"; break;
>  		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index e7c4dce39007..09fff269e852 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -757,6 +757,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 421M Annex L compliant stream */
>  #define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
>  #define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frame */
> +#define V4L2_PIX_FMT_WEBP_FRAME v4l2_fourcc('W', 'B', 'P', 'F') /* WEBP VP8 parsed frame */
>  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
>  #define V4L2_PIX_FMT_VP9_FRAME v4l2_fourcc('V', 'P', '9', 'F') /* VP9 parsed frame */
>  #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-12-12 14:34 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-20 11:01 [PATCH v2 0/3] Add WebP support to hantro decoder Hugues Fruchet
2024-11-20 11:01 ` [PATCH v2 1/3] media: uapi: add WebP uAPI Hugues Fruchet
2024-11-20 13:42   ` Link Mauve
2024-11-20 13:57     ` Nicolas Dufresne
2024-11-20 14:21   ` Nicolas Dufresne
2024-11-21  9:59     ` Hugues FRUCHET
2024-11-20 15:43   ` Diederik de Haas
2024-11-21 10:08     ` Hugues FRUCHET
2024-12-12 14:34   ` Nicolas Dufresne
2024-11-20 11:01 ` [PATCH v2 2/3] media: verisilicon: add WebP decoding support Hugues Fruchet
2024-11-20 14:25   ` Nicolas Dufresne
2024-11-21 10:07     ` Hugues FRUCHET
2024-11-21 14:20       ` Nicolas Dufresne
2024-11-21 14:28         ` Hugues FRUCHET
2024-11-20 11:01 ` [PATCH v2 3/3] media: verisilicon: postproc: 4K support Hugues Fruchet
2024-12-12 14:26   ` Nicolas Dufresne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox