Re: [PATCH v2] media: v4l2-ctrls: validate HEVC slice reference lists

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

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

Le jeudi 09 avril 2026 à 15:52 +0200, Paul Kocialkowski a écrit :
> Hi Nicolas,
> 
> On Mon 23 Mar 26, 09:41, Nicolas Dufresne wrote:
> > > +
> > > +		for (i = 0; i <= p_hevc_slice_params->num_ref_idx_l0_active_minus1;
> > > +		     i++)
> > > +			if (p_hevc_slice_params->ref_idx_l0[i] >=
> > > +			    V4L2_HEVC_DPB_ENTRIES_NUM_MAX)
> > > +				return -EINVAL;
> > 
> > That one is a breaking change since userspace already passes off limit values
> > such as 0xff when a reference is missing (was lost). See:
> > 
> > 	47825b1646a6a9eca0f90baa3d4f98947c2add96
> > 
> > The hardware may or may not be capable of doing concealment, but with this
> > change, we bring down all drivers to failing the decode completely.
> 
> So while some decoders may be able to deal with missing references, it seems
> that cedrus should still error out in that case. I don't think it will be very
> happy if we configure the hardware with L0/L1 lists that don't match what was
> used for the encode.

The L0/L1 list passed by application do match the decoder list, but has gaps. By
the spec, decoder should be gap resistant. You are better place them me to know
what the Cedrus hardware can and cannot do. This is rarely well document, and in
RE case like this, you probably have to do some trial and errors.

> 
> But maybe we could pick up another (existing or empty) reference to replace the
> missing one, which would be better than failing to decode the frame. IMO this
> would be best done by userspace, but maybe we'd need some indication to know
> that the hardware cannot deal with missing references.

Exact, experimenting first seems key, every hardware I've worked with behaves
differently. Hantro G2 notably tends to cause system wide issues on imx8mq if
you don't carefully select your replacement. Yet, after testing and looking at
the visual result, its way better (visually) to pick a replacement. I have
patches coming that tracks which frame have decoded successfully and holds
initialized MV data (which was the reason it was going wild).

Note that going one step further, on HEVC we could use the poc to find a valid
replacement that is temporarily closer, but that would simply be an enhancement
for streams with reordering.

Nicolas

> 
> What do you think?
> 
> All the best,
> 
> Paul
> 
> > > +
> > > +		if (p_hevc_slice_params->slice_type != V4L2_HEVC_SLICE_TYPE_B)
> > > +			break;
> > > +
> > > +		if (p_hevc_slice_params->num_ref_idx_l1_active_minus1 >=
> > > +		    V4L2_HEVC_DPB_ENTRIES_NUM_MAX)
> > > +			return -EINVAL;
> > 
> > Ack.
> > 
> > > +
> > > +		for (i = 0; i <= p_hevc_slice_params->num_ref_idx_l1_active_minus1;
> > > +		     i++)
> > > +			if (p_hevc_slice_params->ref_idx_l1[i] >=
> > > +			    V4L2_HEVC_DPB_ENTRIES_NUM_MAX)
> > > +				return -EINVAL;
> > 
> > Same.
> > 
> > cheers,
> > Nicolas
> > 
> > >  		break;
> > >  
> > >  	case V4L2_CTRL_TYPE_HEVC_EXT_SPS_ST_RPS:
> 
> 

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