On Thu, Oct 17, 2019 at 12:08 AM Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote: > > 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. > Okay, I think you convinced me. :) +Alexandre Courbot to be aware of the upcoming UAPI change. Best regards, Tomasz
