On Wed, 2019-10-16 at 15:37 +0200, Paul Kocialkowski wrote: [...] > > > The bottomline is that we have use cases for each of the two set of fields > > > independently, so I feel like this is reason enough to avoid mixing them > > > together. > > > > What do you mean by mixing together? Hardware parsing the slices > > always uses num_ref_idx_l[01]_default_active_minus1 from the PPS. > > Hardware not parsing the slices always sets override to 1 and uses > > num_ref_idx_l[01]_active_minus1 from the slice header struct. > > To summarize, what I don't understand is why it's worth re-purposing > the slice header's num_ref_idx_l[01]_active_minus1 to contain > num_ref_idx_l[01]_default_active_minus1 when the flag is not set in the initial > bitstream instead of exposing the flag. > > There's hardware (like cedrus) which takes both fields and the flag directly > in-registers, so it's really not a simplification here. And even in cases where > the hardware only takes one field, I believe that the downside of re-purposing > the field of the control is much greater than the benefit of the supposed > simplification. > > I know this sounds quite futile, but I thought there was an implicit agreement > that controls must stick as close as possible to the bitstream. This is an > occurence where we are diverging for no particularly strong reason. FWIW, I agree with Paul on this. That drivers for codecs which do not parse the slice headers always completely ignore the num_ref_idx_l[01]_default_active_minus1 fields, but instead expect the num_ref_idx_l[01]_active_minus1 field to be repurposed to contain the default values if the corresponding fields do not exist in the slice header (that is, when the num_ref_idx_active_override_flag is not set), confused me at first [1]. This seems to follow what libva does [2], and it does simplify drivers a tiny bit, but I'd still prefer to explicitly have the num_ref_idx_active_override_flag contained in the API, and to have the num_ref_idx_l[01]_active_minus1 fields only be used for num_ref_idx_l[01]_active_minus1, and not have them sometimes contain the values of another field. [1] https://patchwork.linuxtv.org/patch/58580/ [2] https://github.com/intel/libva/blob/95eb8cf469367b532b391042fa0e77ca513ac94e/va/va.h#L3138 > Expecting that userspace does this pre-processing of fields feels quite > counter-intuitive and confusing for people wishing to use the API, too. > One would certainly naively expect that the fields in the controls carry the > same meaning as in the bitstream when they have the same name. I certainly naively did. regards Philipp
