Work-around MediaMetadataRetriever bug doubly-rotating HDR video 180…

… deg frames by rotating extracted frames from those videos. Target API 33 as well to allow Android T checks. PiperOrigin-RevId: 475676584
bumptech · Sep 20, 2022 · 790c351 · 790c351
1 parent fd5d261
commit 790c351
Show file tree

Hide file tree

Showing 3 changed files with 159 additions and 2 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -39,7 +39,7 @@ VERSION_PATCH=0
 VERSION_NAME=4.14.0-SNAPSHOT
 
 ## SDK versioning
-COMPILE_SDK_VERSION=32
+COMPILE_SDK_VERSION=33
 MIN_SDK_VERSION=14
 TARGET_SDK_VERSION=32
 

diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/VideoDecoder.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/VideoDecoder.java
@@ -3,9 +3,12 @@
 import android.annotation.TargetApi;
 import android.content.res.AssetFileDescriptor;
 import android.graphics.Bitmap;
+import android.graphics.Matrix;
 import android.media.MediaDataSource;
+import android.media.MediaFormat;
 import android.media.MediaMetadataRetriever;
 import android.os.Build;
+import android.os.Build.VERSION;
 import android.os.Build.VERSION_CODES;
 import android.os.ParcelFileDescriptor;
 import android.util.Log;
@@ -22,6 +25,9 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.security.MessageDigest;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * Decodes video data to Bitmaps from {@link ParcelFileDescriptor}s and {@link
@@ -110,6 +116,15 @@ public void update(
   private static final MediaMetadataRetrieverFactory DEFAULT_FACTORY =
       new MediaMetadataRetrieverFactory();
 
+  /**
+   * List of Pixel Android T build id prefixes missing a fix for HDR video with 180 deg rotations
+   * having doubly-rotated thumbnails.
+   *
+   * <p>More recent Android T builds should have the fix.
+   */
+  private static final List<String> PIXEL_T_BUILD_ID_PREFIXES_REQUIRING_HDR_180_ROTATION_FIX =
+      Collections.unmodifiableList(Arrays.asList("TP1A", "TD1A.220804.031"));
+
   private final MediaMetadataRetrieverInitializer<T> initializer;
   private final BitmapPool bitmapPool;
   private final MediaMetadataRetrieverFactory factory;
@@ -218,6 +233,11 @@ private static Bitmap decodeFrame(
       result = decodeOriginalFrame(mediaMetadataRetriever, frameTimeMicros, frameOption);
     }
 
+    // MediaMetadataRetriever has a bug where HDR videos with 180 deg rotations are rotated twice,
+    // causing the output frame to appear upside. This needs to be corrected for all versions of
+    // Android until a platform fix lands.
+    result = correctHdr180DegVideoFrameOrientation(mediaMetadataRetriever, result);
+
     // Throwing an exception works better in our error logging than returning null. It shouldn't
     // be expensive because video decoders are attempted after image loads. Video errors are often
     // logged by the framework, so we can also use this error to suggest callers look for the
@@ -229,6 +249,100 @@ private static Bitmap decodeFrame(
     return result;
   }
 
+  /**
+   * Corrects the orientation of a bitmap extracted from an HDR video with a 180 degree rotation
+   * angle.
+   *
+   * <p>This method will only return a rotated bitmap instead of the input bitmap if
+   *
+   * <ul>
+   *   <li>The Android SDK level is >= R && < T OR the build id is one of T builds without the
+   *       platform fix.
+   *   <li>The video has a color transfer function with an HLG or ST2084 (PQ) transfer function.
+   *   <li>The video has a color standard of BT.2020.
+   *   <li>The video has a rotation angle of +/- 180 degrees.
+   * </ul>
+   */
+  private static Bitmap correctHdr180DegVideoFrameOrientation(
+      MediaMetadataRetriever mediaMetadataRetriever, Bitmap frame) {
+    if (!isHdr180RotationFixRequired()) {
+      return frame;
+    }
+    boolean requiresHdr180RotationFix = false;
+    try {
+      if (isHDR(mediaMetadataRetriever)) {
+        String rotationString =
+            mediaMetadataRetriever.extractMetadata(
+                MediaMetadataRetriever.METADATA_KEY_VIDEO_ROTATION);
+        int rotation = Integer.parseInt(rotationString);
+        requiresHdr180RotationFix = Math.abs(rotation) == 180;
+      }
+    } catch (NumberFormatException e) {
+      if (Log.isLoggable(TAG, Log.DEBUG)) {
+        Log.d(TAG, "Exception trying to extract HDR transfer function or rotation");
+      }
+    }
+
+    if (!requiresHdr180RotationFix) {
+      return frame;
+    }
+
+    if (Log.isLoggable(TAG, Log.DEBUG)) {
+      Log.d(TAG, "Applying HDR 180 deg thumbnail correction");
+    }
+    Matrix rotationMatrix = new Matrix();
+    rotationMatrix.postRotate(
+        /* degrees= */ 180, frame.getWidth() / 2.0f, frame.getHeight() / 2.0f);
+    return Bitmap.createBitmap(
+        frame,
+        /* x= */ 0,
+        /* y= */ 0,
+        frame.getWidth(),
+        frame.getHeight(),
+        rotationMatrix,
+        /* filter= */ true);
+  }
+
+  @RequiresApi(VERSION_CODES.R)
+  private static boolean isHDR(MediaMetadataRetriever mediaMetadataRetriever)
+      throws NumberFormatException {
+    String colorTransferString =
+        mediaMetadataRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_COLOR_TRANSFER);
+    String colorStandardString =
+        mediaMetadataRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_COLOR_STANDARD);
+    int colorTransfer = Integer.parseInt(colorTransferString);
+    int colorStandard = Integer.parseInt(colorStandardString);
+    // This check needs to match the isHDR check in
+    // frameworks/av/media/libstagefright/FrameDecoder.cpp.
+    return (colorTransfer == MediaFormat.COLOR_TRANSFER_HLG
+            || colorTransfer == MediaFormat.COLOR_TRANSFER_ST2084)
+        && colorStandard == MediaFormat.COLOR_STANDARD_BT2020;
+  }
+
+  /** Returns true if the build requires a fix for the HDR 180 degree rotation bug. */
+  @VisibleForTesting
+  static boolean isHdr180RotationFixRequired() {
+    // Only pixel devices have android T builds without the framework fix.
+    if (Build.MODEL.startsWith("Pixel") && VERSION.SDK_INT == VERSION_CODES.TIRAMISU) {
+      return isTBuildRequiringRotationFix();
+    } else {
+      return VERSION.SDK_INT >= VERSION_CODES.R && VERSION.SDK_INT < VERSION_CODES.TIRAMISU;
+    }
+  }
+
+  /**
+   * Returns true if the build is an Android T build that requires a fix for the HDR 180 degree
+   * rotation bug.
+   */
+  private static boolean isTBuildRequiringRotationFix() {
+    for (String buildId : PIXEL_T_BUILD_ID_PREFIXES_REQUIRING_HDR_180_ROTATION_FIX) {
+      if (Build.ID.startsWith(buildId)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   @Nullable
   @TargetApi(Build.VERSION_CODES.O_MR1)
   private static Bitmap decodeScaledFrame(

diff --git a/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/VideoDecoderTest.java b/library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/VideoDecoderTest.java
@@ -12,6 +12,7 @@
 import android.graphics.Bitmap;
 import android.media.MediaMetadataRetriever;
 import android.os.Build;
+import android.os.Build.VERSION_CODES;
 import android.os.ParcelFileDescriptor;
 import com.bumptech.glide.load.Options;
 import com.bumptech.glide.load.engine.Resource;
@@ -29,9 +30,10 @@
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.annotation.Config;
+import org.robolectric.util.ReflectionHelpers;
 
 @RunWith(RobolectricTestRunner.class)
-@Config(sdk = 27)
+@Config(sdk = VERSION_CODES.O_MR1)
 public class VideoDecoderTest {
   @Mock private ParcelFileDescriptor resource;
   @Mock private VideoDecoder.MediaMetadataRetrieverFactory factory;
@@ -41,6 +43,9 @@ public class VideoDecoderTest {
   private VideoDecoder<ParcelFileDescriptor> decoder;
   private Options options;
   private int initialSdkVersion;
+  private String initialMake;
+  private String initialModel;
+  private String initialBuildId;
 
   @Before
   public void setup() {
@@ -50,11 +55,16 @@ public void setup() {
     options = new Options();
 
     initialSdkVersion = Build.VERSION.SDK_INT;
+    initialMake = Build.MANUFACTURER;
+    initialModel = Build.MODEL;
+    initialBuildId = Build.ID;
   }
 
   @After
   public void tearDown() {
     Util.setSdkVersionInt(initialSdkVersion);
+    setMakeAndModel(initialMake, initialModel);
+    setBuildId(initialBuildId);
   }
 
   @Test
@@ -183,4 +193,37 @@ public void decodeFrame_withTargetSizeOriginalHeightOnly_onApi27_doesNotThrow()
     assertThat(decoder.decode(resource, 100, Target.SIZE_ORIGINAL, options).get())
         .isSameInstanceAs(expected);
   }
+
+  @Test
+  @Config(sdk = VERSION_CODES.M)
+  public void isHdr180RotationFixRequired_androidM_returnsFalse() {
+    assertThat(VideoDecoder.isHdr180RotationFixRequired()).isFalse();
+  }
+
+  @Test
+  @Config(sdk = VERSION_CODES.Q)
+  public void isHdr180RotationFixRequired_androidQ_returnsFalse() {
+    assertThat(VideoDecoder.isHdr180RotationFixRequired()).isFalse();
+  }
+
+  @Test
+  @Config(sdk = VERSION_CODES.R)
+  public void isHdr180RotationFixRequired_androidR_returnsTrue() {
+    assertThat(VideoDecoder.isHdr180RotationFixRequired()).isTrue();
+  }
+
+  @Test
+  @Config(sdk = VERSION_CODES.S)
+  public void isHdr180RotationFixRequired_androidS_returnsTrue() {
+    assertThat(VideoDecoder.isHdr180RotationFixRequired()).isTrue();
+  }
+
+  private void setMakeAndModel(String make, String model) {
+    ReflectionHelpers.setStaticField(Build.class, "MANUFACTURER", make);
+    ReflectionHelpers.setStaticField(Build.class, "MODEL", model);
+  }
+
+  private void setBuildId(String buildId) {
+    ReflectionHelpers.setStaticField(Build.class, "ID", buildId);
+  }
 }