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

[Paul Kocialkowski]

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.

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.

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:



-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

Attachment: signature.asc
Description: PGP signature