Re: [PATCH v4 1/9] media: uapi: mpeg2: Rework quantization matrices semantics

[Ezequiel Garcia <ezequiel@collabora.com>]

Hi Nicolas,

On Mon, 2021-03-29 at 16:53 -0400, Nicolas Dufresne wrote:
> Le lundi 29 mars 2021 à 15:13 -0300, Ezequiel Garcia a écrit :
> > As stated in the MPEG-2 specification, section 6.3.7 "Quant matrix
> > extension":
> > 
> >   Each quantisation matrix has a default set of values. When a
> >   sequence_header_code is decoded all matrices shall be reset to
> >   their default values. User defined matrices may be downloaded
> >   and this can occur in a sequence_header() or in a
> >   quant_matrix_extension().
> > 
> > The load_intra_quantiser_matrix syntax elements are transmitted
> > in the bistream headers, signalling that a quantization matrix
>                                              quantisation
> 
> Not really a typo, just a suggestion to follow the specification spelling. I
> would like to see concistant spelling the API. My rational is that you can copy
> and paste the strings when searching inside the specification PDF, and you don't
> mix both in the API like we do now.
> 

Absolutely. Thanks for spotting this.

> > needs to be loaded and used for pictures transmitted afterwards
> > (until the matrices are reset).
> > 
> > These "load" semantics are implemented in the V4L2 interface
> > without the need of any "load" flags: passing the control
> > is effectively a load.
> > 
> > Therefore, rework the V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION
>                                                         S
> 
> > semantics to match the MPEG-2 semantics. Quantization matrices
>                                                  s
> 
> etc.
> 
> > values are now initialized by the V4L2 control core to their
> > reset default value, and applications are expected to reset
> > their values as specified.
> > 
> > The quantization control is therefore optional, and used to
> > load bitstream-defined values in the quantization matrices.
> 
> Perhaps:
> 
> "The quantisation controls is therefore optional for decoding streams that uses
> the default matrices."
> 
> A stack that would not handle the default, would have to read the control at
> least once in order to avoid overriding valid values with 0s, not sure if that
> is worth mentioning ?
> 

Hm, not entirely sure. If application calls S_EXT_CTRL with 0s,
then that's what will happen.

Thanks,
Ezequiel