| Message ID | 20181005001226.12789-5-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Rockchip VPU JPEG encoder | expand |
Em Thu, 4 Oct 2018 21:12:24 -0300 Ezequiel Garcia <ezequiel@collabora.com> escreveu: > From: Shunqian Zheng <zhengsq@rock-chips.com> > > Add V4L2_PIX_FMT_JPEG_RAW format that does not contain > JPEG header in the output frame. > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > Documentation/media/uapi/v4l/pixfmt-compressed.rst | 9 +++++++++ > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/uapi/linux/videodev2.h | 1 + > 3 files changed, 11 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst > index ba0f6c49d9bf..ad73076276ec 100644 > --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst > +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst > @@ -23,6 +23,15 @@ Compressed Formats > - 'JPEG' > - TBD. See also :ref:`VIDIOC_G_JPEGCOMP <VIDIOC_G_JPEGCOMP>`, > :ref:`VIDIOC_S_JPEGCOMP <VIDIOC_G_JPEGCOMP>`. > + * .. _V4L2-PIX-FMT-JPEG-RAW: > + > + - ``V4L2_PIX_FMT_JPEG_RAW`` > + - 'Raw JPEG' > + - Raw JPEG bitstream, containing a compressed payload. This format > + contains an image scan, i.e. without any metadata or headers. > + The user is expected to set the needed metadata such as > + quantization and entropy encoding tables, via ``V4L2_CID_JPEG`` > + controls, see :ref:`jpeg-control-id`. IMO, it is not very clear when someone should use V4L2_CID_JPEG or V4L2_PIX_FMT_JPEG_RAW. Some drivers do add a JPEG header internally. Also, if we're now starting to accept headerless JPEG images, you should very patch libv4l as well, in order to accept this new format. Regards, Thanks, Mauro
On Fri, 2018-10-05 at 08:09 -0300, Mauro Carvalho Chehab wrote: > Em Thu, 4 Oct 2018 21:12:24 -0300 > Ezequiel Garcia <ezequiel@collabora.com> escreveu: > > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > Add V4L2_PIX_FMT_JPEG_RAW format that does not contain > > JPEG header in the output frame. > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > Documentation/media/uapi/v4l/pixfmt-compressed.rst | 9 +++++++++ > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > include/uapi/linux/videodev2.h | 1 + > > 3 files changed, 11 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst > > index ba0f6c49d9bf..ad73076276ec 100644 > > --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst > > +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst > > @@ -23,6 +23,15 @@ Compressed Formats > > - 'JPEG' > > - TBD. See also :ref:`VIDIOC_G_JPEGCOMP <VIDIOC_G_JPEGCOMP>`, > > :ref:`VIDIOC_S_JPEGCOMP <VIDIOC_G_JPEGCOMP>`. > > + * .. _V4L2-PIX-FMT-JPEG-RAW: > > + > > + - ``V4L2_PIX_FMT_JPEG_RAW`` > > + - 'Raw JPEG' > > + - Raw JPEG bitstream, containing a compressed payload. This format > > + contains an image scan, i.e. without any metadata or headers. > > + The user is expected to set the needed metadata such as > > + quantization and entropy encoding tables, via ``V4L2_CID_JPEG`` > > + controls, see :ref:`jpeg-control-id`. > > IMO, it is not very clear when someone should use V4L2_CID_JPEG or > V4L2_PIX_FMT_JPEG_RAW. Some drivers do add a JPEG header internally. > For device drivers, if the hardware can parse JPEG frames, then V4L2_PIX_FMT_JPEG should be used. Otherwise, if the hardware can only accept a parsed JPEG (i.e. payload and tables), then only V4L2_PIX_FMT_JPEG_RAW should be supported. Parsing headers in the driver is discouraged by the stateful codec API specification. With the Request API in place, and the stateful and stateless specs, device driver writers should be now using the right model for each type of hardware. There are exceptions, though. If the hardware handles full JPEG frames, but requires some extra parsing on the OS side, then the driver should be using V4L2_PIX_FMT_JPEG, and doing some extra parsing. For instance, mtk-jpeg seems to work like this. > Also, if we're now starting to accept headerless JPEG images, you should > very patch libv4l as well, in order to accept this new format. > Right. Thanks, Eze
Em Fri, 05 Oct 2018 08:53:14 -0300 Ezequiel Garcia <ezequiel@collabora.com> escreveu: > On Fri, 2018-10-05 at 08:09 -0300, Mauro Carvalho Chehab wrote: > > Em Thu, 4 Oct 2018 21:12:24 -0300 > > Ezequiel Garcia <ezequiel@collabora.com> escreveu: > > > > > From: Shunqian Zheng <zhengsq@rock-chips.com> > > > > > > Add V4L2_PIX_FMT_JPEG_RAW format that does not contain > > > JPEG header in the output frame. > > > > > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > > --- > > > Documentation/media/uapi/v4l/pixfmt-compressed.rst | 9 +++++++++ > > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > > include/uapi/linux/videodev2.h | 1 + > > > 3 files changed, 11 insertions(+) > > > > > > diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst > > > index ba0f6c49d9bf..ad73076276ec 100644 > > > --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst > > > +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst > > > @@ -23,6 +23,15 @@ Compressed Formats > > > - 'JPEG' > > > - TBD. See also :ref:`VIDIOC_G_JPEGCOMP <VIDIOC_G_JPEGCOMP>`, > > > :ref:`VIDIOC_S_JPEGCOMP <VIDIOC_G_JPEGCOMP>`. > > > + * .. _V4L2-PIX-FMT-JPEG-RAW: > > > + > > > + - ``V4L2_PIX_FMT_JPEG_RAW`` > > > + - 'Raw JPEG' > > > + - Raw JPEG bitstream, containing a compressed payload. This format > > > + contains an image scan, i.e. without any metadata or headers. > > > + The user is expected to set the needed metadata such as > > > + quantization and entropy encoding tables, via ``V4L2_CID_JPEG`` > > > + controls, see :ref:`jpeg-control-id`. > > > > IMO, it is not very clear when someone should use V4L2_CID_JPEG or > > V4L2_PIX_FMT_JPEG_RAW. Some drivers do add a JPEG header internally. > > > > For device drivers, if the hardware can parse JPEG frames, then > V4L2_PIX_FMT_JPEG should be used. Otherwise, if the hardware can > only accept a parsed JPEG (i.e. payload and tables), then > only V4L2_PIX_FMT_JPEG_RAW should be supported. It seems you're talking here about V4L2 output devices. I'm more concerned about its usage on V4L2 capture devices. Yet, it should be possible to use it with both, as formats should be generic enough to work on both directions. > Parsing headers in the driver is discouraged by the stateful codec > API specification. True, but we do parsing/adding headers for some formats in Kernel space, as this is a way to abstract the hardware for applications. That's specially true for MPEG and JPEG formats. > With the Request API in place, and the stateful and stateless specs, > device driver writers should be now using the right model for each > type of hardware. > > There are exceptions, though. If the hardware handles full JPEG frames, > but requires some extra parsing on the OS side, then the driver > should be using V4L2_PIX_FMT_JPEG, and doing some extra parsing. > For instance, mtk-jpeg seems to work like this. Actually, all existing drivers do this. I'm not saying that this the right/best model, but it is the way it is. Changing it is possible, but only if it can be done in a driver-independent way and userspace code inside a library exists to support it. > > > Also, if we're now starting to accept headerless JPEG images, you should > > very patch libv4l as well, in order to accept this new format. > > > > Right. > > Thanks, > Eze Thanks, Mauro
diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst index ba0f6c49d9bf..ad73076276ec 100644 --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst @@ -23,6 +23,15 @@ Compressed Formats - 'JPEG' - TBD. See also :ref:`VIDIOC_G_JPEGCOMP <VIDIOC_G_JPEGCOMP>`, :ref:`VIDIOC_S_JPEGCOMP <VIDIOC_G_JPEGCOMP>`. + * .. _V4L2-PIX-FMT-JPEG-RAW: + + - ``V4L2_PIX_FMT_JPEG_RAW`` + - 'Raw JPEG' + - Raw JPEG bitstream, containing a compressed payload. This format + contains an image scan, i.e. without any metadata or headers. + The user is expected to set the needed metadata such as + quantization and entropy encoding tables, via ``V4L2_CID_JPEG`` + controls, see :ref:`jpeg-control-id`. * .. _V4L2-PIX-FMT-MPEG: - ``V4L2_PIX_FMT_MPEG`` diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index c148c44caffb..5eab42277f47 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1301,6 +1301,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) /* Max description length mask: descr = "0123456789012345678901234567890" */ case V4L2_PIX_FMT_MJPEG: descr = "Motion-JPEG"; break; case V4L2_PIX_FMT_JPEG: descr = "JFIF JPEG"; break; + case V4L2_PIX_FMT_JPEG_RAW: descr = "Raw JPEG"; break; case V4L2_PIX_FMT_DV: descr = "1394"; break; case V4L2_PIX_FMT_MPEG: descr = "MPEG-1/2/4"; break; case V4L2_PIX_FMT_H264: descr = "H.264"; break; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 7412a255d9ce..1c7b5df4eb83 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -627,6 +627,7 @@ struct v4l2_pix_format { /* compressed formats */ #define V4L2_PIX_FMT_MJPEG v4l2_fourcc('M', 'J', 'P', 'G') /* Motion-JPEG */ #define V4L2_PIX_FMT_JPEG v4l2_fourcc('J', 'P', 'E', 'G') /* JFIF JPEG */ +#define V4L2_PIX_FMT_JPEG_RAW v4l2_fourcc('J', 'P', 'G', 'R') /* JFIF JPEG RAW without headers */ #define V4L2_PIX_FMT_DV v4l2_fourcc('d', 'v', 's', 'd') /* 1394 */ #define V4L2_PIX_FMT_MPEG v4l2_fourcc('M', 'P', 'E', 'G') /* MPEG-1/2/4 Multiplexed */ #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */