Improve VideoDecoder error messages in Glide.

PiperOrigin-RevId: 345121436

library/src/main/java/com/bumptech/glide/load/ResourceDecoder.java

@@ -49,6 +49,14 @@ public interface ResourceDecoder<T, Z> {
    *     this particular implementation. Implementations should not assume that any or all of their
    *     option keys are present. However, implementations may assume that if one of their option
    *     keys is present, it's value is non-null and is of the expected type.
+   * @throws IOException typically only if the {@code source} ({@link java.io.InputStream}, {@link
+   *     android.os.ParcelFileDescriptor} etc) throws while being read.
+   * @throws OutOfMemoryError is sometimes thrown if the the request produces an overly large result
+   *     due to some combination of source size, requested size, source format and requested format.
+   *     Callers do/must handle this error and implementations can throw this error.
+   * @throws RuntimeException is thrown by a variety of decoding libraries, including various
+   *     Android libraries. Callers do/must handle this error and implementations can throw this
+   *     exception or, preferably, more detailed subclasses.
    */
   @Nullable
   Resource<Z> decode(@NonNull T source, int width, int height, @NonNull Options options)

library/src/main/java/com/bumptech/glide/load/engine/GlideException.java

@@ -166,9 +166,9 @@ public String getMessage() {
     if (rootCauses.isEmpty()) {
       return result.toString();
     } else if (rootCauses.size() == 1) {
-      result.append("\nThere was 1 cause:");
+      result.append("\nThere was 1 root cause:");
     } else {
-      result.append("\nThere were ").append(rootCauses.size()).append(" causes:");
+      result.append("\nThere were ").append(rootCauses.size()).append(" root causes:");
     }
     for (Throwable cause : rootCauses) {
       result

library/src/main/java/com/bumptech/glide/load/resource/bitmap/VideoDecoder.java

@@ -179,10 +179,6 @@ public Resource<Bitmap> decode(
               outWidth,
               outHeight,
               downsampleStrategy);
-
-    } catch (RuntimeException e) {
-      // MediaMetadataRetriever APIs throw generic runtime exceptions when given invalid data.
-      throw new IOException(e);
     } finally {
       mediaMetadataRetriever.release();
     }
@@ -218,9 +214,18 @@ private static Bitmap decodeFrame(
       result = decodeOriginalFrame(mediaMetadataRetriever, frameTimeMicros, frameOption);
     }
 
+    // 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
+    // appropriate tags in adb.
+    if (result == null) {
+      throw new VideoDecoderException();
+    }
+
     return result;
   }
 
+  @Nullable
   @TargetApi(Build.VERSION_CODES.O_MR1)
   private static Bitmap decodeScaledFrame(
       MediaMetadataRetriever mediaMetadataRetriever,
@@ -264,7 +269,10 @@ private static Bitmap decodeScaledFrame(
       // just from decoding the frame, then it will be thrown and exposed to callers by the method
       // below.
       if (Log.isLoggable(TAG, Log.DEBUG)) {
-        Log.d(TAG, "Exception trying to decode frame on oreo+", t);
+        Log.d(
+            TAG,
+            "Exception trying to decode a scaled frame on oreo+, falling back to a fullsize frame",
+            t);
       }
 
       return null;
@@ -336,4 +344,15 @@ public void close() {}
           });
     }
   }
+
+  private static final class VideoDecoderException extends RuntimeException {
+
+    private static final long serialVersionUID = -2556382523004027815L;
+
+    VideoDecoderException() {
+      super(
+          "MediaMetadataRetriever failed to retrieve a frame without throwing, check the adb logs"
+              + " for .*MetadataRetriever.* prior to this exception for details");
+    }
+  }
 }

library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/VideoDecoderTest.java

@@ -2,6 +2,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.never;
@@ -22,6 +23,7 @@
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.function.ThrowingRunnable;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -70,9 +72,16 @@ public void testReturnsRetrievedFrameForResource() throws IOException {
   }
 
   @Test
-  public void testReleasesMediaMetadataRetriever() throws IOException {
+  public void testReleasesMediaMetadataRetriever() {
     Util.setSdkVersionInt(19);
-    decoder.decode(resource, 1, 2, options);
+    assertThrows(
+        RuntimeException.class,
+        new ThrowingRunnable() {
+          @Override
+          public void run() throws IOException {
+            decoder.decode(resource, 1, 2, options);
+          }
+        });
 
     verify(retriever).release();
   }
@@ -85,22 +94,36 @@ public void testThrowsExceptionIfCalledWithInvalidFrame() throws IOException {
   }
 
   @Test
-  public void testSpecifiesThumbnailFrameIfICalledWithFrameNumber() throws IOException {
+  public void testSpecifiesThumbnailFrameIfICalledWithFrameNumber() {
     Util.setSdkVersionInt(19);
     long frame = 5;
     options.set(VideoDecoder.TARGET_FRAME, frame);
     decoder = new VideoDecoder<>(bitmapPool, initializer, factory);
 
-    decoder.decode(resource, 100, 100, options);
+    assertThrows(
+        RuntimeException.class,
+        new ThrowingRunnable() {
+          @Override
+          public void run() throws IOException {
+            decoder.decode(resource, 1, 2, options);
+          }
+        });
 
     verify(retriever).getFrameAtTime(frame, VideoDecoder.DEFAULT_FRAME_OPTION);
   }
 
   @Test
-  public void testDoesNotSpecifyThumbnailFrameIfCalledWithoutFrameNumber() throws IOException {
+  public void testDoesNotSpecifyThumbnailFrameIfCalledWithoutFrameNumber() {
     Util.setSdkVersionInt(19);
     decoder = new VideoDecoder<>(bitmapPool, initializer, factory);
-    decoder.decode(resource, 100, 100, options);
+    assertThrows(
+        RuntimeException.class,
+        new ThrowingRunnable() {
+          @Override
+          public void run() throws IOException {
+            decoder.decode(resource, 100, 100, options);
+          }
+        });
 
     verify(retriever).getFrameAtTime(VideoDecoder.DEFAULT_FRAME, VideoDecoder.DEFAULT_FRAME_OPTION);
   }