[libcamera-devel] JPEG support for libcamera

Wed Jul 6 23:21:26 CEST 2022

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.

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
>