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
