Re: [PATCH v4l-utils 2/2] calculate expected length using v4l2-fwht info

[Hans Verkuil <hverkuil+cisco@kernel.org>]

Hi Sarah,

On 16/03/2026 14:22, Sarah Gershuni wrote:
> Update get_expected_length_trace to calculate the expected buffer length
> based on v4l2_fwht_find_pixfmt info.
> 
> Signed-off-by: Sarah Gershuni <sarah556726@gmail.com>
> ---
>  utils/v4l2-tracer/trace-helper.cpp | 40 +++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/utils/v4l2-tracer/trace-helper.cpp b/utils/v4l2-tracer/trace-helper.cpp
> index 6c296dbf..a59d7761 100644
> --- a/utils/v4l2-tracer/trace-helper.cpp
> +++ b/utils/v4l2-tracer/trace-helper.cpp
> @@ -6,6 +6,11 @@
>  #include "trace.h"
>  #include <math.h>
>  
> +extern "C" {
> +#include "codec-v4l2-fwht.h"
> +}
> +
> +
>  struct trace_context ctx_trace = {};
>  
>  bool is_video_or_media_device(const char *path)
> @@ -233,18 +238,29 @@ void print_buffers_trace(void)
>  
>  unsigned get_expected_length_trace()
>  {
> -	/*
> -	 * TODO: this assumes that the stride is equal to the real width and that the
> -	 * padding follows the end of the chroma plane. It could be improved by
> -	 * following the model in v4l2-ctl-streaming.cpp read_write_padded_frame()
> -	 */
> -	unsigned expected_length = ctx_trace.width * ctx_trace.height;
> -	if (ctx_trace.pixelformat == V4L2_PIX_FMT_NV12 || ctx_trace.pixelformat == V4L2_PIX_FMT_YUV420) {
> -		expected_length *= 3;
> -		expected_length /= 2;
> -		expected_length += (expected_length % 2);
> -	}
> -	return expected_length;
> +	const auto *info = v4l2_fwht_find_pixfmt(ctx_trace.pixelformat);
> +    if (!info)
> +		return 0;
> +
> +    unsigned coded_height = ctx_trace.height;
> +    unsigned expected = 0;
> +
> +    for (unsigned plane_idx = 0; plane_idx < info->planes_num; ++plane_idx) {
> +        unsigned stride = ctx_trace.plane_bytesperline[plane_idx];

Ah, this does not work. All the pixelformats that v4l2_fwht_find_pixfmt knows are
all single buffer formats, i.e. all planes are combined in a single buffer.
So there is also just one bytesperline value.

I know, it's very confusing: the _MPLANE buffer types should really be called
_MBUFFER in hindsight. I.e. each plane has its own buffer as opposed to
concatenating all planes in a single buffer.

I'm not sure what problem you are attempting to fix, but I don't think this does
what you want it to do.

Although patch 1/2 looks fine, I'll drop both patches since it makes no sense
in combination with this patch.

It really only makes sense for multiplanar pixelformats like V4L2_PIX_FMT_NV12M,
V4L2_PIX_FMT_YUV420M, etc., but that's not what this second patch deals with.

Regards,

	Hans

> +        
> +        bool is_chroma = (plane_idx == 1 || plane_idx == 2);
> +        unsigned h_div = is_chroma ? info->height_div : 1;
> +
> +        if (info->planes_num == 3 && plane_idx == 1)
> +            stride /= 2;
> +
> +        if (plane_idx == 1 &&
> +            (info->id == V4L2_PIX_FMT_NV24 || info->id == V4L2_PIX_FMT_NV42))
> +            stride *= 2;
> +
> +        expected += stride * (coded_height / h_div);
> +    }
> +    return expected;
>  }
>  
>  void s_ext_ctrls_setup(struct v4l2_ext_controls *ext_controls)