On Fri, Oct 4, 2019 at 6:12 AM Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> wrote: > > Hi, > > On Thu 05 Sep 19, 13:42, Philipp Zabel wrote: > > To explain why num_ref_idx_active_override_flag is not part of the API, > > describe how the num_ref_idx_l[01]_active_minus1 fields and the > > num_ref_idx_l[01]_default_active_minus1 fields are used, depending on > > whether the decoder parses slice headers itself or not. > > Is there any particular reason why this is preferable to exposing the flag? > It feels like having the flag around sticks closer to the bitstream, > so it's more straightforward for everyone. > > In case there's only one set of fields exposed by the hardware (and it doesn't > do slice parsing itself), we could always check the flag in the driver and use > either the default PPS values or the slice-specific values there. > > What do you think? IMHO it would only add further logic to the drivers, because they would need to have a conditional block that selects default or per-slice value based on the flag. The goal was to be able for the driver to just passively write num_ref_idx_l[01]_active_minus1 (or the default one for slice-parsing hardware) to corresponding hardware registers. We're talking about kernel drivers here and for security reasons any logic should be reduced to the strictly necessary minimum. Best regards, Tomasz > > Cheers, > > Paul > > > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > > --- > > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > index bc5dd8e76567..b9834625a939 100644 > > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > @@ -1630,10 +1630,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > - > > * - __u8 > > - ``num_ref_idx_l0_default_active_minus1`` > > - - > > + - This field is only used by decoders that parse slices themselves. > > * - __u8 > > - ``num_ref_idx_l1_default_active_minus1`` > > - - > > + - This field is only used by decoders that parse slices themselves. > > * - __u8 > > - ``weighted_bipred_idc`` > > - > > @@ -1820,10 +1820,14 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > - > > * - __u8 > > - ``num_ref_idx_l0_active_minus1`` > > - - > > + - This field is used by decoders that do not parse slices themselves. > > + If num_ref_idx_active_override_flag is not set, this field must be > > + set to the value of num_ref_idx_l0_default_active_minus1. > > * - __u8 > > - ``num_ref_idx_l1_active_minus1`` > > - - > > + - This field is used by decoders that do not parse slices themselves. > > + If num_ref_idx_active_override_flag is not set, this field must be > > + set to the value of num_ref_idx_l1_default_active_minus1. > > * - __u32 > > - ``slice_group_change_cycle`` > > - > > -- > > 2.20.1 > > > > -- > Paul Kocialkowski, Bootlin > Embedded Linux and kernel engineering > https://bootlin.com
