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

[Umang Jain]

Hi Laurent,

On 8/27/20 11:14 PM, Laurent Pinchart wrote:
> Hi Umang,
>
> On Thu, Aug 27, 2020 at 05:53:54AM +0000, Umang Jain wrote:
>> On 8/25/20 2:58 AM, Kieran Bingham wrote:
>>> 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 ?
>>>
>>>> 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 ;-)
>>>
>>>> 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.
>> I found out that, this is not an error. The error messsage however, is
>> misleading.
>> https://github.com/Aries85/LightZone/issues/171#issuecomment-195730879
>>
>> Color Space 65535 => Uncalibrated.
>> We are running exif_data_fix(data_); in exif's constructor, so I think it's where
>> this "Uncalibrated" value is getting set as the default value for color space.
>>
>> Can we query colorspace as of now? Then we can try setting it here as well.
> See src/qcam/dng_writer.cpp, we record some colorspace-related
> information there.
Ah thanks for the pointer.
However, it seems the values are somewhat implied and hardcoded for
the TIFF_PHOTOMETRIC, which I suppose is the tag for conveying
color space information.  PHOTOMETRIC_RGB for the thumbnail and
PHOTOMETRIC_CFA for the raw image.

However, I just come across std::map<PixelFormat, JPEGPixelFormatInfo>
which wraps colorspace information in src/android/jpeg/encoder_libjpeg.cpp
Kieran, can we somehow use it in order to set the ColorSpace? The problem
here is we are setting the tags one level up (in 
src/android/CameraDevice.cpp)
so we might to relay the information at that point. What do you think?

Also, I hope it can be follow up patch, not necessarily blocking this 
patchset
:-)
>
>>>> 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.
>>>
>>>> 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.
>>>
>>>> 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