From: Hans Verkuil <hverkuil+cisco@kernel.org>
To: Nicolas Dufresne <nicolas.dufresne@collabora.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
linux-media@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org,
linux-rockchip@lists.infradead.org,
linux-staging@lists.linux.dev,
linux-stm32@st-md-mailman.stormreply.com,
linux-sunxi@lists.linux.dev
Cc: "Andrzej Pietrasiewicz" <andrzejtp2010@gmail.com>,
"Bin Liu" <bin.liu@mediatek.com>,
"Ezequiel Garcia" <ezequiel@vanguardiasur.com.ar>,
"Hans Verkuil" <hverkuil@kernel.org>,
"Jacob Chen" <jacob-chen@iotwrt.com>,
"Jernej Skrabec" <jernej.skrabec@gmail.com>,
"Jiasheng Jiang" <jiashengjiangcool@gmail.com>,
"Matthew Majewski" <mattwmajewski@gmail.com>,
"Mirela Rabulea" <mirela.rabulea@nxp.com>,
"Neil Armstrong" <neil.armstrong@linaro.org>,
"Philipp Zabel" <p.zabel@pengutronix.de>,
"Shuah Khan" <skhan@linuxfoundation.org>,
"Steve Longerbeam" <slongerbeam@gmail.com>,
"Uwe Kleine-König" <u.kleine-koenig@baylibre.com>,
"Xavier Roumegue" <xavier.roumegue@oss.nxp.com>
Subject: Re: [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices
Date: Wed, 15 Oct 2025 10:02:37 +0200 [thread overview]
Message-ID: <a4cf7a4a-f833-487d-9861-a6957f25e7e0@kernel.org> (raw)
In-Reply-To: <45a3e39fb87fc4e32626dfaa277a314bce5f4f68.camel@collabora.com>
On 10/9/25 15:04, Nicolas Dufresne wrote:
> Hi,
>
> Le jeudi 09 octobre 2025 à 14:11 +0300, Laurent Pinchart a écrit :
>> The v4l2_m2m_buf_copy_metadata() function takes a boolean
>> copy_frame_flags argument. When true, it causes the function to copy the
>> V4L2_BUF_FLAG_KEYFRAME, V4L2_BUF_FLAG_BFRAME and V4L2_BUF_FLAG_PFRAME
>> flags from the output buffer to the capture buffer.
>>
>> Many frame-based M2M drivers (e.g. for JPEG encoders, scalers,
>> dewarpers, 2D blenders, ...) set the argument to true, while the frame
>> flags are not applicable to those drivers as they have no concept of
>> key, B or P frames. Set the argument to false to avoid further
>> cargo-cult mistakes.
>
> There is no use cases in any upstream driver for copying them over (even in
> Tegra 20 decoder driver). KEY/P/B are properties of bitstream buffer in some
> formats. Once decoded, this is no longer a property of the video frame and
> should be discarded. So even decoder supporting that feature should not copy
> them over.
I agree. I think I thought at the time that it could be useful to know if
an uncompressed frame was decoded from a I/P/B compressed frame, and to
preserve that information if that same uncompressed frame was passed through
another M2M device (e.g. a scaler).
But looking back at the documentation it is clear that these flags are meant
for compressed frames, so copying them makes no sense.
Regards,
Hans
>
> I'd remove the boolean completely to be fair, this is always inappropriate to
> set this to true. I'd be fine doing it in two steps, but then the list should be
> completed, see below:
>
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>> drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 4 ++--
>> drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c | 4 ++--
>> drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c | 4 ++--
>> drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c | 2 +-
>> drivers/media/platform/nxp/dw100/dw100.c | 2 +-
>> drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 2 +-
>> drivers/media/platform/rockchip/rga/rga.c | 2 +-
>> drivers/media/platform/st/stm32/dma2d/dma2d.c | 2 +-
>> drivers/media/platform/sunxi/sun8i-di/sun8i-di.c | 2 +-
>> drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c | 2 +-
>> drivers/media/test-drivers/vim2m.c | 2 +-
>> drivers/staging/media/imx/imx-media-csc-scaler.c | 2 +-
>
> Missing:
>
> - drivers/media/platform/amphion/vdec.c
> - drivers/media/platform/mediatek/vcodec/decoder/vdec/*
> - drivers/media/platform/nvidia/tegra-vde/h264.c
> - drivers/media/platform/rockchip/rkvdec/rkvdec.c
> - drivers/media/platform/verisilicon/hantro_drv.c
> - drivers/staging/media/sunxi/cedrus/cedrus_dec.c
>
> cheers,
> Nicolas
>
>> 12 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> index 35c70ec3ad2c..6bd5036430dc 100644
>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> @@ -1619,7 +1619,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
>> if (!dst_buf)
>> goto getbuf_fail;
>>
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>
>> mtk_jpegenc_set_hw_param(ctx, hw_id, src_buf, dst_buf);
>> ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
>> @@ -1715,7 +1715,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>> if (!dst_buf)
>> goto getbuf_fail;
>>
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>> jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
>> jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
>>
>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> index e78e1d11093c..556865100872 100644
>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
>> @@ -530,7 +530,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct
>> *work)
>>
>> src_buf = cjpeg->hw_param.src_buffer;
>> dst_buf = cjpeg->hw_param.dst_buffer;
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>
>> mtk_jpeg_dec_reset(cjpeg->reg_base);
>> clk_disable_unprepare(cjpeg->jdec_clk.clks->clk);
>> @@ -560,7 +560,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq,
>> void *priv)
>> ctx = jpeg->hw_param.curr_ctx;
>> src_buf = jpeg->hw_param.src_buffer;
>> dst_buf = jpeg->hw_param.dst_buffer;
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>
>> irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
>> dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> index 9ab27aee302a..4c8427b3c384 100644
>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
>> @@ -261,7 +261,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct
>> *work)
>>
>> src_buf = cjpeg->hw_param.src_buffer;
>> dst_buf = cjpeg->hw_param.dst_buffer;
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>
>> mtk_jpeg_enc_reset(cjpeg->reg_base);
>> clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
>> @@ -289,7 +289,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq,
>> void *priv)
>> ctx = jpeg->hw_param.curr_ctx;
>> src_buf = jpeg->hw_param.src_buffer;
>> dst_buf = jpeg->hw_param.dst_buffer;
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>
>> irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
>> JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
>> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> index 59ce5cce0698..dba46a69c6be 100644
>> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
>> @@ -51,7 +51,7 @@ static void mdp_m2m_process_done(void *priv, int vb_state)
>> ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC];
>> src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++;
>> dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++;
>> - v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true);
>> + v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, false);
>>
>> v4l2_m2m_buf_done(src_vbuf, vb_state);
>> v4l2_m2m_buf_done(dst_vbuf, vb_state);
>> diff --git a/drivers/media/platform/nxp/dw100/dw100.c
>> b/drivers/media/platform/nxp/dw100/dw100.c
>> index 7a0ee44d9e1f..b73302d54635 100644
>> --- a/drivers/media/platform/nxp/dw100/dw100.c
>> +++ b/drivers/media/platform/nxp/dw100/dw100.c
>> @@ -1483,7 +1483,7 @@ static void dw100_start(struct dw100_ctx *ctx, struct
>> vb2_v4l2_buffer *in_vb,
>> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE),
>> in_vb->sequence, out_vb->sequence);
>>
>> - v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
>> + v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>>
>> /* Now, let's deal with hardware ... */
>> dw100_hw_master_bus_disable(dw_dev);
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 37e0670f98c5..e1dda1e834e4 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -1537,7 +1537,7 @@ static void mxc_jpeg_device_run(void *priv)
>> src_buf->sequence = q_data_out->sequence++;
>> dst_buf->sequence = q_data_cap->sequence++;
>>
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>
>> jpeg_src_buf = vb2_to_mxc_buf(&src_buf->vb2_buf);
>> if (q_data_cap->fmt->mem_planes != dst_buf->vb2_buf.num_planes) {
>> diff --git a/drivers/media/platform/rockchip/rga/rga.c
>> b/drivers/media/platform/rockchip/rga/rga.c
>> index 48b88da59da0..075b684fb178 100644
>> --- a/drivers/media/platform/rockchip/rga/rga.c
>> +++ b/drivers/media/platform/rockchip/rga/rga.c
>> @@ -75,7 +75,7 @@ static irqreturn_t rga_isr(int irq, void *prv)
>> WARN_ON(!src);
>> WARN_ON(!dst);
>>
>> - v4l2_m2m_buf_copy_metadata(src, dst, true);
>> + v4l2_m2m_buf_copy_metadata(src, dst, false);
>>
>> dst->sequence = ctx->csequence++;
>>
>> diff --git a/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> b/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> index 643913adc1f3..4184bdb96e6d 100644
>> --- a/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> +++ b/drivers/media/platform/st/stm32/dma2d/dma2d.c
>> @@ -483,7 +483,7 @@ static void device_run(void *prv)
>>
>> src->sequence = frm_out->sequence++;
>> dst->sequence = frm_cap->sequence++;
>> - v4l2_m2m_buf_copy_metadata(src, dst, true);
>> + v4l2_m2m_buf_copy_metadata(src, dst, false);
>>
>> if (clk_enable(dev->gate))
>> goto end;
>> diff --git a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> index 3e7f2df70408..11a6c7f5212e 100644
>> --- a/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> +++ b/drivers/media/platform/sunxi/sun8i-di/sun8i-di.c
>> @@ -71,7 +71,7 @@ static void deinterlace_device_run(void *priv)
>> src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>> dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>>
>> - v4l2_m2m_buf_copy_metadata(src, dst, true);
>> + v4l2_m2m_buf_copy_metadata(src, dst, false);
>>
>> deinterlace_write(dev, DEINTERLACE_MOD_ENABLE,
>> DEINTERLACE_MOD_ENABLE_EN);
>> diff --git a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> index abd10b218aa1..f6e2f11a20dd 100644
>> --- a/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> +++ b/drivers/media/platform/sunxi/sun8i-rotate/sun8i_rotate.c
>> @@ -70,7 +70,7 @@ static void rotate_device_run(void *priv)
>> src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>> dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>>
>> - v4l2_m2m_buf_copy_metadata(src, dst, true);
>> + v4l2_m2m_buf_copy_metadata(src, dst, false);
>>
>> val = ROTATE_GLB_CTL_MODE(ROTATE_MODE_COPY_ROTATE);
>> if (ctx->hflip)
>> diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-
>> drivers/vim2m.c
>> index dc82830a35a5..3e8476165721 100644
>> --- a/drivers/media/test-drivers/vim2m.c
>> +++ b/drivers/media/test-drivers/vim2m.c
>> @@ -482,7 +482,7 @@ static int device_process(struct vim2m_ctx *ctx,
>>
>> out_vb->sequence = q_data_out->sequence++;
>> in_vb->sequence = q_data_in->sequence++;
>> - v4l2_m2m_buf_copy_metadata(in_vb, out_vb, true);
>> + v4l2_m2m_buf_copy_metadata(in_vb, out_vb, false);
>>
>> if (ctx->mode & MEM2MEM_VFLIP) {
>> start = height - 1;
>> diff --git a/drivers/staging/media/imx/imx-media-csc-scaler.c
>> b/drivers/staging/media/imx/imx-media-csc-scaler.c
>> index 19fd31cb9bb0..770ba3fbaba2 100644
>> --- a/drivers/staging/media/imx/imx-media-csc-scaler.c
>> +++ b/drivers/staging/media/imx/imx-media-csc-scaler.c
>> @@ -96,7 +96,7 @@ static void ipu_ic_pp_complete(struct ipu_image_convert_run
>> *run, void *_ctx)
>> src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>> dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>>
>> - v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>> + v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
>>
>> src_buf->sequence = ctx->sequence++;
>> dst_buf->sequence = src_buf->sequence;
>>
>> base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
next prev parent reply other threads:[~2025-10-15 8:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-09 11:11 [PATCH] media: v4l2-mem2mem: Don't copy frame flags for frame-based devices Laurent Pinchart
2025-10-09 13:04 ` Nicolas Dufresne
2025-10-15 8:02 ` Hans Verkuil [this message]
2025-10-09 14:40 ` Frank Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a4cf7a4a-f833-487d-9861-a6957f25e7e0@kernel.org \
--to=hverkuil+cisco@kernel.org \
--cc=andrzejtp2010@gmail.com \
--cc=bin.liu@mediatek.com \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=hverkuil@kernel.org \
--cc=imx@lists.linux.dev \
--cc=jacob-chen@iotwrt.com \
--cc=jernej.skrabec@gmail.com \
--cc=jiashengjiangcool@gmail.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-staging@lists.linux.dev \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux-sunxi@lists.linux.dev \
--cc=mattwmajewski@gmail.com \
--cc=mirela.rabulea@nxp.com \
--cc=neil.armstrong@linaro.org \
--cc=nicolas.dufresne@collabora.com \
--cc=p.zabel@pengutronix.de \
--cc=skhan@linuxfoundation.org \
--cc=slongerbeam@gmail.com \
--cc=u.kleine-koenig@baylibre.com \
--cc=xavier.roumegue@oss.nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).