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

Fri Aug 28 10:32:11 CEST 2020

Hi Umang,

On 28/08/2020 07:26, Umang Jain wrote:
> 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
> :-)

Yes, this can be a follow on from my perspective.

However it might not need any further action:

Looking at:

https://www.jeita.or.jp/japanese/standard/book/CP-3451E_E/#target/page_no=49
(Section 4.6.5-B)

states the following:

"""
The color space information tag (ColorSpace) is always recorded as the
color space specifier. Normally sRGB (=1) is used to define the color
space based on the PC monitor conditions and environment. If a color
space other than sRGB is used, Uncalibrated (=FFFF.H) is set.
"""

The question I don't yet know the answer to (color space is very
complex, and possibly dependant upon the connected sensor attributes) is
if we can assume we're in sRGB even though we are using YUV.

Perhaps taking a look at the CrOS code to see what it does would be
useful, but otherwise, I think we can leave this tag for now.

>>>>> 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

-- 
Regards
--
Kieran