Re: [PATCH v4 5/6] media: Add controls for JPEG quantization tables

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ian, Hans:

On Mon, 2018-09-03 at 14:29 +0100, Ian Arkver wrote:
> Hi,
> 
> On 03/09/2018 10:50, Hans Verkuil wrote:
> > On 08/31/2018 05:52 PM, Ezequiel Garcia wrote:
> > > From: Shunqian Zheng <zhengsq@xxxxxxxxxxxxxx>
> > > 
> > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace
> > > configure the JPEG quantization tables.
> > > 
> > > Signed-off-by: Shunqian Zheng <zhengsq@xxxxxxxxxxxxxx>
> > > Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> > > ---
> > >   .../media/uapi/v4l/extended-controls.rst      | 23 +++++++++++++++++++
> > >   .../media/videodev2.h.rst.exceptions          |  1 +
> > >   drivers/media/v4l2-core/v4l2-ctrls.c          | 10 ++++++++
> > >   include/uapi/linux/v4l2-controls.h            |  5 ++++
> > >   include/uapi/linux/videodev2.h                |  1 +
> > >   5 files changed, 40 insertions(+)
> > > 
> > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> > > index 9f7312bf3365..e0dd03e452de 100644
> > > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > > @@ -3354,7 +3354,30 @@ JPEG Control IDs
> > >       Specify which JPEG markers are included in compressed stream. This
> > >       control is valid only for encoders.
> > >   
> > > +.. _jpeg-quant-tables-control:
> > >   
> > > +``V4L2_CID_JPEG_QUANTIZATION (struct)``
> > > +    Specifies the luma and chroma quantization matrices for encoding
> > > +    or decoding a V4L2_PIX_FMT_JPEG_RAW format buffer. The two matrices
> > > +    must be set in JPEG zigzag order, as per the JPEG specification.
> > 
> > Can you change "JPEG specification" to a reference to the JPEG spec entry
> > in bibio.rst?
> > 
> > > +
> > > +
> > > +.. c:type:: struct v4l2_ctrl_jpeg_quantization
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table:: struct v4l2_ctrl_jpeg_quantization
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +    :widths:       1 1 2
> > > +
> > > +    * - __u8
> > > +      - ``luma_quantization_matrix[64]``
> > > +      - Sets the luma quantization table.
> > > +
> > > +    * - __u8
> > > +      - ``chroma_quantization_matrix[64]``
> > > +      - Sets the chroma quantization table.
> > 
> > Just checking: the JPEG standard specifies this as unsigned 8-bit values as well?
> 

I thought this was already discussed, but I think the only thing I've added
is this comment in one of the driver's headers:

 JPEG encoder
 ------------
 The VPU JPEG encoder produces JPEG baseline sequential format.
 The quantization coefficients are 8-bit values, complying with
 the baseline specification. Therefore, it requires application-defined
 luma and chroma quantization tables. The hardware does entrophy
 encoding using internal Huffman tables, as specified in the JPEG
 specification.

Certainly controls should be specified better.

> As far as I can see ISO/IEC 10918-1 does not specify the precision or 
> signedness of the quantisation value Qvu. The default tables for 8-bit 
> baseline JPEG all fit into __u8 though.
> 

Paragraph 4.7 of that spec, indicates the "sample" precision:
8-bit for baseline; 8-bit or 12-bit for extended.

For the quantization coefficients, the DQT segment contains a bit
that indicates if the quantization coefficients are 8-bit or 16-bit.
See B.2.4.1 for details.

> However there can be four sets of tables in non-baseline JPEG and it's 

You lost me here, which four sets of tables are you refering to?

> not clear (to me) whether 12-bit JPEG would need more precision (I'd 
> guess it would).

It seems it would. From B.2.4.1:

"An 8-bit DCT-based process shall not use a 16-bit precision quantization table."

> Since this patch is defining UAPI I think it might be 
> good to build in some additional information, eg. number of tables, 
> element size. Maybe this can all be inferred from the selected pixel 
> format? If so then it would need documented that the above structure 
> only applies to baseline.
> 

For quantization coefficients, I can only see two tables: one for luma
one for chroma. Huffman coefficients are a different story and we are
not really adding them here.

Thanks,
Eze



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux