[BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur

[Christian Cortner <ccortner@gmail.com>]

Hi,

On an ASUS Zenbook A16 (UX3607OA, Snapdragon X2 Elite / glymur),
ffmpeg's h264_v4l2m2m decoder outputs one corrupted frame and stops,
on every stream I tried. GStreamer's V4L2 decoders work. Below are the
two driver behaviours behind it (1 and 2), two more that stop ffmpeg
at a resolution change in the middle of a stream (3 and 4), and an
inconsistent CAPTURE format that ffmpeg happens to avoid (5).

I'm not a kernel developer. I tracked this down with an AI coding
assistant (Claude, by Anthropic): it traced the driver, wrote a small
V4L2 test client and tried changes to the driver. Everything below was
run on this laptop, and each local change described made its problem
go away here. I can't review or defend kernel patches myself, so I'm
reporting the problems instead of sending fixes, but I'm glad to test
patches on this machine.

Setup: the linux-msm laptops tree at 65f4ae7320b8 (v7.3-rc7), which
has the glymur iris support, plus the iris patches needed to run it
here: patch 06/13 of "media: iris: Migrate iommus to iris sub nodes"
v5, a driver-only form of the restrict-region IOVA fix, the PAS ID fix
and the runtime PM reference fix from the list, and radxa's libva-v4l2
kernel patches. Firmware: video-firmware 3.6 (aeb98f00ce9f) from the
Windows driver package. ffmpeg 8.1.3, GStreamer 1.28.8. The functions
involved are the same in media-committers next.

I could only test glymur. Birk Skyum's h264_v4l2m2m decodes on
X1E80100 matched a software decode [1], so problem 1 probably depends
on timing. Problems 2-4 are in code that runs at every resolution
change (2 and 4 on HFI gen2), so I expect them on other platforms too,
but I have not seen that.


1. A full resolution change at the start, although nothing changed

After STREAMON(OUTPUT), the firmware reports the first source change
about 2 ms later. ffmpeg has set up and started CAPTURE before that,
with a format that matches the stream. iris_inst_sub_state_change_drc()
then takes the IRIS_INST_SUB_DRC path rather than
IRIS_INST_SUB_FIRST_IPSC, so the client gets a full resolution change:
an empty LAST buffer, then no decoding until V4L2_DEC_CMD_START or a
CAPTURE restart. ffmpeg sends V4L2_DEC_CMD_START as soon as it sees the
event, gets -EBUSY because the drain is still running, as
dev-decoder.rst allows, and never sends it again after the LAST
buffer, which is a bug in ffmpeg. A test client that starts CAPTURE
early in the same way decodes all 60 frames when it sends START after
the LAST buffer, and none when it behaves like ffmpeg.

The question is whether iris should run a resolution change here at
all. The note in step 3 of the Initialization sequence says: "If the
values configured by the client do not match those parsed by the
decoder, a Dynamic Resolution Change will be triggered to reconfigure
them." vicodec raises no source change when the stream matches the
CAPTURE format. Keeping the event, but resuming the firmware in the
driver once its drain completes and giving the empty PSC_LAST buffer
back to the firmware, when this is the first source change and the
parsed format fits the CAPTURE queue, made ffmpeg decode bit-exact
here.


2. The empty LAST buffer of a source change is flagged as an error

iris_hfi_gen2_handle_output_buffer() sets V4L2_BUF_FLAG_ERROR on an
empty capture buffer unless the firmware flagged it
HFI_BUF_FW_FLAG_LAST. The empty buffer that ends a resolution change
carries HFI_BUF_FW_FLAG_PSC_LAST instead and reaches the client as
LAST|ERROR, although dev-decoder.rst says such a buffer "must be
ignored by the client". ffmpeg prints "capture: driver decode error"
for it and outputs it as a corrupted frame; that is the one frame in
problem 1. Exempting PSC_LAST the way 7adc11e6abf6 exempts LAST fixed
it here.


3. V4L2_EVENT_EOS at every resolution change

iris_vb2_buffer_done() and iris_vb2_buf_queue() queue V4L2_EVENT_EOS
and call v4l2_m2m_mark_stopped() for every LAST capture buffer,
including the one that ends a resolution change, and ffmpeg stops
decoding when it gets the event. Nothing in iris clears the stopped
mark again, so the EOS of the real drain at the end of the stream is
never sent. With a stream that changes from 640x360 to 1280x720 after
30 frames, the test client, following the spec, got an EOS at the
change and none at the end in 6 of 6 runs. Sending the event, and
marking the context stopped, only when IRIS_INST_SUB_DRAIN_LAST is set
fixed it here.


4. Stopping CAPTURE during a resolution change's drain kills the
   instance

ffmpeg handles a resolution change by stopping CAPTURE about 10 us
after it dequeues the event, which dev-decoder.rst allows as a way to
abort the drain. When the stop reaches the firmware before it has
returned the PSC_LAST buffer, IRIS_INST_SUB_DRC_LAST is never set. At
the next STREAMON(CAPTURE), iris_process_streamon_output() resumes the
firmware without reallocating the internal input buffers. 1.3 ms
later the firmware returns the PSC_LAST buffer it still owed,
iris_inst_sub_state_change_drc_last() rejects it because the input is
no longer paused, and the instance goes to IRIS_INST_ERROR. Nothing is
returned after that, and the client waits in poll() forever. It is a
race: the test client imitating ffmpeg stalled in 2 of 4, 0 of 6 and
1 of 6 runs in three series, and ffmpeg stopped at the change every
time, from this or from problem 3. Waiting in
iris_vb2_stop_streaming(), for at most 500 ms, until the firmware has
returned the PSC_LAST buffer fixed it here. Completing the resolution
change at STREAMON instead did not help: the firmware then returned
nothing at all.


5. S_FMT(OUTPUT) leaves the CAPTURE stride and size stale

Setting the OUTPUT format updates the CAPTURE width and height
(8aadfd445373) but not bytesperline and sizeimage, which keep their
320x240 defaults on a new instance. For a 640x360 stream,
G_FMT(CAPTURE) then reports 640x384 NV12 with a sizeimage of 147456,
and iris_vb2_buf_prepare() rejects CAPTURE buffers smaller than 368640
bytes, so a client that allocates them from G_FMT cannot queue them.
ffmpeg is not affected, because it sets the CAPTURE format itself.
Recomputing both fields as S_FMT(CAPTURE) does fixed it here.


With all five changes

 - ffmpeg h264_v4l2m2m: bit-exact against the software decoder on raw
   H.264 (5 of 5 runs), MP4 (3 of 3), and across the 640x360 to
   1280x720 change (2 of 2).
 - GStreamer: v4l2h264dec, v4l2h265dec, v4l2vp9dec and v4l2av1dec
   bit-exact at 1080p, H.264 and HEVC also at 2160p, and bit-exact
   across the resolution change. The encoders still pass.
 - the test client, with CAPTURE set up before or after the first
   event, and in a mode that aborts the drain like ffmpeg: 27 of 27
   runs bit-exact, and at the resolution change one EOS per run, at
   the end (12 of 12).
 - v4l2-compliance 1.33.0 (c6600a7e342e): decoder with -s5
   --stream-from=<720p H.264> and encoder with -s, 55 of 55 each. This
   tree predates 1facae9bc3c1, so the bus_info prefix check had to be
   relaxed for these runs.
 - 60 s of 2160p60 H.264 with 15 IDR frames: the firmware raised one
   source change, at the start.

[1] https://lore.kernel.org/linux-media/179224901845.60580.12634525576974095945@pm.me/

Thanks,
Christian Cortner