[PATCH 0/3] media: venus: fix several caps parsing bugs

[PATCH 0/3] media: venus: fix several caps parsing bugs

[Dmitry Baryshkov]

Testing Venus on MSM8996 revealed several issues with caps parsing,
which didn't manifest on other generations of Venus hardware / firmware.
Fix them to make Venus decode streams properly on MSM8996-era of the
hardware.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (3):
      media: venus: hfi_parser: account for all capabilities when skipping a property
      media: venus: hfi_parser: size a raw format property by its own entries
      media: venus: report the frame size of the codec being enumerated

 drivers/media/platform/qcom/venus/helpers.c    | 75 +++++++++++++++++---------
 drivers/media/platform/qcom/venus/helpers.h    |  2 +
 drivers/media/platform/qcom/venus/hfi_parser.c |  9 ++--
 drivers/media/platform/qcom/venus/hfi_parser.h | 12 +++--
 drivers/media/platform/qcom/venus/vdec.c       |  7 +--
 drivers/media/platform/qcom/venus/venc.c       |  7 +--
 6 files changed, 66 insertions(+), 46 deletions(-)
---
base-commit: 78bc8af4affb9a732504eb22eeac7d1e50883853
change-id: 20260812-venus-8996-1eb92496a45d

Best regards,
--  
With best wishes
Dmitry

[PATCH 1/3] media: venus: hfi_parser: account for all capabilities when skipping a property

[Dmitry Baryshkov]

hfi_parser() walks the property list of the message it is given by
advancing over each property by the length its handler returns.
parse_caps() returns a fixed sizeof(*caps), which only covers the
num_capabilities field and, at the time the code was written, the single
struct hfi_capability that struct hfi_capabilities then declared.  A
property carrying N capabilities is therefore under-skipped by N-1 entries,
and struct hfi_capabilities has since become a flexible array member, so
today the whole capability array is left behind.

The parser recovers from this because unrecognized words are skipped one at
a time and capability types, limits and step sizes do not collide with the
HFI property identifiers, but nothing guarantees that: any capability value
that happens to equal a property ID is parsed as a property, at an offset
that is not a property boundary.

Return the length the payload actually has.  Like the other handlers this
leaves the property identifier itself unaccounted for, so the walk resumes
on the last word of the payload rather than on the next property, and
relies on that word being skipped as unrecognized on the following
iteration.

Fixes: 09c2845e8fe4 ("[media] media: venus: hfi: add Host Firmware Interface (HFI)")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/media/platform/qcom/venus/hfi_parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c
index b1657443f23f..3413b91b0b7e 100644
--- a/drivers/media/platform/qcom/venus/hfi_parser.c
+++ b/drivers/media/platform/qcom/venus/hfi_parser.c
@@ -146,7 +146,7 @@ parse_caps(struct venus_core *core, u32 codecs, u32 domain, void *data)
 	for_each_codec(core->caps, ARRAY_SIZE(core->caps), codecs, domain,
 		       fill_caps, caps_arr, num_caps);
 
-	return sizeof(*caps);
+	return struct_size(caps, data, num_caps);
 }
 
 static void fill_raw_fmts(struct hfi_plat_caps *cap, const void *fmts,

-- 
2.47.3

Re: [PATCH 1/3] media: venus: hfi_parser: account for all capabilities when skipping a property

[Konrad Dybcio]

On 8/12/26 10:01 PM, Dmitry Baryshkov wrote:
> hfi_parser() walks the property list of the message it is given by
> advancing over each property by the length its handler returns.
> parse_caps() returns a fixed sizeof(*caps), which only covers the
> num_capabilities field and, at the time the code was written, the single
> struct hfi_capability that struct hfi_capabilities then declared.  A
> property carrying N capabilities is therefore under-skipped by N-1 entries,
> and struct hfi_capabilities has since become a flexible array member, so
> today the whole capability array is left behind.
> 
> The parser recovers from this because unrecognized words are skipped one at
> a time and capability types, limits and step sizes do not collide with the
> HFI property identifiers, but nothing guarantees that: any capability value
> that happens to equal a property ID is parsed as a property, at an offset
> that is not a property boundary.
> 
> Return the length the payload actually has.  Like the other handlers this
> leaves the property identifier itself unaccounted for, so the walk resumes
> on the last word of the payload rather than on the next property, and
> relies on that word being skipped as unrecognized on the following
> iteration.
> 
> Fixes: 09c2845e8fe4 ("[media] media: venus: hfi: add Host Firmware Interface (HFI)")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/media/platform/qcom/venus/hfi_parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c
> index b1657443f23f..3413b91b0b7e 100644
> --- a/drivers/media/platform/qcom/venus/hfi_parser.c
> +++ b/drivers/media/platform/qcom/venus/hfi_parser.c
> @@ -146,7 +146,7 @@ parse_caps(struct venus_core *core, u32 codecs, u32 domain, void *data)
>  	for_each_codec(core->caps, ARRAY_SIZE(core->caps), codecs, domain,
>  		       fill_caps, caps_arr, num_caps);
>  
> -	return sizeof(*caps);
> +	return struct_size(caps, data, num_caps);

This is already solved in:

a51cea23e409 ("media: venus: fix payload size returned by parse_caps() and parse_alloc_mode()")

using struct_size instead of the open-coded version would still
be welcome though

Konrad

[PATCH 2/3] media: venus: hfi_parser: size a raw format property by its own entries

[Dmitry Baryshkov]

parse_raw_formats() walks the format entries one at a time, each of
which is as long as its own plane count makes it, but then computes the
length of the whole property as if every entry had as many planes as the
last one it looked at.  The result is only correct when all the entries
agree, and there is nothing in the interface that says they must.

On MSM8996 they do not, and the parser walks off into the middle of the
message as a result.  The raw format property of the HEVC decoder is 120
bytes long and the walk claims 152, so hfi_parser() resumes 28 bytes
past the end of it, skipping the codec mask that follows and landing
inside the property after that.  It resynchronises eventually, because
an unrecognized word is skipped one at a time, but everything it jumped
over is lost.

What is lost matters: the property skipped that way is the capability
set of the codec mask 0x7002, which is to say the frame size, macroblock
and frame rate limits of H.264, VP8, VP9 and HEVC decoding, all four of
them described in one block.  Those decoders end up holding a bitrate
and nothing else, so frame_width_min() and friends return zero and
VIDIOC_ENUM_FRAMESIZES advertises a stepwise range of 0x0 to 0x0 with a
step of 0.

Userspace cannot negotiate against that.  GStreamer builds the sink caps
of its V4L2 decoders from the enumerated frame sizes, an empty integer
range collapses them to EMPTY, and no pad is found to be compatible with
the parser feeding the decoder, so a pipeline as simple as

  filesrc ! parsebin ! v4l2vp9dec ! videoconvert ! fakesink

fails to link and the stream stops with "not-linked" before a single
buffer is queued.  Hardware decoding is unavailable on the board for
every codec in that block.

An earlier overshoot loses a raw format property too, leaving the
decoders with two of the five formats the firmware describes and HEVC
with four of seven.

Accumulate the length of the entries as they are walked, the way the
downstream driver does, rather than extrapolating from the last one.

Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/media/platform/qcom/venus/hfi_parser.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c
index 3413b91b0b7e..f79c368647d7 100644
--- a/drivers/media/platform/qcom/venus/hfi_parser.c
+++ b/drivers/media/platform/qcom/venus/hfi_parser.c
@@ -171,7 +171,7 @@ parse_raw_formats(struct venus_core *core, u32 codecs, u32 domain, void *data)
 	u32 entries = fmt->format_entries;
 	unsigned int i = 0;
 	u32 num_planes = 0;
-	u32 size;
+	u32 size = 2 * sizeof(u32);
 
 	while (entries) {
 		num_planes = pinfo->num_planes;
@@ -183,9 +183,10 @@ parse_raw_formats(struct venus_core *core, u32 codecs, u32 domain, void *data)
 		if (i >= MAX_FMT_ENTRIES)
 			return -EINVAL;
 
-		if (pinfo->num_planes > MAX_PLANES)
+		if (num_planes > MAX_PLANES)
 			break;
 
+		size += sizeof(*constr) * num_planes + 2 * sizeof(u32);
 		pinfo = (void *)pinfo + sizeof(*constr) * num_planes +
 			2 * sizeof(u32);
 		entries--;
@@ -193,8 +194,6 @@ parse_raw_formats(struct venus_core *core, u32 codecs, u32 domain, void *data)
 
 	for_each_codec(core->caps, ARRAY_SIZE(core->caps), codecs, domain,
 		       fill_raw_fmts, rawfmts, i);
-	size = fmt->format_entries * (sizeof(*constr) * num_planes + 2 * sizeof(u32))
-		+ 2 * sizeof(u32);
 
 	return size;
 }

-- 
2.47.3

Re: [PATCH 2/3] media: venus: hfi_parser: size a raw format property by its own entries

[Konrad Dybcio]

On 8/12/26 10:01 PM, Dmitry Baryshkov wrote:
> parse_raw_formats() walks the format entries one at a time, each of
> which is as long as its own plane count makes it, but then computes the
> length of the whole property as if every entry had as many planes as the
> last one it looked at.  The result is only correct when all the entries
> agree, and there is nothing in the interface that says they must.
> 
> On MSM8996 they do not, and the parser walks off into the middle of the
> message as a result.  The raw format property of the HEVC decoder is 120
> bytes long and the walk claims 152, so hfi_parser() resumes 28 bytes
> past the end of it, skipping the codec mask that follows and landing
> inside the property after that.  It resynchronises eventually, because
> an unrecognized word is skipped one at a time, but everything it jumped
> over is lost.
> 
> What is lost matters: the property skipped that way is the capability
> set of the codec mask 0x7002, which is to say the frame size, macroblock
> and frame rate limits of H.264, VP8, VP9 and HEVC decoding, all four of
> them described in one block.  Those decoders end up holding a bitrate
> and nothing else, so frame_width_min() and friends return zero and
> VIDIOC_ENUM_FRAMESIZES advertises a stepwise range of 0x0 to 0x0 with a
> step of 0.
> 
> Userspace cannot negotiate against that.  GStreamer builds the sink caps
> of its V4L2 decoders from the enumerated frame sizes, an empty integer
> range collapses them to EMPTY, and no pad is found to be compatible with
> the parser feeding the decoder, so a pipeline as simple as
> 
>   filesrc ! parsebin ! v4l2vp9dec ! videoconvert ! fakesink
> 
> fails to link and the stream stops with "not-linked" before a single
> buffer is queued.  Hardware decoding is unavailable on the board for
> every codec in that block.
> 
> An earlier overshoot loses a raw format property too, leaving the
> decoders with two of the five formats the firmware describes and HEVC
> with four of seven.
> 
> Accumulate the length of the entries as they are walked, the way the
> downstream driver does, rather than extrapolating from the last one.
> 
> Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/media/platform/qcom/venus/hfi_parser.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c
> index 3413b91b0b7e..f79c368647d7 100644
> --- a/drivers/media/platform/qcom/venus/hfi_parser.c
> +++ b/drivers/media/platform/qcom/venus/hfi_parser.c
> @@ -171,7 +171,7 @@ parse_raw_formats(struct venus_core *core, u32 codecs, u32 domain, void *data)
>  	u32 entries = fmt->format_entries;
>  	unsigned int i = 0;
>  	u32 num_planes = 0;
> -	u32 size;
> +	u32 size = 2 * sizeof(u32);
>  
>  	while (entries) {
>  		num_planes = pinfo->num_planes;
> @@ -183,9 +183,10 @@ parse_raw_formats(struct venus_core *core, u32 codecs, u32 domain, void *data)
>  		if (i >= MAX_FMT_ENTRIES)
>  			return -EINVAL;
>  
> -		if (pinfo->num_planes > MAX_PLANES)
> +		if (num_planes > MAX_PLANES)

Most of this patch (minus the line above) is in next as:

bd595b745eb7 ("media: venus: fix payload size calculation in parse_raw_formats()")

Konrad

[PATCH 3/3] media: venus: report the frame size of the codec being enumerated

[Dmitry Baryshkov]

VIDIOC_ENUM_FRAMESIZES takes the pixel format to describe from
userspace, but both vdec and venc answer it out of frame_width_min() and
friends, which resolve the capabilities through inst->hfi_codec, the
codec the instance is currently set to.  Enumerating any format other
than that one therefore returns the limits of an unrelated codec: on a
freshly opened decoder, which starts out as H.264, asking about MPEG-2
on MSM8996 reports the H.264 limits where the firmware describes MPEG-2
as 16x16 to 1920x1920.

Resolve the capabilities through the format being enumerated instead,
falling back to the codec in use for the raw formats, which have no
codec of their own.  Both drivers now share one helper, alongside the
pixel format to codec mapping that venus_helper_check_codec() already
carried.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/media/platform/qcom/venus/helpers.c    | 75 +++++++++++++++++---------
 drivers/media/platform/qcom/venus/helpers.h    |  2 +
 drivers/media/platform/qcom/venus/hfi_parser.h | 12 +++--
 drivers/media/platform/qcom/venus/vdec.c       |  7 +--
 drivers/media/platform/qcom/venus/venc.c       |  7 +--
 5 files changed, 62 insertions(+), 41 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 59eee3dd9e06..17415d0cdf8e 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -38,47 +38,43 @@ struct intbuf {
 	u32 dpb_out_tag;
 };
 
-bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt)
+static u32 venus_pixfmt_to_hfi_codec(u32 v4l2_pixfmt)
 {
-	struct venus_core *core = inst->core;
-	u32 session_type = inst->session_type;
-	u32 codec;
-
 	switch (v4l2_pixfmt) {
 	case V4L2_PIX_FMT_H264:
-		codec = HFI_VIDEO_CODEC_H264;
-		break;
+		return HFI_VIDEO_CODEC_H264;
 	case V4L2_PIX_FMT_H263:
-		codec = HFI_VIDEO_CODEC_H263;
-		break;
+		return HFI_VIDEO_CODEC_H263;
 	case V4L2_PIX_FMT_MPEG1:
-		codec = HFI_VIDEO_CODEC_MPEG1;
-		break;
+		return HFI_VIDEO_CODEC_MPEG1;
 	case V4L2_PIX_FMT_MPEG2:
-		codec = HFI_VIDEO_CODEC_MPEG2;
-		break;
+		return HFI_VIDEO_CODEC_MPEG2;
 	case V4L2_PIX_FMT_MPEG4:
-		codec = HFI_VIDEO_CODEC_MPEG4;
-		break;
+		return HFI_VIDEO_CODEC_MPEG4;
 	case V4L2_PIX_FMT_VC1_ANNEX_G:
 	case V4L2_PIX_FMT_VC1_ANNEX_L:
-		codec = HFI_VIDEO_CODEC_VC1;
-		break;
+		return HFI_VIDEO_CODEC_VC1;
 	case V4L2_PIX_FMT_VP8:
-		codec = HFI_VIDEO_CODEC_VP8;
-		break;
+		return HFI_VIDEO_CODEC_VP8;
 	case V4L2_PIX_FMT_VP9:
-		codec = HFI_VIDEO_CODEC_VP9;
-		break;
+		return HFI_VIDEO_CODEC_VP9;
 	case V4L2_PIX_FMT_XVID:
-		codec = HFI_VIDEO_CODEC_DIVX;
-		break;
+		return HFI_VIDEO_CODEC_DIVX;
 	case V4L2_PIX_FMT_HEVC:
-		codec = HFI_VIDEO_CODEC_HEVC;
-		break;
+		return HFI_VIDEO_CODEC_HEVC;
 	default:
-		return false;
+		return 0;
 	}
+}
+
+bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt)
+{
+	struct venus_core *core = inst->core;
+	u32 session_type = inst->session_type;
+	u32 codec = venus_pixfmt_to_hfi_codec(v4l2_pixfmt);
+
+	if (!codec)
+		return false;
 
 	if (session_type == VIDC_SESSION_TYPE_ENC && core->enc_codecs & codec)
 		return true;
@@ -90,6 +86,33 @@ bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt)
 }
 EXPORT_SYMBOL_GPL(venus_helper_check_codec);
 
+/*
+ * Report the frame size of the codec being enumerated, which is not
+ * necessarily the one the instance is currently set to.  A raw format maps to
+ * no codec of its own and is described by the coded format in use.
+ */
+void venus_helper_get_frame_sizes(struct venus_inst *inst, u32 v4l2_pixfmt,
+				  struct v4l2_frmsize_stepwise *fsize)
+{
+	struct venus_core *core = inst->core;
+	u32 dom = inst->session_type;
+	u32 codec = venus_pixfmt_to_hfi_codec(v4l2_pixfmt) ?: inst->hfi_codec;
+
+	fsize->min_width = get_codec_cap(core, codec, dom,
+					 HFI_CAPABILITY_FRAME_WIDTH, WHICH_CAP_MIN);
+	fsize->max_width = get_codec_cap(core, codec, dom,
+					 HFI_CAPABILITY_FRAME_WIDTH, WHICH_CAP_MAX);
+	fsize->step_width = get_codec_cap(core, codec, dom,
+					  HFI_CAPABILITY_FRAME_WIDTH, WHICH_CAP_STEP);
+	fsize->min_height = get_codec_cap(core, codec, dom,
+					  HFI_CAPABILITY_FRAME_HEIGHT, WHICH_CAP_MIN);
+	fsize->max_height = get_codec_cap(core, codec, dom,
+					  HFI_CAPABILITY_FRAME_HEIGHT, WHICH_CAP_MAX);
+	fsize->step_height = get_codec_cap(core, codec, dom,
+					   HFI_CAPABILITY_FRAME_HEIGHT, WHICH_CAP_STEP);
+}
+EXPORT_SYMBOL_GPL(venus_helper_get_frame_sizes);
+
 static void free_dpb_buf(struct venus_inst *inst, struct intbuf *buf)
 {
 	ida_free(&inst->dpb_ids, buf->dpb_out_tag);
diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
index 358e4f39c9c0..80d623db106c 100644
--- a/drivers/media/platform/qcom/venus/helpers.h
+++ b/drivers/media/platform/qcom/venus/helpers.h
@@ -12,6 +12,8 @@ struct venus_inst;
 struct venus_core;
 
 bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt);
+void venus_helper_get_frame_sizes(struct venus_inst *inst, u32 v4l2_pixfmt,
+				  struct v4l2_frmsize_stepwise *fsize);
 struct vb2_v4l2_buffer *venus_helper_find_buf(struct venus_inst *inst,
 					      unsigned int type, u32 idx);
 void venus_helper_change_dpb_owner(struct venus_inst *inst,
diff --git a/drivers/media/platform/qcom/venus/hfi_parser.h b/drivers/media/platform/qcom/venus/hfi_parser.h
index 5751d0140700..bcd11e7a9c1c 100644
--- a/drivers/media/platform/qcom/venus/hfi_parser.h
+++ b/drivers/media/platform/qcom/venus/hfi_parser.h
@@ -12,14 +12,14 @@ u32 hfi_parser(struct venus_core *core, struct venus_inst *inst,
 #define WHICH_CAP_MAX	1
 #define WHICH_CAP_STEP	2
 
-static inline u32 get_cap(struct venus_inst *inst, u32 type, u32 which)
+static inline u32 get_codec_cap(struct venus_core *core, u32 codec, u32 domain,
+				u32 type, u32 which)
 {
-	struct venus_core *core = inst->core;
 	struct hfi_capability *cap = NULL;
 	struct hfi_plat_caps *caps;
 	unsigned int i;
 
-	caps = venus_caps_by_codec(core, inst->hfi_codec, inst->session_type);
+	caps = venus_caps_by_codec(core, codec, domain);
 	if (!caps)
 		return 0;
 
@@ -47,6 +47,12 @@ static inline u32 get_cap(struct venus_inst *inst, u32 type, u32 which)
 	return 0;
 }
 
+static inline u32 get_cap(struct venus_inst *inst, u32 type, u32 which)
+{
+	return get_codec_cap(inst->core, inst->hfi_codec, inst->session_type,
+			     type, which);
+}
+
 static inline u32 cap_min(struct venus_inst *inst, u32 type)
 {
 	return get_cap(inst, type, WHICH_CAP_MIN);
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 6a43ea191da1..0f62931eea4c 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -512,12 +512,7 @@ static int vdec_enum_framesizes(struct file *file, void *fh,
 
 	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
 
-	fsize->stepwise.min_width = frame_width_min(inst);
-	fsize->stepwise.max_width = frame_width_max(inst);
-	fsize->stepwise.step_width = frame_width_step(inst);
-	fsize->stepwise.min_height = frame_height_min(inst);
-	fsize->stepwise.max_height = frame_height_max(inst);
-	fsize->stepwise.step_height = frame_height_step(inst);
+	venus_helper_get_frame_sizes(inst, fsize->pixel_format, &fsize->stepwise);
 
 	return 0;
 }
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 79acf7c1ec9a..e0f7b9817ccf 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -456,12 +456,7 @@ static int venc_enum_framesizes(struct file *file, void *fh,
 	if (fsize->index)
 		return -EINVAL;
 
-	fsize->stepwise.min_width = frame_width_min(inst);
-	fsize->stepwise.max_width = frame_width_max(inst);
-	fsize->stepwise.step_width = frame_width_step(inst);
-	fsize->stepwise.min_height = frame_height_min(inst);
-	fsize->stepwise.max_height = frame_height_max(inst);
-	fsize->stepwise.step_height = frame_height_step(inst);
+	venus_helper_get_frame_sizes(inst, fsize->pixel_format, &fsize->stepwise);
 
 	return 0;
 }

-- 
2.47.3

Re: [PATCH 3/3] media: venus: report the frame size of the codec being enumerated

[Konrad Dybcio]

On 8/12/26 10:01 PM, Dmitry Baryshkov wrote:
> VIDIOC_ENUM_FRAMESIZES takes the pixel format to describe from
> userspace, but both vdec and venc answer it out of frame_width_min() and
> friends, which resolve the capabilities through inst->hfi_codec, the
> codec the instance is currently set to.  Enumerating any format other
> than that one therefore returns the limits of an unrelated codec: on a
> freshly opened decoder, which starts out as H.264, asking about MPEG-2
> on MSM8996 reports the H.264 limits where the firmware describes MPEG-2
> as 16x16 to 1920x1920.

You gave some numbers for MPEG-2, saying H.264 has some different numbers.
I think it would be slightly easier to parse if you either skipped them or
gave numbers for both codecs

> Resolve the capabilities through the format being enumerated instead,
> falling back to the codec in use for the raw formats, which have no
> codec of their own.  Both drivers now share one helper, alongside the
> pixel format to codec mapping that venus_helper_check_codec() already
> carried.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

[...]

> -bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt)
> +static u32 venus_pixfmt_to_hfi_codec(u32 v4l2_pixfmt)
>  {
> -	struct venus_core *core = inst->core;
> -	u32 session_type = inst->session_type;
> -	u32 codec;
> -
>  	switch (v4l2_pixfmt) {
>  	case V4L2_PIX_FMT_H264:
> -		codec = HFI_VIDEO_CODEC_H264;
> -		break;
> +		return HFI_VIDEO_CODEC_H264;

We already have this - to_codec_type() in hfi.c

Konrad