[libcamera-devel] [PATCH] android: exif: Contain IMAGE_WIDTH and IMAGE_LENGTH data

[Hirokazu Honda]

Hi,

On Thu, Mar 25, 2021 at 3:18 PM Hirokazu Honda <hiroh at chromium.org> wrote:
>
> Hi Umang, thanks for reviewing,
>
> On Tue, Mar 23, 2021 at 6:04 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
> >
> > Hi Hiro,
> >
> > On 3/23/21 1:32 PM, Hirokazu Honda wrote:
> > > ChromeOS camera test checks if exif data has the IMAGE_WIDTH and
> > > IMAGE_LENGTH and they are the same as the requested jpeg size.
> > > This adds the resolution data to exif.
> > >
> > > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > >
> > > ---
> > >   src/android/jpeg/exif.cpp | 2 ++
> > >   1 file changed, 2 insertions(+)
> > >
> > > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> > > index 922086cd..29c7be0f 100644
> > > --- a/src/android/jpeg/exif.cpp
> > > +++ b/src/android/jpeg/exif.cpp
> > > @@ -286,6 +286,8 @@ void Exif::setModel(const std::string &model)
> > >
> > >   void Exif::setSize(const Size &size)
> > >   {
> > > +     setLong(EXIF_IFD_0, EXIF_TAG_IMAGE_LENGTH, size.height);
> > > +     setLong(EXIF_IFD_0, EXIF_TAG_IMAGE_WIDTH, size.width);
> > I am reading the EXIF spec and for ImageLength and ImageWidth, it states:
> >
> > ```
> > ImageWidth
> > The number of columns of image data, equal to the number of pixels per
> > row. In JPEG compressed
> > data, this tag shall not be used because a JPEG marker is used instead
> > of it.
> > ```
> >
> > Same for ImageLength.
> >
> > We compress the image using JPEG post-processor right? Hence, I think
> > these tags shouldn't be applicable (as per spec's point of view).
> >
>
> I think you're right.
> Our test code [1] seems to come from Android CTS test code [2].
> I am asking Android camera people if the check is correct.
> I will update here their response.
>
> [1] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform2/camera/camera3_test/camera3_exif_validator.cc;l=337;drc=82fcae1960c0283214607107037c747c0e545617
> [2] https://cs.android.com/android/platform/superproject/+/master:cts/tests/camera/utils/src/android/hardware/camera2/cts/CameraTestUtils.java;l=2669;drc=556fa09af46eeb2e8abe418d0f049c154bc4181a
>

I found, if there is no exif tags of IMAGE_WIDTH and IMAGE_LENGTH,
Android sets them to ones of JPEG markers. [1]
It is because some CTS tests that checks the exif tags passed albeit
we don't set the tags.
I reached out Android developers about the tests and the code.
They agree the EXIF spec states these tags are not needed.
However, they said it was pretty common that these tags exist in pract
and their CTS tries to make sure common tags are included in EXIF.
So they are reluctant to change it and apps may be expecting them to
exif on JPEGS from Android devices now as the code exists since 2016.
FWIW, CDD doesn't include required EXIF tags unfortunately.

There are two solutions for us.
1.) Don't add these tags in libcamera
This is correct in terms of EXIF spec.
Even if we don't set the tags, they are set by Android to ones of JPEG markers.
The problem is that chromeos camera test fails due to the wrong check
[2]. We remove the checks on this solution.

2.) Add these tags in libcamera
Android and chromeos camera tests will pass.
Although this is wrong in terms of EXIF spec, it might be practical to
do this because some Android HAL clients (wrongly) expect the
behavior.

How do you think?

[1] https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/ExifInterface.java;l=2907
[2] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform2/camera/camera3_test/camera3_exif_validator.cc;l=337;drc=82fcae1960c0283214607107037c747c0e545617

Best Regards,
-Hiro
> Regards,
> -Hiro
>
> > >       setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_Y_DIMENSION, size.height);
> > >       setLong(EXIF_IFD_EXIF, EXIF_TAG_PIXEL_X_DIMENSION, size.width);
> > >   }
> > > --
> > > 2.31.0.rc2.261.g7f71774620-goog
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> > --
> > Regards,
> > Umang Jain
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel