[libcamera-devel] [PATCH v3] android: jpeg: get ISO from SENSOR_SENSITIVITY

[paul.elder at ideasonboard.com]

Hi Hiro,

On Thu, Jun 17, 2021 at 02:47:55PM +0900, Hirokazu Honda wrote:
> Hi Paul,
> 
> On Thu, Jun 17, 2021 at 2:44 PM <paul.elder at ideasonboard.com> wrote:
> 
>     Hi Hiro,
> 
>     Thank you for the review.
> 
>     On Thu, Jun 17, 2021 at 02:31:11PM +0900, Hirokazu Honda wrote:
>     > HI Paul, thank you for the patch.
>     >
>     > On Thu, Jun 17, 2021 at 1:58 PM Paul Elder <paul.elder at ideasonboard.com>
>     wrote:
>     >
>     >     The data for the exif ISO tag needs to come from SENSOR_SENSITIVITY.
>     Set
>     >     it.
>     >
>     >     Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>     >     Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>     >
>     >     ---
>     >     This on its own doesn't fix any CTS tests, but it prevents a test
>     >     failure later on when we add the proper static metadata for the FULL
>     >     hardware level.
>     >     ---
>     >      src/android/jpeg/post_processor_jpeg.cpp | 5 ++++-
>     >      1 file changed, 4 insertions(+), 1 deletion(-)
>     >
>     >     diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/
>     jpeg/
>     >     post_processor_jpeg.cpp
>     >     index 058ccc99..921ac823 100644
>     >     --- a/src/android/jpeg/post_processor_jpeg.cpp
>     >     +++ b/src/android/jpeg/post_processor_jpeg.cpp
>     >     @@ -119,7 +119,10 @@ int PostProcessorJpeg::process(const FrameBuffer
>     &
>     >     source,
>     >             ret = requestMetadata.getEntry(ANDROID_LENS_APERTURE, &
>     entry);
>     >             if (ret)
>     >                     exif.setAperture(*entry.data.f);
>     >     -       exif.setISO(100);
>     >     +
>     >     +       ret = resultMetadata->getEntry(ANDROID_SENSOR_SENSITIVITY, &
>     entry);
>     >     +       exif.setISO(ret ? *entry.data.i32 : 100);
>     >
>     >
>     > Noob: where does 100 come from?
> 
>     It was an arbitrary number that passed CTS LIMITED :D
> 
> 
> 
> I see.
> Shall we add a comment about it?

Yeah we probably should.

Also I checked the EXIF documentation [1] and it says this field is for
"Indicates the ISO Speed and ISO Latitude of the camera or input device
as specified in ISO 12232", so that doesn't seem to match with
SENSOR_SENSITIVITY [2]...

But also this allowed CTS FULL to pass when I tested it a long time
ago... I'll have to check again.

[1] https://www.exif.org/Exif2-2.PDF
[2] https://developer.android.com/reference/android/hardware/camera2/CaptureRequest#SENSOR_SENSITIVITY


Paul

> 
>     >
>     > Reviewed-by: Hirokazu Honda <hiroh at chromium.org> 
>     >
>     >     +
>     >             exif.setFlash(Exif::Flash::FlashNotPresent);
>     >             exif.setWhiteBalance(Exif::WhiteBalance::Auto);
>     >
>     >     --
>     >     2.27.0
>     >
>     >
>