Re: [PATCH] media: cedrus: skip invalid H.264 reference list entries

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3173 bytes --]

Le jeudi 09 avril 2026 à 17:31 +0200, Paul Kocialkowski a écrit :
> Hi,
> 
> On Thu 09 Apr 26, 10:39, Nicolas Dufresne wrote:
> > Hi,
> > 
> > Le jeudi 09 avril 2026 à 16:31 +0200, Paul Kocialkowski a écrit :
> > > I think it make sense yes, but it would be good to document it in the uAPI
> > > document too.
> > 
> > Basically, extend in the M2M decoder spec(s) on the existing documentation:
> >    
> >    V4L2_BUF_FLAG_ERROR:
> >    -
> >    When this flag is set, the buffer has been dequeued successfully, although
> >    the data might **have been corrupted**. This is recoverable, streaming may
> >    continue as normal and the buffer may be reused normally. Drivers set this
> >    flag when the VIDIOC_DQBUF ioctl is called.
> 
> Well this part is about v4l2 buffers in general, not just m2m/decoders.
> But I guess this mechanism would make sense for more device classes than just
> decoders, so we could indeed specify it there. Maybe with a sufficiently broad
> wording.

This is current spec, I did meant to use that as basis and improve that codec
specific part.

> 
> But it would be good to also update the stateless decode document (and maybe
> stateful too) where V4L2_BUF_FLAG_ERROR is already mentionned a few times.
> We could indicate how this behavior related to reference frames there.
> 
> If we agree I could make a series with the following:
> - Introduce a V4L2_H264_REF_MISSING 0xff define (same for HEVC)
> - Update the v4l2_h264_reference doc to mention it
> - Update the cedrus driver to error out (zero-size payload) when the L0/L1 index
>   is either V4L2_H264_REF_MISSING or an invalid index that doesn't exist in the
>   DPB (same for HEVC)

Base on what you reported, this is currently the shortest and safe thing to do.

> - Update the v4l2 buffer and stateless(+stateful) documents to mention that
>   buffers marked with V4L2_BUF_FLAG_ERROR may or may not contain usable (yet
>   corrupted) data depending on the payload size and how it relates to reference
>   frames.

wfm

> 
> Then we could later envision having a mechanism (hopefully common) to figure out
> the best replacement to a given missing reference, which would allow cedrus
> (and maybe other drivers too) to return a frame with incorrect data instead of
> a zero-size payload error.

Certainly, though from my experience best or any works quite well, and quite
better then skipping. But if it makes the HW unstable, or sending uninitialized
data to the HW, I agree this is better to hard fail that frame now, enhance
later. Also, be aware that some HW (RK3399 iirc) have error counters, so you
know how many macroblocks are not decoded properly. Other HW have error IRQ,
with a register that tells which macroblock it failed on. The reference decoder
have optional support for software concealment for the failing macroblock, the
down side is that in the worse case you get an irq per block, which is quite a
disaster performance whise.

> 
> What do you think?

Yes, I'm happy with the proposal to get this moving forward.

Nicolas

> 
> All the best,
> 
> Paul

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]