[libcamera-devel] [PATCH 0/3] Initial EXIF metadata support

[Laurent Pinchart]

Hi Umang,

On Tue, Aug 25, 2020 at 08:16:44PM +0000, Umang Jain wrote:
> Hi Kieran,
> 
> Thanks for the review. Almost all of the comments from this and other
> patches review should be addressed now in v2.
> 
> On 8/25/20 2:58 AM, Kieran Bingham wrote:
> > Hi Umang,
> >
> > Thanks for taking this on.
> >
> > On 24/08/2020 21:46, Umang Jain wrote:
> >> Hi all,
> >>
> >> This patch series targets adding initial EXIF metadata support
> >> for the frames captured in Android/Libcamera framework.
> >>
> >> It is initial in the sense, it adds the base infrastructure for
> >> Exif metadata support via 'libexif' and also encodes a initial
> >> set of metadata tags into the frames.
> >>
> >> Testing:
> >> The testing of this work was carried out via vivid pipeline-handler
> >> in conjunction with patch in `cam` using EncoderLibJpeg as the
> >> file sink [1]. That way we could see what EXIF metadata could be written
> >> and inspect that `exif` or `exiv2` commandline tools.
> >>
> >> ($) ./build/src/cam/cam -c vivid -C5 -F -s pixelformat=NV12
> >>
> >> Sample output (1):  exif  frame-stream0-000000.jpg
> >> EXIF tags in 'frame-stream0-000000.jpg' ('Intel' byte order):
> >> --------------------+----------------------------------------------------------
> >> Tag                 |Value
> >> --------------------+----------------------------------------------------------
> >> Image Width         |1280
> >> Image Length        |720
> >> Manufacturer        |Libcamera
> >
> > Looking at my branch, it was of course me who put this in (incorrectly)
> > with a capital L, but please make it lowercase ;-)
> >
> >> Model               |Camera2111
> >
> > I assume you set this Camera2111 string manually for testing ?
> 
> Yes, hardcoded in cam's frame_sink_compressor.cpp
> 
> >> Orientation         |Bottom-left
> >> X-Resolution        |72
> >> Y-Resolution        |72
> >> Resolution Unit     |Inch
> >> Date and Time       |Sun Oct 13 07:03:20 734137
> >>
> >> YCbCr Positioning   |Centered
> >> Exif Version        |Exif Version 2.1
> >> Date and Time (Origi|Sun Oct 13 07:03:20 734137
> >>
> >> Date and Time (Digit|Sun Oct 13 07:03:20 734137
> >
> > Hrm ... we might have some work to do on converting the timestamp values
> > appropriately here. That doesn't look right at all ;-)
>
> We now use, 'time of encode' as the frame's timestamp as discussed over IRC.
>
> >> Components Configura|Y Cb Cr -
> >> FlashPixVersion     |FlashPix Version 1.0
> >> Color Space         |Internal error (unknown value 65535)
> >
> > This one needs to be checked if there is an error/incorrect parameter set.
>
> This one still needs looking. I will queue it up.
>
> >> Pixel X Dimension   |1280
> >> Pixel Y Dimension   |720
> >>
> >>
> >>
> >> Sample output (2): exiv2  frame-stream0-000000.jpg
> >> File name       : frame-stream0-000000.jpg
> >> File size       : 38787 Bytes
> >> MIME type       : image/jpeg
> >> Image size      : 1280 x 720
> >> Camera make     : Libcamera
> >> Camera model    : Camera2111
> >> Image timestamp : Sun Oct 13 07:03:20 734137
> >> Image number    :
> >
> > Hrm, I wonder if we should put the sequence number in here.
>
> No idea, seems sane though. :)

That doesn't seem right to me. What is this tag ? I've grepped "Image
number" in the exiv2 sources, and the only occurrences are related to
tags specific to Minolta and Canon.

> >> Exposure time   :
> >> Aperture        :
> >> Exposure bias   :
> >> Flash           :
> >> Flash bias      :
> >> Focal length    :
> >> Subject distance:
> >> ISO speed       :
> >> Exposure mode   :
> >> Metering mode   :
> >> Macro mode      :
> >> Image quality   :
> >
> > Plenty of parameters we'll want to get from the IPAs there, but that
> > will be later.
>
> Indeed, I intentionally attached exiv2's output to see where we stand
> and what should be the so called 'next fields' to populate.
> 
> We can build it on top of this
>
> >> Exif Resolution : 1280 x 720
> >> White balance   :
> >> Thumbnail       : None
> >> Copyright       :
> >> Exif comment    :
> >>
> >> There are still a lot of tags that needs to be populated,
> >> that can be built on top of this groundwork.
> >>
> >> [1]: https://github.com/kbingham/libcamera/commit/10ed8ec
> >>
> >> Kieran Bingham (1):
> >>    libcamera: android: Add EXIF infrastructure
> >>
> >> Umang Jain (2):
> >>    android: jpeg: Pass a Exif object while encoding
> >>    android: Support initial set of EXIF metadata tags
> >>
> >>   src/android/camera_device.cpp        |  12 +-
> >>   src/android/jpeg/encoder.h           |   5 +-
> >>   src/android/jpeg/encoder_libjpeg.cpp |  15 +-
> >>   src/android/jpeg/encoder_libjpeg.h   |   3 +-
> >>   src/android/jpeg/exif.cpp            | 210 +++++++++++++++++++++++++++
> >>   src/android/jpeg/exif.h              |  53 +++++++
> >>   src/android/meson.build              |   2 +
> >>   7 files changed, 296 insertions(+), 4 deletions(-)
> >>   create mode 100644 src/android/jpeg/exif.cpp
> >>   create mode 100644 src/android/jpeg/exif.h

-- 
Regards,

Laurent Pinchart