Skip to content

Commit 4bfda58

Browse files
sjuddglide-copybara-robot
sjudd
authored and
glide-copybara-robot
committedOct 25, 2022
Prevent decoding VP8 videos on ARC devices, which can put the device into a bad state.
PiperOrigin-RevId: 483541133
·
v5.0.0-rc01compose-alpha6
1 parent 4298bb7 commit 4bfda58

File tree

2 files changed

+162
-40
lines changed

2 files changed

+162
-40
lines changed
 

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

+112-33
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import android.graphics.Bitmap;
66
import android.graphics.Matrix;
77
import android.media.MediaDataSource;
8+
import android.media.MediaExtractor;
89
import android.media.MediaFormat;
910
import android.media.MediaMetadataRetriever;
1011
import android.os.Build;
@@ -125,7 +126,9 @@ public void update(
125126
private static final List<String> PIXEL_T_BUILD_ID_PREFIXES_REQUIRING_HDR_180_ROTATION_FIX =
126127
Collections.unmodifiableList(Arrays.asList("TP1A", "TD1A.220804.031"));
127128

128-
private final MediaMetadataRetrieverInitializer<T> initializer;
129+
private static final String WEBM_MIME_TYPE = "video/webm";
130+
131+
private final MediaInitializer<T> initializer;
129132
private final BitmapPool bitmapPool;
130133
private final MediaMetadataRetrieverFactory factory;
131134

@@ -142,14 +145,14 @@ public static ResourceDecoder<ByteBuffer, Bitmap> byteBuffer(BitmapPool bitmapPo
142145
return new VideoDecoder<>(bitmapPool, new ByteBufferInitializer());
143146
}
144147

145-
VideoDecoder(BitmapPool bitmapPool, MediaMetadataRetrieverInitializer<T> initializer) {
148+
VideoDecoder(BitmapPool bitmapPool, MediaInitializer<T> initializer) {
146149
this(bitmapPool, initializer, DEFAULT_FACTORY);
147150
}
148151

149152
@VisibleForTesting
150153
VideoDecoder(
151154
BitmapPool bitmapPool,
152-
MediaMetadataRetrieverInitializer<T> initializer,
155+
MediaInitializer<T> initializer,
153156
MediaMetadataRetrieverFactory factory) {
154157
this.bitmapPool = bitmapPool;
155158
this.initializer = initializer;
@@ -185,9 +188,10 @@ public Resource<Bitmap> decode(
185188
final Bitmap result;
186189
MediaMetadataRetriever mediaMetadataRetriever = factory.build();
187190
try {
188-
initializer.initialize(mediaMetadataRetriever, resource);
191+
initializer.initializeRetriever(mediaMetadataRetriever, resource);
189192
result =
190193
decodeFrame(
194+
resource,
191195
mediaMetadataRetriever,
192196
frameTimeMicros,
193197
frameOption,
@@ -206,13 +210,18 @@ public Resource<Bitmap> decode(
206210
}
207211

208212
@Nullable
209-
private static Bitmap decodeFrame(
213+
private Bitmap decodeFrame(
214+
@NonNull T resource,
210215
MediaMetadataRetriever mediaMetadataRetriever,
211216
long frameTimeMicros,
212217
int frameOption,
213218
int outWidth,
214219
int outHeight,
215220
DownsampleStrategy strategy) {
221+
if (isUnsupportedFormat(resource, mediaMetadataRetriever)) {
222+
throw new IllegalStateException("Cannot decode VP8 video on CrOS.");
223+
}
224+
216225
Bitmap result = null;
217226
// Arguably we should handle the case where just width or just height is set to
218227
// Target.SIZE_ORIGINAL. Up to and including OMR1, MediaMetadataRetriever defaults to setting
@@ -402,6 +411,54 @@ private static Bitmap decodeOriginalFrame(
402411
return mediaMetadataRetriever.getFrameAtTime(frameTimeMicros, frameOption);
403412
}
404413

414+
/** Returns true if the format type is unsupported on the device. */
415+
private boolean isUnsupportedFormat(
416+
@NonNull T resource, MediaMetadataRetriever mediaMetadataRetriever) {
417+
// MediaFormat.KEY_MIME check below requires at least JELLY_BEAN
418+
if (Build.VERSION.SDK_INT < VERSION_CODES.JELLY_BEAN) {
419+
return false;
420+
}
421+
422+
// The primary known problem is vp8 video on ChromeOS (ARC) devices.
423+
boolean isArc = Build.DEVICE != null && Build.DEVICE.matches(".+_cheets|cheets_.+");
424+
if (!isArc) {
425+
return false;
426+
}
427+
428+
MediaExtractor mediaExtractor = null;
429+
try {
430+
// Include the MediaMetadataRetriever extract in the try block out of an abundance of caution.
431+
String mimeType =
432+
mediaMetadataRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_MIMETYPE);
433+
if (!WEBM_MIME_TYPE.equals(mimeType)) {
434+
return false;
435+
}
436+
437+
// Only construct a MediaExtractor for webm files, since the constructor makes a JNI call
438+
mediaExtractor = new MediaExtractor();
439+
initializer.initializeExtractor(mediaExtractor, resource);
440+
int numTracks = mediaExtractor.getTrackCount();
441+
for (int i = 0; i < numTracks; ++i) {
442+
MediaFormat mediaformat = mediaExtractor.getTrackFormat(i);
443+
String trackMimeType = mediaformat.getString(MediaFormat.KEY_MIME);
444+
if (MediaFormat.MIMETYPE_VIDEO_VP8.equals(trackMimeType)) {
445+
return true;
446+
}
447+
}
448+
} catch (Throwable t) {
449+
// Catching everything here out of an abundance of caution
450+
if (Log.isLoggable(TAG, Log.DEBUG)) {
451+
Log.d(TAG, "Exception trying to extract track info for a webm video on CrOS.", t);
452+
}
453+
} finally {
454+
if (mediaExtractor != null) {
455+
mediaExtractor.release();
456+
}
457+
}
458+
459+
return false;
460+
}
461+
405462
@VisibleForTesting
406463
static class MediaMetadataRetrieverFactory {
407464
public MediaMetadataRetriever build() {
@@ -410,56 +467,78 @@ public MediaMetadataRetriever build() {
410467
}
411468

412469
@VisibleForTesting
413-
interface MediaMetadataRetrieverInitializer<T> {
414-
void initialize(MediaMetadataRetriever retriever, T data);
470+
interface MediaInitializer<T> {
471+
void initializeRetriever(MediaMetadataRetriever retriever, T data);
472+
473+
void initializeExtractor(MediaExtractor extractor, T data) throws IOException;
415474
}
416475

417476
private static final class AssetFileDescriptorInitializer
418-
implements MediaMetadataRetrieverInitializer<AssetFileDescriptor> {
477+
implements MediaInitializer<AssetFileDescriptor> {
419478

420479
@Override
421-
public void initialize(MediaMetadataRetriever retriever, AssetFileDescriptor data) {
480+
public void initializeRetriever(MediaMetadataRetriever retriever, AssetFileDescriptor data) {
422481
retriever.setDataSource(data.getFileDescriptor(), data.getStartOffset(), data.getLength());
423482
}
483+
484+
@Override
485+
public void initializeExtractor(MediaExtractor extractor, AssetFileDescriptor data)
486+
throws IOException {
487+
extractor.setDataSource(data.getFileDescriptor(), data.getStartOffset(), data.getLength());
488+
}
424489
}
425490

426491
// Visible for VideoBitmapDecoder.
427492
static final class ParcelFileDescriptorInitializer
428-
implements MediaMetadataRetrieverInitializer<ParcelFileDescriptor> {
493+
implements MediaInitializer<ParcelFileDescriptor> {
429494

430495
@Override
431-
public void initialize(MediaMetadataRetriever retriever, ParcelFileDescriptor data) {
496+
public void initializeRetriever(MediaMetadataRetriever retriever, ParcelFileDescriptor data) {
432497
retriever.setDataSource(data.getFileDescriptor());
433498
}
499+
500+
@Override
501+
public void initializeExtractor(MediaExtractor extractor, ParcelFileDescriptor data)
502+
throws IOException {
503+
extractor.setDataSource(data.getFileDescriptor());
504+
}
434505
}
435506

436507
@RequiresApi(Build.VERSION_CODES.M)
437-
static final class ByteBufferInitializer
438-
implements MediaMetadataRetrieverInitializer<ByteBuffer> {
508+
static final class ByteBufferInitializer implements MediaInitializer<ByteBuffer> {
439509

440510
@Override
441-
public void initialize(MediaMetadataRetriever retriever, final ByteBuffer data) {
442-
retriever.setDataSource(
443-
new MediaDataSource() {
444-
@Override
445-
public int readAt(long position, byte[] buffer, int offset, int size) {
446-
if (position >= data.limit()) {
447-
return -1;
448-
}
449-
data.position((int) position);
450-
int numBytesRead = Math.min(size, data.remaining());
451-
data.get(buffer, offset, numBytesRead);
452-
return numBytesRead;
453-
}
511+
public void initializeRetriever(MediaMetadataRetriever retriever, final ByteBuffer data) {
512+
retriever.setDataSource(getMediaDataSource(data));
513+
}
454514

455-
@Override
456-
public long getSize() {
457-
return data.limit();
458-
}
515+
@Override
516+
public void initializeExtractor(MediaExtractor extractor, final ByteBuffer data)
517+
throws IOException {
518+
extractor.setDataSource(getMediaDataSource(data));
519+
}
459520

460-
@Override
461-
public void close() {}
462-
});
521+
private MediaDataSource getMediaDataSource(final ByteBuffer data) {
522+
return new MediaDataSource() {
523+
@Override
524+
public int readAt(long position, byte[] buffer, int offset, int size) {
525+
if (position >= data.limit()) {
526+
return -1;
527+
}
528+
data.position((int) position);
529+
int numBytesRead = Math.min(size, data.remaining());
530+
data.get(buffer, offset, numBytesRead);
531+
return numBytesRead;
532+
}
533+
534+
@Override
535+
public long getSize() {
536+
return data.limit();
537+
}
538+
539+
@Override
540+
public void close() {}
541+
};
463542
}
464543
}
465544

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

+50-7
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static com.google.common.truth.Truth.assertThat;
44
import static org.junit.Assert.assertEquals;
55
import static org.junit.Assert.assertThrows;
6+
import static org.mockito.ArgumentMatchers.any;
67
import static org.mockito.ArgumentMatchers.anyInt;
78
import static org.mockito.ArgumentMatchers.anyLong;
89
import static org.mockito.Mockito.never;
@@ -37,7 +38,7 @@
3738
public class VideoDecoderTest {
3839
@Mock private ParcelFileDescriptor resource;
3940
@Mock private VideoDecoder.MediaMetadataRetrieverFactory factory;
40-
@Mock private VideoDecoder.MediaMetadataRetrieverInitializer<ParcelFileDescriptor> initializer;
41+
@Mock private VideoDecoder.MediaInitializer<ParcelFileDescriptor> initializer;
4142
@Mock private MediaMetadataRetriever retriever;
4243
@Mock private BitmapPool bitmapPool;
4344
private VideoDecoder<ParcelFileDescriptor> decoder;
@@ -46,6 +47,7 @@ public class VideoDecoderTest {
4647
private String initialMake;
4748
private String initialModel;
4849
private String initialBuildId;
50+
private String initialDevice;
4951

5052
@Before
5153
public void setup() {
@@ -58,13 +60,13 @@ public void setup() {
5860
initialMake = Build.MANUFACTURER;
5961
initialModel = Build.MODEL;
6062
initialBuildId = Build.ID;
63+
initialDevice = Build.DEVICE;
6164
}
6265

6366
@After
6467
public void tearDown() {
6568
Util.setSdkVersionInt(initialSdkVersion);
66-
setMakeAndModel(initialMake, initialModel);
67-
setBuildId(initialBuildId);
69+
resetBuildInfo(initialMake, initialModel, initialBuildId, initialDevice);
6870
}
6971

7072
@Test
@@ -77,7 +79,7 @@ public void testReturnsRetrievedFrameForResource() throws IOException {
7779
Resource<Bitmap> result =
7880
Preconditions.checkNotNull(decoder.decode(resource, 100, 100, options));
7981

80-
verify(initializer).initialize(retriever, resource);
82+
verify(initializer).initializeRetriever(retriever, resource);
8183
assertEquals(expected, result.get());
8284
}
8385

@@ -194,6 +196,45 @@ public void decodeFrame_withTargetSizeOriginalHeightOnly_onApi27_doesNotThrow()
194196
.isSameInstanceAs(expected);
195197
}
196198

199+
@Test
200+
public void decodeFrame_notArcDeviceButWebm_doesNotInitializeMediaExtractor() throws IOException {
201+
setDevice("notArc");
202+
when(retriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_MIMETYPE))
203+
.thenReturn("video/webm");
204+
when(retriever.getFrameAtTime(-1, MediaMetadataRetriever.OPTION_CLOSEST_SYNC))
205+
.thenReturn(Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888));
206+
207+
decoder.decode(resource, Target.SIZE_ORIGINAL, Target.SIZE_ORIGINAL, options).get();
208+
209+
verify(initializer, never()).initializeExtractor(any(), any());
210+
}
211+
212+
@Test
213+
public void decodeFrame_arcDeviceButNotWebm_doesNotInitializeMediaExtractor() throws IOException {
214+
setDevice("arc_cheets");
215+
when(retriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_MIMETYPE))
216+
.thenReturn("video/mp4");
217+
when(retriever.getFrameAtTime(-1, MediaMetadataRetriever.OPTION_CLOSEST_SYNC))
218+
.thenReturn(Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888));
219+
220+
decoder.decode(resource, Target.SIZE_ORIGINAL, Target.SIZE_ORIGINAL, options).get();
221+
222+
verify(initializer, never()).initializeExtractor(any(), any());
223+
}
224+
225+
@Test
226+
public void decodeFrame_arcDeviceAndWebm_initializesMediaExtractor() throws IOException {
227+
setDevice("arc_cheets");
228+
when(retriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_MIMETYPE))
229+
.thenReturn("video/webm");
230+
when(retriever.getFrameAtTime(-1, MediaMetadataRetriever.OPTION_CLOSEST_SYNC))
231+
.thenReturn(Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888));
232+
233+
decoder.decode(resource, Target.SIZE_ORIGINAL, Target.SIZE_ORIGINAL, options).get();
234+
235+
verify(initializer).initializeExtractor(any(), any());
236+
}
237+
197238
@Test
198239
@Config(sdk = VERSION_CODES.M)
199240
public void isHdr180RotationFixRequired_androidM_returnsFalse() {
@@ -218,12 +259,14 @@ public void isHdr180RotationFixRequired_androidS_returnsTrue() {
218259
assertThat(VideoDecoder.isHdr180RotationFixRequired()).isTrue();
219260
}
220261

221-
private void setMakeAndModel(String make, String model) {
262+
private void resetBuildInfo(String make, String model, String buildId, String device) {
222263
ReflectionHelpers.setStaticField(Build.class, "MANUFACTURER", make);
223264
ReflectionHelpers.setStaticField(Build.class, "MODEL", model);
265+
ReflectionHelpers.setStaticField(Build.class, "ID", buildId);
266+
setDevice(device);
224267
}
225268

226-
private void setBuildId(String buildId) {
227-
ReflectionHelpers.setStaticField(Build.class, "ID", buildId);
269+
private void setDevice(String device) {
270+
ReflectionHelpers.setStaticField(Build.class, "DEVICE", device);
228271
}
229272
}

0 commit comments

Comments
 (0)