[libcamera-devel] [PATCH 3/6] android: jpeg: exif: Fix setOrientation EXIF values

[paul.elder at ideasonboard.com]

Hi Laurent,

On Fri, Jan 15, 2021 at 07:27:54PM +0200, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Thu, Jan 14, 2021 at 07:40:32PM +0900, Paul Elder wrote:
> > The input to setOrientation is angle clockwise from 12 o'clock, while
> > the EXIF output values were swapped for 90 and 270 degrees.
> > 
> > From the EXIF spec:
> > 
> > 6 = The 0th row is the visual right-hand side of the image, and the
> >     0th column is the visual top.
> > 8 = The 0th row is the visual left-hand side of the image, and the
> >     0th column is the visual bottom.
> > 
> > 6 should be 90 degrees clockwise, while 8 should 270 degrees clockwise.
> > Fix this.
> 
> https://lists.libcamera.org/pipermail/libcamera-devel/2020-September/012647.html
> 
> The important part is that "Android defines the rotation as the
> clockwise angle by which the image needs to be rotated to appear in the
> correct orientation on the device screen". Could you read the thread and

I see that both android jpeg orientation and sensor orientation share
this same definition.

In the tests, in verifyJpegExifExtraTags() (CameraTestUtils.java), the
android result metadata JPEG orientation is compared with the EXIF
orientation tag:

int exifOrientation = exif.getAttributeInt(ExifInterface.TAG_ORIENTATION, -1)
...
int requestedOrientation = result.get(CaptureResult.JPEG_ORIENTATION);
...
collector.expectEquals("Exif orientaiton should match requested orientation",
	requestedOrientation, getExifOrientationInDegree(exifOrientation,
	collector));

In getExifOrientationInDegree() (CameraTestUtils.java), there's a nice
switch-case on exifOrientation:

case ExifInterface.ORIENTATION_NORMAL:
	return 0;
case ExifInterface.ORIENTATION_ROTATE_90:
	return 90;
case ExifInterface.ORIENTATION_ROTATE_180:
	return 180;
case ExifInterface.ORIENTATION_ROTATE_270:
	return 270;

And the definitions of ExifInterface.ORIENTATION_ROTATE_{90,270}
according to [1]:

ORIENTATION_ROTATE_90
Constant Value: 6 (0x00000006) 

ORIENTATION_ROTATE_270
Constant Value: 8 (0x00000008) 

Which matches up with the test results, which prompted this patch.

[1] https://developer.android.com/reference/android/media/ExifInterface

> let me know if you still think the current implementation is incorrect ?

So according to the android properties documentation on jpeg and sensor
orientation, the current implementation should be correct, but CTS
disagrees.

> Does this patch fix an issue you've noticed, or does it stem from
> reading the code only ?
> 
> > Sogned-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> Sogned-off-by ?

Whoops :p

> > ---
> >  src/android/jpeg/exif.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> > index 33b3fa7f..b19cb4cd 100644
> > --- a/src/android/jpeg/exif.cpp
> > +++ b/src/android/jpeg/exif.cpp
> > @@ -263,13 +263,13 @@ void Exif::setOrientation(int orientation)
> >  		value = 1;
> >  		break;
> >  	case 90:
> > -		value = 8;
> > +		value = 6;
> >  		break;
> >  	case 180:
> >  		value = 3;
> >  		break;
> >  	case 270:
> > -		value = 6;
> > +		value = 8;
> >  		break;
> >  	}
> >  


Thanks,

Paul