Re: [PATCH v7 4/6] media: Add JPEG_RAW format

2018-10-05 Thread Mauro Carvalho Chehab
Em Fri, 05 Oct 2018 08:53:14 -0300
Ezequiel Garcia  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  escreveu:
> >   
> > > From: Shunqian Zheng 
> > > 
> > > Add V4L2_PIX_FMT_JPEG_RAW format that does not contain
> > > JPEG header in the output frame.
> > > 
> > > Signed-off-by: Shunqian Zheng 
> > > Signed-off-by: Ezequiel Garcia 
> > > ---
> > >  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 `,
> > >   :ref:`VIDIOC_S_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


Re: [PATCH v7 4/6] media: Add JPEG_RAW format

2018-10-05 Thread Ezequiel Garcia
On Fri, 2018-10-05 at 08:09 -0300, Mauro Carvalho Chehab wrote:
> Em Thu,  4 Oct 2018 21:12:24 -0300
> Ezequiel Garcia  escreveu:
> 
> > From: Shunqian Zheng 
> > 
> > Add V4L2_PIX_FMT_JPEG_RAW format that does not contain
> > JPEG header in the output frame.
> > 
> > Signed-off-by: Shunqian Zheng 
> > Signed-off-by: Ezequiel Garcia 
> > ---
> >  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 `,
> > :ref:`VIDIOC_S_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


Re: [PATCH v7 4/6] media: Add JPEG_RAW format

2018-10-05 Thread Mauro Carvalho Chehab
Em Thu,  4 Oct 2018 21:12:24 -0300
Ezequiel Garcia  escreveu:

> From: Shunqian Zheng 
> 
> Add V4L2_PIX_FMT_JPEG_RAW format that does not contain
> JPEG header in the output frame.
> 
> Signed-off-by: Shunqian Zheng 
> Signed-off-by: Ezequiel Garcia 
> ---
>  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 `,
>   :ref:`VIDIOC_S_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


[PATCH v7 4/6] media: Add JPEG_RAW format

2018-10-04 Thread Ezequiel Garcia
From: Shunqian Zheng 

Add V4L2_PIX_FMT_JPEG_RAW format that does not contain
JPEG header in the output frame.

Signed-off-by: Shunqian Zheng 
Signed-off-by: Ezequiel Garcia 
---
 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 `,
:ref:`VIDIOC_S_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_MJPEGv4l2_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 */
-- 
2.19.0.rc2