Exif orientation correction and cropping of Exif oriented images

common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/MappingTest.scala

@@ -56,7 +56,10 @@ object MappingTest {
     size = Some(12345L),
     mimeType = Some(Jpeg),
     dimensions = Some(Dimensions(1000, 2000)),
-    secureUrl = Some(new URL("http://host/filename.jpg"))
+    secureUrl = Some(new URL("http://host/filename.jpg")),
+    orientationMetadata = Some(OrientationMetadata(exifOrientation = Some(6))),
+    orientedDimensions = Some(Dimensions(2000, 1000)),
+    orientation = Some("landscape")
   )
 
   val testUploader = "grid-internal-mapping-test-image" // Do not change this, we use it to clean up old test images
@@ -77,14 +80,21 @@ object MappingTest {
       size = Some(1234L),
       mimeType = Some(Jpeg),
       dimensions = Some(Dimensions(500, 1000)),
-      secureUrl = Some(new URL("http://host/thumb.jpg"))
+      secureUrl = Some(new URL("http://host/thumb.jpg")),
+      orientationMetadata = Some(OrientationMetadata(exifOrientation = Some(6))),
+      orientedDimensions = Some(Dimensions(2000, 1000)),
+      orientation = Some("landscape")
+
     )),
     optimisedPng = Some(Asset(
       file = new URI("file://filename.png"),
       size = Some(1245L),
       mimeType = Some(Png),
       dimensions = Some(Dimensions(1000, 2000)),
-      secureUrl = Some(new URL("http://host/filename.jpg"))
+      secureUrl = Some(new URL("http://host/filename.jpg")),
+      orientationMetadata = Some(OrientationMetadata(exifOrientation = Some(6))),
+      orientedDimensions = Some(Dimensions(2000, 1000)),
+      orientation = Some("landscape")
     )),
     fileMetadata = FileMetadata(
       iptc = Map("iptc1" -> "value1"),
@@ -131,7 +141,8 @@ object MappingTest {
             x = 100, y = 100, width = 500, height = 300
           ),
           aspectRatio = Some("5:3"),
-          `type` = CropExport
+          `type` = CropExport,
+          rotation = Some(90),
         ),
         master = Some(testAsset),
         assets = List(testAsset)

common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/Mappings.scala

@@ -81,15 +81,18 @@ object Mappings {
 
   def dimensionsMapping(name: String) = nonDynamicObjectField(name).copy(properties = Seq(
     intField("width"),
-    intField("height")
+    intField("height"),
   ))
 
   def assetMapping(name: String) = nonDynamicObjectField(name).copy(properties = Seq(
     nonIndexedString("file"),
     nonIndexedString("secureUrl"),
     intField("size"),
     keywordField("mimeType"),
-    dimensionsMapping("dimensions")
+    dimensionsMapping("dimensions"),
+    dynamicObj("orientationMetadata"),
+    dimensionsMapping("orientedDimensions"),
+    keywordField("orientation"),
   ))
 
   def metadataMapping(name: String): ObjectField = nonDynamicObjectField(name).copy(properties = Seq(

common-lib/src/main/scala/com/gu/mediaservice/lib/imaging/ImageOperations.scala

@@ -82,12 +82,14 @@ class ImageOperations(playPath: String) extends GridLogging {
     iccColourSpace: Option[String],
     colourModel: Option[String],
     fileType: MimeType,
-    isTransformedFromSource: Boolean
+    isTransformedFromSource: Boolean,
+    orientationMetadata: Option[OrientationMetadata]
   )(implicit logMarker: LogMarker): Future[File] = {
     for {
       outputFile <- createTempFile(s"crop-", s"${fileType.fileExtension}", tempDir)
       cropSource    = addImage(sourceFile)
-      qualified     = quality(cropSource)(qual)
+      oriented      = orient(cropSource, orientationMetadata)
+      qualified     = quality(oriented)(qual)
       corrected     = correctColour(qualified)(iccColourSpace, colourModel, isTransformedFromSource)
       converted     = applyOutputProfile(corrected)
       stripped      = stripMeta(converted)
@@ -127,6 +129,14 @@ class ImageOperations(playPath: String) extends GridLogging {
     yield outputFile
   }
 
+  private def orient(op: IMOperation, orientationMetadata: Option[OrientationMetadata]): IMOperation = {
+    logger.info("Correcting for orientation: " + orientationMetadata)
+    orientationMetadata.map(_.orientationCorrection()) match {
+      case Some(angle) => rotate(op)(angle)
+      case _ => op
+    }
+  }
+
   def optimiseImage(resizedFile: File, mediaType: MimeType): File = mediaType match {
     case Png =>
       val fileName: String = resizedFile.getAbsolutePath
@@ -153,7 +163,7 @@ class ImageOperations(playPath: String) extends GridLogging {
   val backgroundColour = "#333333"
 
   /**
-    * Given a source file containing a png (the 'browser viewable' file),
+    * Given a source file containing an image (the 'browser viewable' file),
     * construct a thumbnail file in the provided temp directory, and return
     * the file with metadata about it.
     * @param browserViewableImage
@@ -169,12 +179,14 @@ class ImageOperations(playPath: String) extends GridLogging {
                       qual: Double = 100d,
                       outputFile: File,
                       iccColourSpace: Option[String],
-                      colourModel: Option[String]
+                      colourModel: Option[String],
+                      orientationMetadata: Option[OrientationMetadata]
   )(implicit logMarker: LogMarker): Future[(File, MimeType)] = {
     val stopwatch = Stopwatch.start
 
     val cropSource     = addImage(browserViewableImage.file)
-    val thumbnailed    = thumbnail(cropSource)(width)
+    val orientated     = orient(cropSource, orientationMetadata)
+    val thumbnailed    = thumbnail(orientated)(width)
     val corrected      = correctColour(thumbnailed)(iccColourSpace, colourModel, browserViewableImage.isTransformedFromSource)
     val converted      = applyOutputProfile(corrected, optimised = true)
     val stripped       = stripMeta(converted)

common-lib/src/main/scala/com/gu/mediaservice/lib/imaging/im4jwrapper/ImageMagick.scala

@@ -48,6 +48,12 @@ object ImageMagick extends GridLogging {
     op.profile(profileFileLocation)
     op
   }
+
+  def rotate(op: IMOperation)(angle: Double): IMOperation = {
+    op.rotate(angle)
+    op
+  }
+
   def thumbnail(op: IMOperation)(width: Int): IMOperation = {
     op.thumbnail(width)
     op

common-lib/src/main/scala/com/gu/mediaservice/model/Asset.scala

@@ -7,20 +7,54 @@ import com.gu.mediaservice.lib.aws.S3Object
 
 
 // FIXME: size, mimeType and dimensions not optional (must backfill first)
-case class Asset(file: URI, size: Option[Long], mimeType: Option[MimeType], dimensions: Option[Dimensions], secureUrl: Option[URL] = None)
+case class Asset(file: URI, size: Option[Long], mimeType: Option[MimeType], dimensions: Option[Dimensions], secureUrl: Option[URL] = None,
+                 orientationMetadata: Option[OrientationMetadata] = None,
+                 orientedDimensions: Option[Dimensions] = None,
+                 orientation: Option[String] = None
+                )
 
 object Asset {
 
-  def fromS3Object(s3Object: S3Object, dims: Option[Dimensions]): Asset = {
+  def fromS3Object(s3Object: S3Object, dims: Option[Dimensions],
+                   orientationMetadata: Option[OrientationMetadata] = None): Asset = {
     val userMetadata   = s3Object.metadata.userMetadata
     val objectMetadata = s3Object.metadata.objectMetadata
 
+    val orientedDimensions = for {
+      dimensions <- dims
+      orientationMetadata <- orientationMetadata
+    } yield {
+      orientationMetadata.correctedDimensions(dimensions)
+    }
+
+    val maybeCorrectedOrientation = for {
+      dimensions <- dims
+      orientationMetadata <- orientationMetadata
+    } yield {
+      orientationMetadata.correctedOrientation(dimensions)
+    }
+
+    val maybeDimensionsOrientation = for {
+      dimensions <- dims
+    } yield {
+      if (dimensions.width < dimensions.height) {
+        "portrait"
+      } else {
+        "landscape"
+      }
+    }
+
+    val maybeOrientation = Seq(maybeCorrectedOrientation, maybeDimensionsOrientation).flatten.headOption
+
     Asset(
       file       = s3Object.uri,
       size       = Some(s3Object.size),
       mimeType   = objectMetadata.contentType,
       dimensions = dims,
-      secureUrl  = None
+      secureUrl  = None,
+      orientationMetadata = orientationMetadata,
+      orientedDimensions = orientedDimensions,
+      orientation = maybeOrientation
     )
   }
 
@@ -29,25 +63,66 @@ object Asset {
       (__ \ "size").readNullable[Long] ~
       (__ \ "mimeType").readNullable[MimeType] ~
       (__ \ "dimensions").readNullable[Dimensions] ~
-      (__ \ "secureUrl").readNullable[String].map(_.map(new URL(_)))
+      (__ \ "secureUrl").readNullable[String].map(_.map(new URL(_)))  ~
+      (__ \ "orientationMetadata").readNullable[OrientationMetadata] ~
+      (__ \ "orientedDimensions").readNullable[Dimensions] ~
+      (__ \ "orientation").readNullable[String]
       )(Asset.apply _)
 
   implicit val assetWrites: Writes[Asset] =
     ((__ \ "file").write[String].contramap((_: URI).toString) ~
       (__ \ "size").writeNullable[Long] ~
       (__ \ "mimeType").writeNullable[MimeType] ~
       (__ \ "dimensions").writeNullable[Dimensions] ~
-      (__ \ "secureUrl").writeNullable[String].contramap((_: Option[URL]).map(_.toString))
+      (__ \ "secureUrl").writeNullable[String].contramap((_: Option[URL]).map(_.toString)) ~
+      (__ \ "orientationMetadata").writeNullable[OrientationMetadata] ~
+      (__ \ "orientedDimensions").writeNullable[Dimensions] ~
+      (__ \ "orientation").writeNullable[String]
       )(unlift(Asset.unapply))
-
 }
 
 case class Dimensions(width: Int, height: Int)
 object Dimensions {
   implicit val dimensionsReads: Reads[Dimensions] = Json.reads[Dimensions]
-  implicit val dimensionsWrites: Writes[Dimensions] =
-    ((__ \ "width").write[Int] ~
-      (__ \ "height").write[Int]
-      )(unlift(Dimensions.unapply))
+  implicit val dimensionsWrites: Writes[Dimensions] = Json.writes[Dimensions]
 }
 
+case class OrientationMetadata(exifOrientation: Option[Int]) {
+
+  def transformsImage(): Boolean = !exifOrientation.exists(OrientationMetadata.exifOrientationsWhichDoNotTransformTheImage.contains)
+
+  def orientationCorrection(): Int = {
+    exifOrientation match {
+      case Some(6) => 90
+      case Some(8) => -90
+      case Some(3) => 180
+      case _ => 0
+    }
+  }
+
+  def correctedDimensions(sourceDimensions: Dimensions): Dimensions = {
+    if (flipsDimensions()) {
+      Dimensions(width = sourceDimensions.height, height = sourceDimensions.width)
+    } else {
+      sourceDimensions
+    }
+  }
+
+  def correctedOrientation(sourceDimensions: Dimensions): String = {
+    val orientedDimensions = correctedDimensions(sourceDimensions)
+    if (orientedDimensions.width < orientedDimensions.height) {
+      "portrait"
+    } else {
+      "landscape"
+    }
+  }
+
+  private def flipsDimensions(): Boolean = exifOrientation.exists(OrientationMetadata.exifOrientationsWhichFlipWidthAndHeight.contains)
+
+}
+
+object OrientationMetadata {
+  private val exifOrientationsWhichDoNotTransformTheImage = Set(1)
+  private val exifOrientationsWhichFlipWidthAndHeight = Set(6, 8)
+  implicit val orientationFormat: OFormat[OrientationMetadata] = Json.format[OrientationMetadata]
+}

common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala

@@ -58,17 +58,19 @@ object ExportType {
 }
 
 
-case class CropSpec(uri: String, bounds: Bounds, aspectRatio: Option[String], `type`: ExportType = ExportType.default)
+case class CropSpec(uri: String, bounds: Bounds, aspectRatio: Option[String], `type`: ExportType = ExportType.default,
+                    rotation: Option[Int])
 
 object CropSpec {
 
   implicit val cropSpecWrites: Writes[CropSpec] = Json.writes[CropSpec]
   implicit val cropSpecReads: Reads[CropSpec] = (
     (__ \ "uri").read[String] ~
-    (__ \ "bounds").read[Bounds] ~
-    (__ \ "aspectRatio").readNullable[String] ~
-    (__ \ "type").readNullable[ExportType].map(_.getOrElse(ExportType.default))
-  )(CropSpec.apply _)
+      (__ \ "bounds").read[Bounds] ~
+      (__ \ "aspectRatio").readNullable[String] ~
+      (__ \ "type").readNullable[ExportType].map(_.getOrElse(ExportType.default)) ~
+      (__ \ "rotation").readNullable[Int]
+    )(CropSpec.apply _)
 }
 
 

common-lib/src/test/scala/com/gu/mediaservice/lib/cleanup/MetadataHelper.scala

@@ -19,7 +19,7 @@ trait MetadataHelper {
       lastModified = None,
       identifiers = Map(),
       uploadInfo = UploadInfo(),
-      source = Asset(URI.create("http://example.com/image.jpg"), Some(0), None, None),
+      source = Asset(URI.create("http://example.com/image.jpg"), Some(0), None, None, None, None),
       thumbnail = None,
       optimisedPng = None,
       fileMetadata = FileMetadata(),

cropper/app/controllers/CropperController.scala

@@ -167,8 +167,10 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no
       apiImage <- fetchSourceFromApi(exportRequest.uri, onBehalfOfPrincipal)
       _ <- verify(apiImage.valid, InvalidImage)
       // Image should always have dimensions, but we want to safely extract the Option
-      dimensions <- ifDefined(apiImage.source.dimensions, InvalidImage)
-      cropSpec = ExportRequest.toCropSpec(exportRequest, dimensions)
+      // We correct for orientation in the cropper UI; so validate against the oriented dimensions.
+      orientedDimensions = Seq(apiImage.source.orientedDimensions, apiImage.source.dimensions).flatten.headOption
+      dimensions <- ifDefined(orientedDimensions, InvalidImage)
+      cropSpec = ExportRequest.toCropSpec(exportRequest, dimensions, apiImage.source.orientationMetadata)
       _ <- verify(crops.isWithinImage(cropSpec.bounds, dimensions), InvalidCropRequest)
       crop = Crop.createFromCropSource(
         by = Some(Authentication.getIdentity(user)),

cropper/app/lib/CropSpecMetadata.scala

@@ -6,7 +6,7 @@ import com.gu.mediaservice.model.{Bounds, Crop, CropSpec, Dimensions, ExportType
 trait CropSpecMetadata {
 
   def metadataForCrop(crop: Crop, dimensions: Dimensions): Map[String, String] = {
-    val CropSpec(sourceUri, Bounds(x, y, w, h), r, t) = crop.specification
+    val CropSpec(sourceUri, Bounds(x, y, w, h), r, t, rotation) = crop.specification
     val metadata = Map("source" -> sourceUri,
       "bounds-x" -> x,
       "bounds-y" -> y,
@@ -17,7 +17,9 @@ trait CropSpecMetadata {
       "date" -> crop.date.map(printDateTime),
       "width" -> dimensions.width,
       "height" -> dimensions.height,
-      "aspect-ratio" -> r)
+      "aspect-ratio" -> r,
+      "rotation" -> rotation
+    )
 
     metadata.collect {
       case (key, Some(value)) => key -> value.toString
@@ -37,8 +39,9 @@ trait CropSpecMetadata {
       h <- getOrElseOrNone(userMetadata, "bounds-height", "bounds_h").map(_.toInt)
       ratio = getOrElseOrNone(userMetadata, "aspect-ratio", "aspect_ratio")
       exportType = userMetadata.get("type").map(ExportType.valueOf).getOrElse(ExportType.default)
+      rotation = userMetadata.get("rotation").map(_.toInt)
     } yield {
-      CropSpec(source, Bounds(x, y, w, h), ratio, exportType)
+      CropSpec(source, Bounds(x, y, w, h), ratio, exportType, rotation)
     }
   }
 

cropper/app/lib/Crops.scala

@@ -34,6 +34,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
     crop: Crop,
     mediaType: MimeType,
     colourModel: Option[String],
+    orientationMetadata: Option[OrientationMetadata],
   )(implicit logMarker: LogMarker): Future[MasterCrop] = {
 
     val source   = crop.specification
@@ -45,7 +46,8 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
     for {
       strip <- imageOperations.cropImage(
         sourceFile, apiImage.source.mimeType, source.bounds, masterCropQuality, config.tempDir,
-        iccColourSpace, colourModel, mediaType, isTransformedFromSource = false
+        iccColourSpace, colourModel, mediaType, isTransformedFromSource = false,
+        orientationMetadata = orientationMetadata
       )
       file: File <- imageOperations.appendMetadata(strip, metadata)
       dimensions  = Dimensions(source.bounds.width, source.bounds.height)
@@ -75,12 +77,13 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
 
   def deleteCrops(id: String)(implicit logMarker: LogMarker): Future[Unit] = store.deleteCrops(id)
 
-  def dimensionsFromConfig(bounds: Bounds, aspectRatio: Float): List[Dimensions] = if (bounds.isPortrait)
+  private def dimensionsFromConfig(bounds: Bounds, aspectRatio: Float): List[Dimensions] = if (bounds.isPortrait)
       config.portraitCropSizingHeights.filter(_ <= bounds.height).map(h => Dimensions(math.round(h * aspectRatio), h))
     else
     config.landscapeCropSizingWidths.filter(_ <= bounds.width).map(w => Dimensions(w, math.round(w / aspectRatio)))
 
   def isWithinImage(bounds: Bounds, dimensions: Dimensions): Boolean = {
+    logger.info(s"Validating crop bounds ($bounds) against dimensions: $dimensions")
     val positiveCoords       = List(bounds.x,     bounds.y     ).forall(_ >= 0)
     val strictlyPositiveSize = List(bounds.width, bounds.height).forall(_  > 0)
     val withinBounds = (bounds.x + bounds.width  <= dimensions.width ) &&
@@ -101,7 +104,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
       for {
         sourceFile <- tempFileFromURL(secureUrl, "cropSource", "", config.tempDir)
         colourModel <- ImageOperations.identifyColourModel(sourceFile, mimeType)
-        masterCrop <- createMasterCrop(apiImage, sourceFile, crop, cropType, colourModel)
+        masterCrop <- createMasterCrop(apiImage, sourceFile, crop, cropType, colourModel, apiImage.source.orientationMetadata)
 
         outputDims = dimensionsFromConfig(source.bounds, masterCrop.aspectRatio) :+ masterCrop.dimensions