| Message ID | 20180905220011.16612-6-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Rockchip VPU JPEG encoder | expand |
On 09/06/2018 12:00 AM, Ezequiel Garcia wrote: > From: Shunqian Zheng <zhengsq@rock-chips.com> > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > configure the JPEG quantization tables. > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > .../media/videodev2.h.rst.exceptions | 1 + > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > include/uapi/linux/v4l2-controls.h | 12 +++++++ > include/uapi/linux/videodev2.h | 1 + > 5 files changed, 55 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > index 9f7312bf3365..1335d27d30f3 100644 > --- a/Documentation/media/uapi/v4l/extended-controls.rst > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > @@ -3354,7 +3354,38 @@ 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 :ref:`itu-t81` > + specification allows 8-bit quantization coefficients for > + baseline profile images, and 8-bit or 16-bit for extended profile > + images. Supporting or not 16-bit precision coefficients is driver-specific. > + Coefficients must be set in JPEG zigzag scan order. > + > + > +.. 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 > + - ``precision`` > + - Specifies the coefficient precision. User shall set 0 > + for 8-bit, and 1 for 16-bit. So does specifying 1 here switch the HW encoder to use extended profile? What if the HW only supports baseline? The rockchip driver doesn't appear to check the precision field at all... I think this needs a bit more thought. I am not at all sure that this is the right place for the precision field. This is really about JPEG profiles, so I would kind of expect a JPEG PROFILE control (just like other codec profiles), or possibly a new pixelformat for extended profiles. And based on that the driver would interpret these matrix values as 8 or 16 bits. Regards, Hans > + > + * - __u16 > + - ``luma_quantization_matrix[64]`` > + - Sets the luma quantization table. > + > + * - __u16 > + - ``chroma_quantization_matrix[64]`` > + - Sets the chroma quantization table. > > .. flat-table:: > :header-rows: 0 > diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions > index ca9f0edc579e..a0a38e92bf38 100644 > --- a/Documentation/media/videodev2.h.rst.exceptions > +++ b/Documentation/media/videodev2.h.rst.exceptions > @@ -129,6 +129,7 @@ replace symbol V4L2_CTRL_TYPE_STRING :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_U16 :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_U32 :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_U8 :c:type:`v4l2_ctrl_type` > +replace symbol V4L2_CTRL_TYPE_JPEG_QUANTIZATION :c:type:`v4l2_ctrl_type` > > # V4L2 capability defines > replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index 599c1cbff3b9..305bd7a9b7f1 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -999,6 +999,7 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_JPEG_RESTART_INTERVAL: return "Restart Interval"; > case V4L2_CID_JPEG_COMPRESSION_QUALITY: return "Compression Quality"; > case V4L2_CID_JPEG_ACTIVE_MARKER: return "Active Markers"; > + case V4L2_CID_JPEG_QUANTIZATION: return "JPEG Quantization Tables"; > > /* Image source controls */ > /* Keep the order of the 'case's the same as in v4l2-controls.h! */ > @@ -1286,6 +1287,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_DETECT_MD_REGION_GRID: > *type = V4L2_CTRL_TYPE_U8; > break; > + case V4L2_CID_JPEG_QUANTIZATION: > + *type = V4L2_CTRL_TYPE_JPEG_QUANTIZATION; > + break; > case V4L2_CID_DETECT_MD_THRESHOLD_GRID: > *type = V4L2_CTRL_TYPE_U16; > break; > @@ -1612,6 +1616,9 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, > return -ERANGE; > return 0; > > + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: > + return 0; > + > default: > return -EINVAL; > } > @@ -2133,6 +2140,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, > case V4L2_CTRL_TYPE_U32: > elem_size = sizeof(u32); > break; > + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: > + elem_size = sizeof(struct v4l2_ctrl_jpeg_quantization); > + break; > default: > if (type < V4L2_CTRL_COMPOUND_TYPES) > elem_size = sizeof(s32); > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index e4ee10ee917d..856b3325052f 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -987,6 +987,18 @@ enum v4l2_jpeg_chroma_subsampling { > #define V4L2_JPEG_ACTIVE_MARKER_DQT (1 << 17) > #define V4L2_JPEG_ACTIVE_MARKER_DHT (1 << 18) > > +#define V4L2_CID_JPEG_QUANTIZATION (V4L2_CID_JPEG_CLASS_BASE + 5) > +struct v4l2_ctrl_jpeg_quantization { > + /* ITU-T.81 specifies two quantization coefficient precisions: > + * 8-bit for baseline profile, > + * 8-bit or 16-bit for extended profile. > + * > + * User shall set "precision" to 0 for 8-bit and 1 for 16-bit. > + */ > + __u8 precision; > + __u16 luma_quantization_matrix[64]; > + __u16 chroma_quantization_matrix[64]; > +}; > > /* Image source controls */ > #define V4L2_CID_IMAGE_SOURCE_CLASS_BASE (V4L2_CTRL_CLASS_IMAGE_SOURCE | 0x900) > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index f9f3ae5b489e..8ace47cb1003 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -1637,6 +1637,7 @@ enum v4l2_ctrl_type { > V4L2_CTRL_TYPE_U8 = 0x0100, > V4L2_CTRL_TYPE_U16 = 0x0101, > V4L2_CTRL_TYPE_U32 = 0x0102, > + V4L2_CTRL_TYPE_JPEG_QUANTIZATION = 0x0103, > }; > > /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */ >
Hi Hans, Thanks for the review. On Mon, 2018-09-10 at 14:42 +0200, Hans Verkuil wrote: > On 09/06/2018 12:00 AM, Ezequiel Garcia wrote: > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > configure the JPEG quantization tables. > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > > .../media/videodev2.h.rst.exceptions | 1 + > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > > include/uapi/linux/v4l2-controls.h | 12 +++++++ > > include/uapi/linux/videodev2.h | 1 + > > 5 files changed, 55 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > index 9f7312bf3365..1335d27d30f3 100644 > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > @@ -3354,7 +3354,38 @@ 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 :ref:`itu-t81` > > + specification allows 8-bit quantization coefficients for > > + baseline profile images, and 8-bit or 16-bit for extended profile > > + images. Supporting or not 16-bit precision coefficients is driver-specific. > > + Coefficients must be set in JPEG zigzag scan order. > > + > > + > > +.. 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 > > + - ``precision`` > > + - Specifies the coefficient precision. User shall set 0 > > + for 8-bit, and 1 for 16-bit. > > So does specifying 1 here switch the HW encoder to use extended profile? > What if the HW only supports baseline? The rockchip driver doesn't appear > to check the precision field at all... > The driver is missing to check that, when the user sets this control. > I think this needs a bit more thought. > > I am not at all sure that this is the right place for the precision field. > This is really about JPEG profiles, so I would kind of expect a JPEG PROFILE > control (just like other codec profiles), or possibly a new pixelformat for > extended profiles. > > And based on that the driver would interpret these matrix values as 8 or > 16 bits. > Right, the JPEG profile control is definitely needed. I haven't add it because it wouldn't be used, since this VPU can only do baseline. However, the problem is that some JPEGs in the wild have with 8-bit data and 16-bit quantization coefficients, as per [1] and [2]: [1] https://github.com/martinhath/jpeg-rust/issues/1 [2] https://github.com/libjpeg-turbo/libjpeg-turbo/pull/90 So, in order to support decoding of these images, I've added the precision field to the quantization control. The user would be able to set a baseline or extended profile thru a (future) profile control, and if 16-bit tables are found, and if the hardware supports them, the driver would be able to support them. Another option, which might be even better, is have explicit baseline and extended quantization tables controls, e.g.: V4L2_CID_JPEG_QUANT and V4L2_CID_JPEG_EXT_QUANT. Thanks, Ezequiel
Hi, On Wed, 2018-09-05 at 19:00 -0300, Ezequiel Garcia wrote: > From: Shunqian Zheng <zhengsq@rock-chips.com> > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > configure the JPEG quantization tables. > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > .../media/videodev2.h.rst.exceptions | 1 + > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > include/uapi/linux/v4l2-controls.h | 12 +++++++ > include/uapi/linux/videodev2.h | 1 + > 5 files changed, 55 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > index 9f7312bf3365..1335d27d30f3 100644 > --- a/Documentation/media/uapi/v4l/extended-controls.rst > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > @@ -3354,7 +3354,38 @@ JPEG Control IDs > Specify which JPEG markers are included in compressed stream. This > control is valid only for encoders. > > +.. _jpeg-quant-tables-control: I just had a look at how the Allwinner VPU handles JPEG decoding and it seems to require the following information (in addition to quantization): * Horizontal and vertical sampling factors for each Y/U/V component: The number of components and sampling factors are coded separately in the bitstream, but it's probably easier to use the already-existing V4L2_CID_JPEG_CHROMA_SUBSAMPLING control for specifying that. However, this is potentially very much related to the destination format. If we decide that this format should match the format resulting from decompression, we don't need to specify it through an external control. On the other hand, it's possible that the VPU has format conversion block integrated in its pipeline so it would also make sense to consider the destination format as independent. * Custom Huffman tables (DC and AC), both for luma and chroma It seems that there is a default table when no Huffman table is provided in the bitstream (I'm not too sure how standard that is, just started learning about JPEG). We probably need a specific control for that. * Reset interval That's extracted from the bitstream as well and there's a V4L2_CID_JPEG_RESTART_INTERVAL control already. In addition to these points, I see that among all the JPEG profiles, some have to do with arithmetic coding which will probably require a specific control on its own (not sure how it should look at this point though). What do you think? Cheers, Paul > +``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 :ref:`itu-t81` > + specification allows 8-bit quantization coefficients for > + baseline profile images, and 8-bit or 16-bit for extended profile > + images. Supporting or not 16-bit precision coefficients is driver-specific. > + Coefficients must be set in JPEG zigzag scan order. > + > + > +.. 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 > + - ``precision`` > + - Specifies the coefficient precision. User shall set 0 > + for 8-bit, and 1 for 16-bit. > + > + * - __u16 > + - ``luma_quantization_matrix[64]`` > + - Sets the luma quantization table. > + > + * - __u16 > + - ``chroma_quantization_matrix[64]`` > + - Sets the chroma quantization table. > > .. flat-table:: > :header-rows: 0 > diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions > index ca9f0edc579e..a0a38e92bf38 100644 > --- a/Documentation/media/videodev2.h.rst.exceptions > +++ b/Documentation/media/videodev2.h.rst.exceptions > @@ -129,6 +129,7 @@ replace symbol V4L2_CTRL_TYPE_STRING :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_U16 :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_U32 :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_U8 :c:type:`v4l2_ctrl_type` > +replace symbol V4L2_CTRL_TYPE_JPEG_QUANTIZATION :c:type:`v4l2_ctrl_type` > > # V4L2 capability defines > replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index 599c1cbff3b9..305bd7a9b7f1 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -999,6 +999,7 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_JPEG_RESTART_INTERVAL: return "Restart Interval"; > case V4L2_CID_JPEG_COMPRESSION_QUALITY: return "Compression Quality"; > case V4L2_CID_JPEG_ACTIVE_MARKER: return "Active Markers"; > + case V4L2_CID_JPEG_QUANTIZATION: return "JPEG Quantization Tables"; > > /* Image source controls */ > /* Keep the order of the 'case's the same as in v4l2-controls.h! */ > @@ -1286,6 +1287,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_DETECT_MD_REGION_GRID: > *type = V4L2_CTRL_TYPE_U8; > break; > + case V4L2_CID_JPEG_QUANTIZATION: > + *type = V4L2_CTRL_TYPE_JPEG_QUANTIZATION; > + break; > case V4L2_CID_DETECT_MD_THRESHOLD_GRID: > *type = V4L2_CTRL_TYPE_U16; > break; > @@ -1612,6 +1616,9 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, > return -ERANGE; > return 0; > > + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: > + return 0; > + > default: > return -EINVAL; > } > @@ -2133,6 +2140,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, > case V4L2_CTRL_TYPE_U32: > elem_size = sizeof(u32); > break; > + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: > + elem_size = sizeof(struct v4l2_ctrl_jpeg_quantization); > + break; > default: > if (type < V4L2_CTRL_COMPOUND_TYPES) > elem_size = sizeof(s32); > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index e4ee10ee917d..856b3325052f 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -987,6 +987,18 @@ enum v4l2_jpeg_chroma_subsampling { > #define V4L2_JPEG_ACTIVE_MARKER_DQT (1 << 17) > #define V4L2_JPEG_ACTIVE_MARKER_DHT (1 << 18) > > +#define V4L2_CID_JPEG_QUANTIZATION (V4L2_CID_JPEG_CLASS_BASE + 5) > +struct v4l2_ctrl_jpeg_quantization { > + /* ITU-T.81 specifies two quantization coefficient precisions: > + * 8-bit for baseline profile, > + * 8-bit or 16-bit for extended profile. > + * > + * User shall set "precision" to 0 for 8-bit and 1 for 16-bit. > + */ > + __u8 precision; > + __u16 luma_quantization_matrix[64]; > + __u16 chroma_quantization_matrix[64]; > +}; > > /* Image source controls */ > #define V4L2_CID_IMAGE_SOURCE_CLASS_BASE (V4L2_CTRL_CLASS_IMAGE_SOURCE | 0x900) > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index f9f3ae5b489e..8ace47cb1003 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -1637,6 +1637,7 @@ enum v4l2_ctrl_type { > V4L2_CTRL_TYPE_U8 = 0x0100, > V4L2_CTRL_TYPE_U16 = 0x0101, > V4L2_CTRL_TYPE_U32 = 0x0102, > + V4L2_CTRL_TYPE_JPEG_QUANTIZATION = 0x0103, > }; > > /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
Hi, On Thu, 2018-09-13 at 14:14 +0200, Paul Kocialkowski wrote: > Hi, > > On Wed, 2018-09-05 at 19:00 -0300, Ezequiel Garcia wrote: > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > configure the JPEG quantization tables. > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > > .../media/videodev2.h.rst.exceptions | 1 + > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > > include/uapi/linux/v4l2-controls.h | 12 +++++++ > > include/uapi/linux/videodev2.h | 1 + > > 5 files changed, 55 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > index 9f7312bf3365..1335d27d30f3 100644 > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > @@ -3354,7 +3354,38 @@ JPEG Control IDs > > Specify which JPEG markers are included in compressed stream. This > > control is valid only for encoders. > > > > +.. _jpeg-quant-tables-control: > > I just had a look at how the Allwinner VPU handles JPEG decoding and it > seems to require the following information (in addition to > quantization): > > * Horizontal and vertical sampling factors for each Y/U/V component: > > The number of components and sampling factors are coded separately in > the bitstream, but it's probably easier to use the already-existing > V4L2_CID_JPEG_CHROMA_SUBSAMPLING control for specifying that. > > However, this is potentially very much related to the destination > format. If we decide that this format should match the format resulting > from decompression, we don't need to specify it through an external > control. On the other hand, it's possible that the VPU has format > conversion block integrated in its pipeline so it would also make sense > to consider the destination format as independent. > > * Custom Huffman tables (DC and AC), both for luma and chroma > > It seems that there is a default table when no Huffman table is provided > in the bitstream (I'm not too sure how standard that is, just started > learning about JPEG). We probably need a specific control for that. > > * Reset interval > > That's extracted from the bitstream as well and there's a > V4L2_CID_JPEG_RESTART_INTERVAL control already. > > In addition to these points, I see that among all the JPEG profiles, > some have to do with arithmetic coding which will probably require a > specific control on its own (not sure how it should look at this point > though). > > What do you think? For clarification: these are required for decoding, not for encoding. For encoding, it seems that the Allwinner hardware only requires quantization tables. Cheers, Paul > > +``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 :ref:`itu-t81` > > + specification allows 8-bit quantization coefficients for > > + baseline profile images, and 8-bit or 16-bit for extended profile > > + images. Supporting or not 16-bit precision coefficients is driver-specific. > > + Coefficients must be set in JPEG zigzag scan order. > > + > > + > > +.. 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 > > + - ``precision`` > > + - Specifies the coefficient precision. User shall set 0 > > + for 8-bit, and 1 for 16-bit. > > + > > + * - __u16 > > + - ``luma_quantization_matrix[64]`` > > + - Sets the luma quantization table. > > + > > + * - __u16 > > + - ``chroma_quantization_matrix[64]`` > > + - Sets the chroma quantization table. > > > > .. flat-table:: > > :header-rows: 0 > > diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions > > index ca9f0edc579e..a0a38e92bf38 100644 > > --- a/Documentation/media/videodev2.h.rst.exceptions > > +++ b/Documentation/media/videodev2.h.rst.exceptions > > @@ -129,6 +129,7 @@ replace symbol V4L2_CTRL_TYPE_STRING :c:type:`v4l2_ctrl_type` > > replace symbol V4L2_CTRL_TYPE_U16 :c:type:`v4l2_ctrl_type` > > replace symbol V4L2_CTRL_TYPE_U32 :c:type:`v4l2_ctrl_type` > > replace symbol V4L2_CTRL_TYPE_U8 :c:type:`v4l2_ctrl_type` > > +replace symbol V4L2_CTRL_TYPE_JPEG_QUANTIZATION :c:type:`v4l2_ctrl_type` > > > > # V4L2 capability defines > > replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > > index 599c1cbff3b9..305bd7a9b7f1 100644 > > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > > @@ -999,6 +999,7 @@ const char *v4l2_ctrl_get_name(u32 id) > > case V4L2_CID_JPEG_RESTART_INTERVAL: return "Restart Interval"; > > case V4L2_CID_JPEG_COMPRESSION_QUALITY: return "Compression Quality"; > > case V4L2_CID_JPEG_ACTIVE_MARKER: return "Active Markers"; > > + case V4L2_CID_JPEG_QUANTIZATION: return "JPEG Quantization Tables"; > > > > /* Image source controls */ > > /* Keep the order of the 'case's the same as in v4l2-controls.h! */ > > @@ -1286,6 +1287,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > > case V4L2_CID_DETECT_MD_REGION_GRID: > > *type = V4L2_CTRL_TYPE_U8; > > break; > > + case V4L2_CID_JPEG_QUANTIZATION: > > + *type = V4L2_CTRL_TYPE_JPEG_QUANTIZATION; > > + break; > > case V4L2_CID_DETECT_MD_THRESHOLD_GRID: > > *type = V4L2_CTRL_TYPE_U16; > > break; > > @@ -1612,6 +1616,9 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, > > return -ERANGE; > > return 0; > > > > + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: > > + return 0; > > + > > default: > > return -EINVAL; > > } > > @@ -2133,6 +2140,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, > > case V4L2_CTRL_TYPE_U32: > > elem_size = sizeof(u32); > > break; > > + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: > > + elem_size = sizeof(struct v4l2_ctrl_jpeg_quantization); > > + break; > > default: > > if (type < V4L2_CTRL_COMPOUND_TYPES) > > elem_size = sizeof(s32); > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > > index e4ee10ee917d..856b3325052f 100644 > > --- a/include/uapi/linux/v4l2-controls.h > > +++ b/include/uapi/linux/v4l2-controls.h > > @@ -987,6 +987,18 @@ enum v4l2_jpeg_chroma_subsampling { > > #define V4L2_JPEG_ACTIVE_MARKER_DQT (1 << 17) > > #define V4L2_JPEG_ACTIVE_MARKER_DHT (1 << 18) > > > > +#define V4L2_CID_JPEG_QUANTIZATION (V4L2_CID_JPEG_CLASS_BASE + 5) > > +struct v4l2_ctrl_jpeg_quantization { > > + /* ITU-T.81 specifies two quantization coefficient precisions: > > + * 8-bit for baseline profile, > > + * 8-bit or 16-bit for extended profile. > > + * > > + * User shall set "precision" to 0 for 8-bit and 1 for 16-bit. > > + */ > > + __u8 precision; > > + __u16 luma_quantization_matrix[64]; > > + __u16 chroma_quantization_matrix[64]; > > +}; > > > > /* Image source controls */ > > #define V4L2_CID_IMAGE_SOURCE_CLASS_BASE (V4L2_CTRL_CLASS_IMAGE_SOURCE | 0x900) > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > index f9f3ae5b489e..8ace47cb1003 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -1637,6 +1637,7 @@ enum v4l2_ctrl_type { > > V4L2_CTRL_TYPE_U8 = 0x0100, > > V4L2_CTRL_TYPE_U16 = 0x0101, > > V4L2_CTRL_TYPE_U32 = 0x0102, > > + V4L2_CTRL_TYPE_JPEG_QUANTIZATION = 0x0103, > > }; > > > > /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
Hi, On Mon, 2018-09-10 at 10:25 -0300, Ezequiel Garcia wrote: > Hi Hans, > > Thanks for the review. > > On Mon, 2018-09-10 at 14:42 +0200, Hans Verkuil wrote: > > On 09/06/2018 12:00 AM, Ezequiel Garcia wrote: > > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > > configure the JPEG quantization tables. > > > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > > --- > > > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > > > .../media/videodev2.h.rst.exceptions | 1 + > > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > > > include/uapi/linux/v4l2-controls.h | 12 +++++++ > > > include/uapi/linux/videodev2.h | 1 + > > > 5 files changed, 55 insertions(+) > > > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > > index 9f7312bf3365..1335d27d30f3 100644 > > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > > @@ -3354,7 +3354,38 @@ 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 :ref:`itu-t81` > > > + specification allows 8-bit quantization coefficients for > > > + baseline profile images, and 8-bit or 16-bit for extended profile > > > + images. Supporting or not 16-bit precision coefficients is driver-specific. > > > + Coefficients must be set in JPEG zigzag scan order. > > > + > > > + > > > +.. 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 > > > + - ``precision`` > > > + - Specifies the coefficient precision. User shall set 0 > > > + for 8-bit, and 1 for 16-bit. > > > > So does specifying 1 here switch the HW encoder to use extended profile? > > What if the HW only supports baseline? The rockchip driver doesn't appear > > to check the precision field at all... > > > > The driver is missing to check that, when the user sets this control. > > > I think this needs a bit more thought. > > > > I am not at all sure that this is the right place for the precision field. > > This is really about JPEG profiles, so I would kind of expect a JPEG PROFILE > > control (just like other codec profiles), or possibly a new pixelformat for > > extended profiles. > > > > And based on that the driver would interpret these matrix values as 8 or > > 16 bits. > > > > Right, the JPEG profile control is definitely needed. I haven't add it because > it wouldn't be used, since this VPU can only do baseline. Well, I suppose it would still be relevant that you add it for the encoder and only report baseline there. > However, the problem is that some JPEGs in the wild have with 8-bit data and > 16-bit quantization coefficients, as per [1] and [2]: > > [1] https://github.com/martinhath/jpeg-rust/issues/1 > [2] https://github.com/libjpeg-turbo/libjpeg-turbo/pull/90 > > So, in order to support decoding of these images, I've added the precision > field to the quantization control. The user would be able to set a baseline > or extended profile thru a (future) profile control, and if 16-bit > tables are found, and if the hardware supports them, the driver > would be able to support them. > > Another option, which might be even better, is have explicit baseline > and extended quantization tables controls, e.g.: V4L2_CID_JPEG_QUANT > and V4L2_CID_JPEG_EXT_QUANT. I think this makes more sense than a common structure with an indication bit on how to interpret the data. However, it seems problematic that userspace can't figure out whether 16-bit quant tables are supported with a baseline profile and just has to try and see. Hans, do you think this is an acceptable approach or should we rather stick to the standard here, at the cost of not supporting these pictures that were encoded with this common abuse of the standard? Cheers, Paul
On Sun, Sep 16, 2018 at 1:48 AM Paul Kocialkowski <contact@paulk.fr> wrote: > > Hi, > > On Mon, 2018-09-10 at 10:25 -0300, Ezequiel Garcia wrote: > > Hi Hans, > > > > Thanks for the review. > > > > On Mon, 2018-09-10 at 14:42 +0200, Hans Verkuil wrote: > > > On 09/06/2018 12:00 AM, Ezequiel Garcia wrote: > > > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > > > configure the JPEG quantization tables. > > > > > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > > > --- > > > > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > > > > .../media/videodev2.h.rst.exceptions | 1 + > > > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > > > > include/uapi/linux/v4l2-controls.h | 12 +++++++ > > > > include/uapi/linux/videodev2.h | 1 + > > > > 5 files changed, 55 insertions(+) > > > > > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > > > index 9f7312bf3365..1335d27d30f3 100644 > > > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > > > @@ -3354,7 +3354,38 @@ 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 :ref:`itu-t81` > > > > + specification allows 8-bit quantization coefficients for > > > > + baseline profile images, and 8-bit or 16-bit for extended profile > > > > + images. Supporting or not 16-bit precision coefficients is driver-specific. > > > > + Coefficients must be set in JPEG zigzag scan order. > > > > + > > > > + > > > > +.. 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 > > > > + - ``precision`` > > > > + - Specifies the coefficient precision. User shall set 0 > > > > + for 8-bit, and 1 for 16-bit. > > > > > > So does specifying 1 here switch the HW encoder to use extended profile? > > > What if the HW only supports baseline? The rockchip driver doesn't appear > > > to check the precision field at all... > > > > > > > The driver is missing to check that, when the user sets this control. > > > > > I think this needs a bit more thought. > > > > > > I am not at all sure that this is the right place for the precision field. > > > This is really about JPEG profiles, so I would kind of expect a JPEG PROFILE > > > control (just like other codec profiles), or possibly a new pixelformat for > > > extended profiles. > > > > > > And based on that the driver would interpret these matrix values as 8 or > > > 16 bits. > > > > > > > Right, the JPEG profile control is definitely needed. I haven't add it because > > it wouldn't be used, since this VPU can only do baseline. > > Well, I suppose it would still be relevant that you add it for the > encoder and only report baseline there. > > > However, the problem is that some JPEGs in the wild have with 8-bit data and > > 16-bit quantization coefficients, as per [1] and [2]: > > > > [1] https://github.com/martinhath/jpeg-rust/issues/1 > > [2] https://github.com/libjpeg-turbo/libjpeg-turbo/pull/90 > > > > So, in order to support decoding of these images, I've added the precision > > field to the quantization control. The user would be able to set a baseline > > or extended profile thru a (future) profile control, and if 16-bit > > tables are found, and if the hardware supports them, the driver > > would be able to support them. > > > > Another option, which might be even better, is have explicit baseline > > and extended quantization tables controls, e.g.: V4L2_CID_JPEG_QUANT > > and V4L2_CID_JPEG_EXT_QUANT. > > I think this makes more sense than a common structure with an indication > bit on how to interpret the data. > > However, it seems problematic that userspace can't figure out whether > 16-bit quant tables are supported with a baseline profile and just has > to try and see. > > Hans, do you think this is an acceptable approach or should we rather > stick to the standard here, at the cost of not supporting these pictures > that were encoded with this common abuse of the standard? Perhaps we just need a control called V4L2_CID_JPEG_QUANT_PRECISION, where drivers can set the min/max (e.g. min = 8, max = 16, step = 8) to the range they support and user space can select the precision it wants, if the hardware gives a choice? Best regards, Tomasz
On Thu, Sep 13, 2018 at 9:15 PM Paul Kocialkowski <contact@paulk.fr> wrote: > > Hi, > > On Wed, 2018-09-05 at 19:00 -0300, Ezequiel Garcia wrote: > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > configure the JPEG quantization tables. > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > > .../media/videodev2.h.rst.exceptions | 1 + > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > > include/uapi/linux/v4l2-controls.h | 12 +++++++ > > include/uapi/linux/videodev2.h | 1 + > > 5 files changed, 55 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > index 9f7312bf3365..1335d27d30f3 100644 > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > @@ -3354,7 +3354,38 @@ JPEG Control IDs > > Specify which JPEG markers are included in compressed stream. This > > control is valid only for encoders. > > > > +.. _jpeg-quant-tables-control: > > I just had a look at how the Allwinner VPU handles JPEG decoding and it > seems to require the following information (in addition to > quantization): I assume the hardware doesn't have the ability to parse those from the stream and so they need to be parsed by user space and given to the driver? > > * Horizontal and vertical sampling factors for each Y/U/V component: > > The number of components and sampling factors are coded separately in > the bitstream, but it's probably easier to use the already-existing > V4L2_CID_JPEG_CHROMA_SUBSAMPLING control for specifying that. > > However, this is potentially very much related to the destination > format. If we decide that this format should match the format resulting > from decompression, we don't need to specify it through an external > control. On the other hand, it's possible that the VPU has format > conversion block integrated in its pipeline so it would also make sense > to consider the destination format as independent. +1 for keeping it separate. > > * Custom Huffman tables (DC and AC), both for luma and chroma > > It seems that there is a default table when no Huffman table is provided > in the bitstream (I'm not too sure how standard that is, just started > learning about JPEG). We probably need a specific control for that. What happens if there is one in the bitstream? Would the hardware pick it automatically? I think it might make sense to just have a general control for Huffman table, which would be always provided by the user space, regardless of whether it's parsed from the stream or default, so that drivers don't have to care and could just always use it. Best regards, Tomasz
Hi, Le mercredi 19 septembre 2018 à 13:28 +0900, Tomasz Figa a écrit : > On Thu, Sep 13, 2018 at 9:15 PM Paul Kocialkowski <contact@paulk.fr> wrote: > > Hi, > > > > On Wed, 2018-09-05 at 19:00 -0300, Ezequiel Garcia wrote: > > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > > configure the JPEG quantization tables. > > > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > > --- > > > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > > > .../media/videodev2.h.rst.exceptions | 1 + > > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > > > include/uapi/linux/v4l2-controls.h | 12 +++++++ > > > include/uapi/linux/videodev2.h | 1 + > > > 5 files changed, 55 insertions(+) > > > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > > index 9f7312bf3365..1335d27d30f3 100644 > > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > > @@ -3354,7 +3354,38 @@ JPEG Control IDs > > > Specify which JPEG markers are included in compressed stream. This > > > control is valid only for encoders. > > > > > > +.. _jpeg-quant-tables-control: > > > > I just had a look at how the Allwinner VPU handles JPEG decoding and it > > seems to require the following information (in addition to > > quantization): > > I assume the hardware doesn't have the ability to parse those from the > stream and so they need to be parsed by user space and given to the > driver? That's correct, we are also dealing with a stateless decoder here. It's actually the same hardware engine that's used for MPEG2 decoding, only configured differently. So we will need to introduce a pixfmt for compressed JPEG data without headers, reuse JPEG controls that apply and perhaps introduce new ones too if needed. I am also wondering about how MJPEG support should fit into this. As far as I understood, it shouldn't be very different from JPEG so we might want to have common controls for both. > > * Horizontal and vertical sampling factors for each Y/U/V component: > > > > The number of components and sampling factors are coded separately in > > the bitstream, but it's probably easier to use the already-existing > > V4L2_CID_JPEG_CHROMA_SUBSAMPLING control for specifying that. > > > > However, this is potentially very much related to the destination > > format. If we decide that this format should match the format resulting > > from decompression, we don't need to specify it through an external > > control. On the other hand, it's possible that the VPU has format > > conversion block integrated in its pipeline so it would also make sense > > to consider the destination format as independent. > > +1 for keeping it separate. Just like for the stateless decoding API, it would make sense to expect userspace to set those before enumerating CAPTURE formats in order to determine what the hardware can output. > > * Custom Huffman tables (DC and AC), both for luma and chroma > > > > It seems that there is a default table when no Huffman table is provided > > in the bitstream (I'm not too sure how standard that is, just started > > learning about JPEG). We probably need a specific control for that. > > What happens if there is one in the bitstream? Would the hardware pick > it automatically? In our case, this part of the bitstream wouldn't be sent to the hardware anyway. > I think it might make sense to just have a general control for Huffman > table, which would be always provided by the user space, regardless of > whether it's parsed from the stream or default, so that drivers don't > have to care and could just always use it. For MPEG-2 support (and probably also H.265), we have considered the quantization tables optional and kept a default value in the driver. That's because said tables are not supported in all profiles, so they are de-facto optional. I think it's fair to consider that userspace does not need to implement more than what is needed for decoding. This makes our interface closer to the data obtained from the bitstream. However, having one copy of the default table per driver is far from optimal. I would suggest moving it to common v4l2 functions instead, but keeping it in kernel space. What do you think? Cheers, Paul
On Fri, 2018-10-12 at 22:00 +0200, Paul Kocialkowski wrote: > Hi, > > Le mercredi 19 septembre 2018 à 13:28 +0900, Tomasz Figa a écrit : > > On Thu, Sep 13, 2018 at 9:15 PM Paul Kocialkowski <contact@paulk.fr> wrote: > > > Hi, > > > > > > On Wed, 2018-09-05 at 19:00 -0300, Ezequiel Garcia wrote: > > > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > > > > > Add V4L2_CID_JPEG_QUANTIZATION compound control to allow userspace > > > > configure the JPEG quantization tables. > > > > > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > > > --- > > > > .../media/uapi/v4l/extended-controls.rst | 31 +++++++++++++++++++ > > > > .../media/videodev2.h.rst.exceptions | 1 + > > > > drivers/media/v4l2-core/v4l2-ctrls.c | 10 ++++++ > > > > include/uapi/linux/v4l2-controls.h | 12 +++++++ > > > > include/uapi/linux/videodev2.h | 1 + > > > > 5 files changed, 55 insertions(+) > > > > > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst > > > > index 9f7312bf3365..1335d27d30f3 100644 > > > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > > > @@ -3354,7 +3354,38 @@ JPEG Control IDs > > > > Specify which JPEG markers are included in compressed stream. This > > > > control is valid only for encoders. > > > > > > > > +.. _jpeg-quant-tables-control: > > > > > > I just had a look at how the Allwinner VPU handles JPEG decoding and it > > > seems to require the following information (in addition to > > > quantization): > > > > I assume the hardware doesn't have the ability to parse those from the > > stream and so they need to be parsed by user space and given to the > > driver? > > That's correct, we are also dealing with a stateless decoder here. It's > actually the same hardware engine that's used for MPEG2 decoding, only > configured differently. > > So we will need to introduce a pixfmt for compressed JPEG data without > headers, reuse JPEG controls that apply and perhaps introduce new ones > too if needed. > > I am also wondering about how MJPEG support should fit into this. As > far as I understood, it shouldn't be very different from JPEG so we > might want to have common controls for both. > We've recently discussed this and we were proposing to just drop MJPEG and stick to JPEG. The reason is that MJPEG is not clearly defined. Note that others treat MJPEG and JPEG as aliases. See "Re: [RFC] V4L2_PIX_FMT_MJPEG vs V4L2_PIX_FMT_JPEG". Also, I'll be adding support for spec-compliant JPEG frames in rockchip JPEG encoder. This will allow to use the driver with already available userspace. IOW, we don't absolutely need a new pixelformat for encoders. Decoders, on the other side, would be a different story, as parsing headers in the kernel can raise some safety concerns. Regards, Ezequiel
diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst index 9f7312bf3365..1335d27d30f3 100644 --- a/Documentation/media/uapi/v4l/extended-controls.rst +++ b/Documentation/media/uapi/v4l/extended-controls.rst @@ -3354,7 +3354,38 @@ 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 :ref:`itu-t81` + specification allows 8-bit quantization coefficients for + baseline profile images, and 8-bit or 16-bit for extended profile + images. Supporting or not 16-bit precision coefficients is driver-specific. + Coefficients must be set in JPEG zigzag scan order. + + +.. 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 + - ``precision`` + - Specifies the coefficient precision. User shall set 0 + for 8-bit, and 1 for 16-bit. + + * - __u16 + - ``luma_quantization_matrix[64]`` + - Sets the luma quantization table. + + * - __u16 + - ``chroma_quantization_matrix[64]`` + - Sets the chroma quantization table. .. flat-table:: :header-rows: 0 diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions index ca9f0edc579e..a0a38e92bf38 100644 --- a/Documentation/media/videodev2.h.rst.exceptions +++ b/Documentation/media/videodev2.h.rst.exceptions @@ -129,6 +129,7 @@ replace symbol V4L2_CTRL_TYPE_STRING :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_U16 :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_U32 :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_U8 :c:type:`v4l2_ctrl_type` +replace symbol V4L2_CTRL_TYPE_JPEG_QUANTIZATION :c:type:`v4l2_ctrl_type` # V4L2 capability defines replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 599c1cbff3b9..305bd7a9b7f1 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -999,6 +999,7 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_JPEG_RESTART_INTERVAL: return "Restart Interval"; case V4L2_CID_JPEG_COMPRESSION_QUALITY: return "Compression Quality"; case V4L2_CID_JPEG_ACTIVE_MARKER: return "Active Markers"; + case V4L2_CID_JPEG_QUANTIZATION: return "JPEG Quantization Tables"; /* Image source controls */ /* Keep the order of the 'case's the same as in v4l2-controls.h! */ @@ -1286,6 +1287,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, case V4L2_CID_DETECT_MD_REGION_GRID: *type = V4L2_CTRL_TYPE_U8; break; + case V4L2_CID_JPEG_QUANTIZATION: + *type = V4L2_CTRL_TYPE_JPEG_QUANTIZATION; + break; case V4L2_CID_DETECT_MD_THRESHOLD_GRID: *type = V4L2_CTRL_TYPE_U16; break; @@ -1612,6 +1616,9 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, return -ERANGE; return 0; + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: + return 0; + default: return -EINVAL; } @@ -2133,6 +2140,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, case V4L2_CTRL_TYPE_U32: elem_size = sizeof(u32); break; + case V4L2_CTRL_TYPE_JPEG_QUANTIZATION: + elem_size = sizeof(struct v4l2_ctrl_jpeg_quantization); + break; default: if (type < V4L2_CTRL_COMPOUND_TYPES) elem_size = sizeof(s32); diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index e4ee10ee917d..856b3325052f 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -987,6 +987,18 @@ enum v4l2_jpeg_chroma_subsampling { #define V4L2_JPEG_ACTIVE_MARKER_DQT (1 << 17) #define V4L2_JPEG_ACTIVE_MARKER_DHT (1 << 18) +#define V4L2_CID_JPEG_QUANTIZATION (V4L2_CID_JPEG_CLASS_BASE + 5) +struct v4l2_ctrl_jpeg_quantization { + /* ITU-T.81 specifies two quantization coefficient precisions: + * 8-bit for baseline profile, + * 8-bit or 16-bit for extended profile. + * + * User shall set "precision" to 0 for 8-bit and 1 for 16-bit. + */ + __u8 precision; + __u16 luma_quantization_matrix[64]; + __u16 chroma_quantization_matrix[64]; +}; /* Image source controls */ #define V4L2_CID_IMAGE_SOURCE_CLASS_BASE (V4L2_CTRL_CLASS_IMAGE_SOURCE | 0x900) diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index f9f3ae5b489e..8ace47cb1003 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1637,6 +1637,7 @@ enum v4l2_ctrl_type { V4L2_CTRL_TYPE_U8 = 0x0100, V4L2_CTRL_TYPE_U16 = 0x0101, V4L2_CTRL_TYPE_U32 = 0x0102, + V4L2_CTRL_TYPE_JPEG_QUANTIZATION = 0x0103, }; /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */