From de772cfbf0031f6a44f9959ffe1b78f3f30a8025 Mon Sep 17 00:00:00 2001 From: Tolriq Date: Mon, 1 May 2023 12:32:40 +0200 Subject: [PATCH 1/3] Catch errors and OOM when decoding ID3 frames. Invalid frames have no impact on ExoPlayer ability to play the media and should not fail on errors. Some tools can add 100Mb images in the tags that will trigger recoverable OOM with this fix. --- .../extractor/metadata/id3/Id3Decoder.java | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java b/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java index b05717e947..9954764f14 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java @@ -372,8 +372,9 @@ public final class Id3Decoder extends SimpleMetadataDecoder { frameSize = removeUnsynchronization(id3Data, frameSize); } + String error = ""; + Id3Frame frame = null; try { - Id3Frame frame; if (frameId0 == 'T' && frameId1 == 'X' && frameId2 == 'X' @@ -430,18 +431,24 @@ public final class Id3Decoder extends SimpleMetadataDecoder { String id = getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3); frame = decodeBinaryFrame(id3Data, frameSize, id); } - if (frame == null) { - Log.w( - TAG, - "Failed to decode frame: id=" - + getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3) - + ", frameSize=" - + frameSize); - } - return frame; - } finally { + } catch (Exception e) { + error = ",error=" + e.getMessage(); + } catch (OutOfMemoryError e) { + error = ",error=" + e.getMessage(); + } + finally { id3Data.setPosition(nextFramePosition); } + if (frame == null) { + Log.w( + TAG, + "Failed to decode frame: id=" + + getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3) + + ", frameSize=" + + frameSize + + error); + } + return frame; } @Nullable From 820278c5646e213528eb37f002c3685f89d0f0bd Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Wed, 3 Jan 2024 15:49:34 +0000 Subject: [PATCH 2/3] Format with google-java-format --- .../androidx/media3/extractor/metadata/id3/Id3Decoder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java b/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java index 9954764f14..6c07ef5223 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java @@ -435,8 +435,7 @@ public final class Id3Decoder extends SimpleMetadataDecoder { error = ",error=" + e.getMessage(); } catch (OutOfMemoryError e) { error = ",error=" + e.getMessage(); - } - finally { + } finally { id3Data.setPosition(nextFramePosition); } if (frame == null) { From f935f59eaac1f537f45970dd6601c3dba3273c1c Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Wed, 3 Jan 2024 15:54:09 +0000 Subject: [PATCH 3/3] Improve logging by including full stack trace, and add release note --- RELEASENOTES.md | 3 +++ .../media3/extractor/metadata/id3/Id3Decoder.java | 12 +++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5b32e1bb58..4b754f3037 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -97,6 +97,9 @@ * Fix bug where `MediaMetadata` was only populated from Vorbis comments with upper-case keys ([#876](https://github.com/androidx/media/issues/876)). + * Catch `OutOfMemoryError` when parsing very large ID3 frames, meaning + playback can continue without the tag info instead of playback failing + completely. * DRM: * Extend workaround for spurious ClearKey `https://default.url` license URL to API 33+ (previously the workaround only applied on API 33 diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java b/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java index 6c07ef5223..2b7bf3c823 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/id3/Id3Decoder.java @@ -372,8 +372,8 @@ public final class Id3Decoder extends SimpleMetadataDecoder { frameSize = removeUnsynchronization(id3Data, frameSize); } - String error = ""; Id3Frame frame = null; + Throwable error = null; try { if (frameId0 == 'T' && frameId1 == 'X' @@ -431,10 +431,8 @@ public final class Id3Decoder extends SimpleMetadataDecoder { String id = getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3); frame = decodeBinaryFrame(id3Data, frameSize, id); } - } catch (Exception e) { - error = ",error=" + e.getMessage(); - } catch (OutOfMemoryError e) { - error = ",error=" + e.getMessage(); + } catch (OutOfMemoryError | Exception e) { + error = e; } finally { id3Data.setPosition(nextFramePosition); } @@ -444,8 +442,8 @@ public final class Id3Decoder extends SimpleMetadataDecoder { "Failed to decode frame: id=" + getFrameId(majorVersion, frameId0, frameId1, frameId2, frameId3) + ", frameSize=" - + frameSize - + error); + + frameSize, + error); } return frame; }