[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@xxxxxxxxxxxxxxxx>
---
 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