[libcamera-devel] JPEG support for libcamera

Wed Jul 6 23:34:15 CEST 2022

Hi Eric,

On Wed, Jul 06, 2022 at 10:21:26PM +0100, Eric Curtin wrote:
> This patch looks good to me.
> 
> In a related area I was thinking of removing SDL2_image (while keeping
> SDL2 main library as a dependancy) and replacing it with libjpeg to
> keep our dependency count low. We were only really using SDL2_image as
> a wrapper for libjpeg (and using no other SDL2_image functionality).
> We already have libjpeg as an optional dependency in some of the
> Android code, do like this instead basically (it's what SDL2_image is
> doing under the covers anyway):
> 
> https://github.com/ericcurtin/twincam/blob/main/src/sdl_texture_mjpg.cpp
> 
> SDL2_image isn't as readily available either, it's not in CentOS
> Stream 9 for example.
> 
> We don't lose any functionality this way.

No objection in principle. Just be careful that the libjpeg API isn't
the most straightforward to use, it can be quite cumbersome, especially
when it comes to error handling.

> We could re-use this decompressor in kms_sink potentially also,
> although we'd still have to do some pixel format conversion after that
> to have decent MJPEG and JPEG support in kms_sink.
> 
> Is mise le meas/Regards,
> 
> Eric Curtin
> 
> 
> 
> On Tue, 5 Jul 2022 at 21:34, Laurent Pinchart via libcamera-devel
> <libcamera-devel at lists.libcamera.org> wrote:
> >
> > On Tue, Jul 05, 2022 at 08:49:53PM +0200, Jacopo Mondi via libcamera-devel wrote:
> > > Hi Pavel, Dave, Nicolas,
> > >
> > >    thank you all for the clarification. I'll review Pavel's patch with
> > > the intent of mapping both JPEG and MJPEG to a single
> > > application visible DRM_FORMAT_MJPEG
> >
> > Sounds good to me. This mess should eventually be fixed in V4L2, until
> > then, keeping a single libcamera pixel format is best to avoid
> > propagating the problem to applications.
> >
> > > On Tue, Jul 05, 2022 at 01:39:02PM -0400, Nicolas Dufresne via libcamera-devel wrote:
> > > > Le mardi 05 juillet 2022 à 16:33 +0100, Dave Stevenson via libcamera-devel a écrit :
> > > > > On Tue, 5 Jul 2022 at 15:48, Pavel Machek via libcamera-devel wrote:
> > > > > >
> > > > > > Hi!
> > > > > >
> > > > > > > Hi Pavel,
> > > > > > >    I discussed it with Rafael (see message id
> > > > > > > <20220701075644.mcjbcwthzsofmssx at uno.localdomain>) and my take was
> > > > > > > slightly different: the driver should be fixed to report MJPEG:
> > > > > > >
> > > > > > > Let me re-paste it here, mostly to validate my understanding as I
> > > > > > > might have got this very wrong.
> > > > > > >
> > > > > > > -------------------------------------------------------------------------------
> > > > > > >
> > > > > > > Libcamera only supports:
> > > > > > > #define V4L2_PIX_FMT_MJPEG    v4l2_fourcc('M', 'J', 'P', 'G') /* Motion-JPEG   */
> > > > > > >
> > > > > > > I admit I know nothing about JPEG related standards but looking at the
> > > > > > > media drivers, it seems FMT_JPEG is used more often than MJPEG.
> > > > > > >
> > > > > > > However libcamera only seems to support MJPEG. You might have noticed
> > > > > > > libcamera uses the DRM pixelformat identifiers, and it seems that in
> > > > > > > the drm_fourcc.h header we ship there's no JPEG format at all, but
> > > > > > > only MJPEG. If my understanding is correct this makes sense, as the
> > > > > > > MJPEG standard defines a video container format for streaming
> > > > > > > JPEG-compressed frames while JFIF JPEG defines a file-encoding scheme
> > > > > > > for exchanging single images. Being DRM about video output devices
> > > > > > > only MJPEG applies there, so there's no "JIF/JFIF JPEG" format in
> > > > > > > there.
> > > > > > > Now, I would not be surprised if the multiple users of FMT_JPEG in
> > > > > > > drivers/media/ are actually sending MJPEG but driver developers pick
> > > > > > > FMT_JPEG because of some form of cargo-cult.
> > > > > >
> > > > > > Best explanation I found so far was that JPEG = EXIF + MJPEG. Kernel
> > > > > > interface exposes JPEG and MJPEG as separate types, so IMO it makes
> > > > > > sense for libcamera to support both.
> > > > >
> > > > > TBH JPEG at least has a specification. MJPEG largely doesn't - it's
> > > > > just a concatenation of JPEG encoded frames, and there seems to be no
> > > > > real definition of which JPEG blocks are required and in which order.
> > > > > Some MJPEG encoders drop the Huffman tables and rely on the decoder
> > > > > assuming defaults.
> > > > > There are then also MJPEG-A and MJPEG-B variants defined by Apple as
> > > > > part of the Quicktime spec [1], with MJPEG-B not being compatible with
> > > > > JPEG decoders due to changing headers.
> > > > >
> > > > > Which flavour V4L2_PIX_FMT_MJPEG relates to seems to be undefined.
> > > > >
> > > > > I've had issues in the past with what conditions VLC judges to be JPEG
> > > > > vs MJPEG, and never found a definitive answer [2]
> > > > >
> > > > > So how Libcamera should handle JPEG vs MJPEG probably doesn't have an
> > > > > obvious answer. There's little point in encoding EXIF in a hardware
> > > > > encoder as you have the overhead of passing all the parameters in.
> > > > > For mapping into V4L2, just treat both of them the same.
> > > >
> > > > Older C920 could be setup to produce jpeg, which embeds H.264 into the EXIF
> > > > header. You'll find support for that in GStreamer [0]. Probably not relevant to
> > > > the discussion. In GStreamer v4l2src [1], all form of jpeg is assumed to be the
> > > > same. Which made me notice that V4L2_PIX_FMT_PJPG not mentionned here. I have no
> > > > idea what this refer too, this special case [1] dates from before me. I think
> > > > kernel driver encoders today will in software append the missing table/headers
> > > > as needed.
> > > >
> > > > [0] https://gitlab.freedesktop.org/gstreamer/gstreamer/-/tree/main/subprojects/gst-plugins-bad/sys/uvch264
> > > > [1] https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/gst-plugins-good/sys/v4l2/gstv4l2object.c#L965
> > > > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/chips-media/coda-jpeg.c#n215
> > > >
> > > > > [1] https://developer.apple.com/standards/qtff-2001.pdf page 95.
> > > > > [2] https://github.com/raspberrypi/userland/issues/345
> > > > >
> > > > > > > To remove the above error I would change your driver to report
> > > > > > > FMT_MJPEG for the time being until the above doesn't get clarified.
> > > > > >
> > > > > > I'm not really able to touch the kernel at the moment. Plus it is not
> > > > > > "my" driver, it is driver in upstream kernel...
> > > > > >
> > > > > > If one of the formats is redundant and should be dropped, I guess we
> > > > > > should have that discussion on the v4l2/kernel mailing lists...

-- 
Regards,

Laurent Pinchart