From cad3b1830e7f2d68aa28eed79a727be524c492ba Mon Sep 17 00:00:00 2001 From: ibaker Date: Fri, 13 Dec 2019 14:26:40 +0000 Subject: [PATCH] Add ISO-8859-1 awareness to IcyDecoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also change IcyInfo.rawMetatadata from String to byte[] ICY doesn't specify the character encoding, and there are streams not using UTF-8 (issue:#6753). It seems the default of at least one server is ISO-8859-1 so let's support that as a fallback: https://github.com/savonet/liquidsoap/issues/411#issuecomment-288759200 Also update IcyDecoder to skip strings it doesn't recognise at all instead of decoding invalid characters. The feed from issue:#6753 now decodes accents correctly: EventLogger: ICY: title="D Pai - Le temps de la rentrée", url="null" PiperOrigin-RevId: 285388522 --- RELEASENOTES.md | 4 + .../java/com/google/android/exoplayer2/C.java | 8 +- .../exoplayer2/metadata/icy/IcyDecoder.java | 57 +++++++-- .../exoplayer2/metadata/icy/IcyInfo.java | 26 ++-- .../metadata/icy/IcyDecoderTest.java | 115 ++++++++++++++---- .../exoplayer2/metadata/icy/IcyInfoTest.java | 4 +- 6 files changed, 160 insertions(+), 54 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 619807c90ab..26cbb991379 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -26,6 +26,10 @@ (e.g. subtitles). * Add `Player.getCurrentLiveOffset` to conveniently return the live offset. * Propagate HTTP request headers through `CacheDataSource`. +* Update `IcyDecoder` to try ISO-8859-1 decoding if UTF-8 decoding fails. + Also change `IcyInfo.rawMetadata` from `String` to `byte[]` to allow + developers to handle data that's neither UTF-8 nor ISO-8859-1 + ([#6753](https://github.com/google/ExoPlayer/issues/6753)). ### 2.11.0 (2019-12-11) ### diff --git a/library/core/src/main/java/com/google/android/exoplayer2/C.java b/library/core/src/main/java/com/google/android/exoplayer2/C.java index 567ce98b1a5..9661b7d0727 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/C.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/C.java @@ -95,14 +95,16 @@ private C() {} * The name of the ASCII charset. */ public static final String ASCII_NAME = "US-ASCII"; + /** * The name of the UTF-8 charset. */ public static final String UTF8_NAME = "UTF-8"; - /** - * The name of the UTF-16 charset. - */ + /** The name of the ISO-8859-1 charset. */ + public static final String ISO88591_NAME = "ISO-8859-1"; + + /** The name of the UTF-16 charset. */ public static final String UTF16_NAME = "UTF-16"; /** The name of the UTF-16 little-endian charset. */ diff --git a/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyDecoder.java b/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyDecoder.java index 3834dce583e..854a8fc3a47 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyDecoder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyDecoder.java @@ -16,13 +16,16 @@ package com.google.android.exoplayer2.metadata.icy; import androidx.annotation.Nullable; -import androidx.annotation.VisibleForTesting; +import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.metadata.Metadata; import com.google.android.exoplayer2.metadata.MetadataDecoder; import com.google.android.exoplayer2.metadata.MetadataInputBuffer; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.Charset; +import java.nio.charset.CharsetDecoder; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -33,24 +36,33 @@ public final class IcyDecoder implements MetadataDecoder { private static final String STREAM_KEY_NAME = "streamtitle"; private static final String STREAM_KEY_URL = "streamurl"; + private final CharsetDecoder utf8Decoder; + private final CharsetDecoder iso88591Decoder; + + public IcyDecoder() { + utf8Decoder = Charset.forName(C.UTF8_NAME).newDecoder(); + iso88591Decoder = Charset.forName(C.ISO88591_NAME).newDecoder(); + } + @Override @SuppressWarnings("ByteBufferBackingArray") public Metadata decode(MetadataInputBuffer inputBuffer) { ByteBuffer buffer = Assertions.checkNotNull(inputBuffer.data); - byte[] data = buffer.array(); - int length = buffer.limit(); - return decode(Util.fromUtf8Bytes(data, 0, length)); - } + @Nullable String icyString = decodeToString(buffer); + byte[] icyBytes = new byte[buffer.limit()]; + buffer.get(icyBytes); + + if (icyString == null) { + return new Metadata(new IcyInfo(icyBytes, /* title= */ null, /* url= */ null)); + } - @VisibleForTesting - /* package */ Metadata decode(String metadata) { @Nullable String name = null; @Nullable String url = null; int index = 0; - Matcher matcher = METADATA_ELEMENT.matcher(metadata); + Matcher matcher = METADATA_ELEMENT.matcher(icyString); while (matcher.find(index)) { - String key = Util.toLowerInvariant(matcher.group(1)); - String value = matcher.group(2); + @Nullable String key = Util.toLowerInvariant(matcher.group(1)); + @Nullable String value = matcher.group(2); switch (key) { case STREAM_KEY_NAME: name = value; @@ -61,6 +73,29 @@ public Metadata decode(MetadataInputBuffer inputBuffer) { } index = matcher.end(); } - return new Metadata(new IcyInfo(metadata, name, url)); + return new Metadata(new IcyInfo(icyBytes, name, url)); + } + + // The ICY spec doesn't specify a character encoding, and there's no way to communicate one + // either. So try decoding UTF-8 first, then fall back to ISO-8859-1. + // https://github.com/google/ExoPlayer/issues/6753 + @Nullable + private String decodeToString(ByteBuffer data) { + try { + return utf8Decoder.decode(data).toString(); + } catch (CharacterCodingException e) { + // Fall through to try ISO-8859-1 decoding. + } finally { + utf8Decoder.reset(); + data.rewind(); + } + try { + return iso88591Decoder.decode(data).toString(); + } catch (CharacterCodingException e) { + return null; + } finally { + iso88591Decoder.reset(); + data.rewind(); + } } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyInfo.java b/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyInfo.java index 1198d1af8be..1a3ed2ea6df 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyInfo.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/metadata/icy/IcyInfo.java @@ -20,34 +20,34 @@ import androidx.annotation.Nullable; import com.google.android.exoplayer2.metadata.Metadata; import com.google.android.exoplayer2.util.Assertions; -import com.google.android.exoplayer2.util.Util; +import java.util.Arrays; /** ICY in-stream information. */ public final class IcyInfo implements Metadata.Entry { - /** The complete metadata string used to construct this IcyInfo. */ - public final String rawMetadata; - /** The stream title if present, or {@code null}. */ + /** The complete metadata bytes used to construct this IcyInfo. */ + public final byte[] rawMetadata; + /** The stream title if present and decodable, or {@code null}. */ @Nullable public final String title; - /** The stream URL if present, or {@code null}. */ + /** The stream URL if present and decodable, or {@code null}. */ @Nullable public final String url; /** - * Construct a new IcyInfo from the source metadata string, and optionally a StreamTitle and - * StreamUrl that have been extracted. + * Construct a new IcyInfo from the source metadata, and optionally a StreamTitle and StreamUrl + * that have been extracted. * * @param rawMetadata See {@link #rawMetadata}. * @param title See {@link #title}. * @param url See {@link #url}. */ - public IcyInfo(String rawMetadata, @Nullable String title, @Nullable String url) { + public IcyInfo(byte[] rawMetadata, @Nullable String title, @Nullable String url) { this.rawMetadata = rawMetadata; this.title = title; this.url = url; } /* package */ IcyInfo(Parcel in) { - rawMetadata = Assertions.checkNotNull(in.readString()); + rawMetadata = Assertions.checkNotNull(in.createByteArray()); title = in.readString(); url = in.readString(); } @@ -62,26 +62,26 @@ public boolean equals(@Nullable Object obj) { } IcyInfo other = (IcyInfo) obj; // title & url are derived from rawMetadata, so no need to include them in the comparison. - return Util.areEqual(rawMetadata, other.rawMetadata); + return Arrays.equals(rawMetadata, other.rawMetadata); } @Override public int hashCode() { // title & url are derived from rawMetadata, so no need to include them in the hash. - return rawMetadata.hashCode(); + return Arrays.hashCode(rawMetadata); } @Override public String toString() { return String.format( - "ICY: title=\"%s\", url=\"%s\", rawMetadata=\"%s\"", title, url, rawMetadata); + "ICY: title=\"%s\", url=\"%s\", rawMetadata.length=\"%s\"", title, url, rawMetadata.length); } // Parcelable implementation. @Override public void writeToParcel(Parcel dest, int flags) { - dest.writeString(rawMetadata); + dest.writeByteArray(rawMetadata); dest.writeString(title); dest.writeString(url); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyDecoderTest.java b/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyDecoderTest.java index 72237d665c8..fcb958e7a6b 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyDecoderTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyDecoderTest.java @@ -16,9 +16,15 @@ package com.google.android.exoplayer2.metadata.icy; import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.ISO_8859_1; +import static java.nio.charset.StandardCharsets.UTF_16; +import static java.nio.charset.StandardCharsets.UTF_8; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.metadata.Metadata; +import com.google.android.exoplayer2.metadata.MetadataInputBuffer; +import com.google.android.exoplayer2.testutil.TestUtil; +import java.nio.ByteBuffer; import org.junit.Test; import org.junit.runner.RunWith; @@ -26,11 +32,13 @@ @RunWith(AndroidJUnit4.class) public final class IcyDecoderTest { + private final IcyDecoder decoder = new IcyDecoder(); + @Test public void decode() { - IcyDecoder decoder = new IcyDecoder(); - String icyContent = "StreamTitle='test title';StreamURL='test_url';"; - Metadata metadata = decoder.decode(icyContent); + byte[] icyContent = "StreamTitle='test title';StreamURL='test_url';".getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); @@ -39,11 +47,29 @@ public void decode() { assertThat(streamInfo.url).isEqualTo("test_url"); } + @Test + // Check the decoder is reading MetadataInputBuffer.data.limit() correctly. + public void decode_respectsLimit() { + byte[] icyTitle = "StreamTitle='test title';".getBytes(UTF_8); + byte[] icyUrl = "StreamURL='test_url';".getBytes(UTF_8); + byte[] paddedRawBytes = TestUtil.joinByteArrays(icyTitle, icyUrl); + MetadataInputBuffer metadataBuffer = createMetadataInputBuffer(paddedRawBytes); + // Stop before the stream URL. + metadataBuffer.data.limit(icyTitle.length); + Metadata metadata = decoder.decode(metadataBuffer); + + assertThat(metadata.length()).isEqualTo(1); + IcyInfo streamInfo = (IcyInfo) metadata.get(0); + assertThat(streamInfo.rawMetadata).isEqualTo(icyTitle); + assertThat(streamInfo.title).isEqualTo("test title"); + assertThat(streamInfo.url).isNull(); + } + @Test public void decode_titleOnly() { - IcyDecoder decoder = new IcyDecoder(); - String icyContent = "StreamTitle='test title';"; - Metadata metadata = decoder.decode(icyContent); + byte[] icyContent = "StreamTitle='test title';".getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); @@ -54,10 +80,11 @@ public void decode_titleOnly() { @Test public void decode_extraTags() { - String icyContent = - "StreamTitle='test title';StreamURL='test_url';CustomTag|withWeirdSeparator"; - IcyDecoder decoder = new IcyDecoder(); - Metadata metadata = decoder.decode(icyContent); + byte[] icyContent = + "StreamTitle='test title';StreamURL='test_url';CustomTag|withWeirdSeparator" + .getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); @@ -68,9 +95,9 @@ public void decode_extraTags() { @Test public void decode_emptyTitle() { - IcyDecoder decoder = new IcyDecoder(); - String icyContent = "StreamTitle='';StreamURL='test_url';"; - Metadata metadata = decoder.decode(icyContent); + byte[] icyContent = "StreamTitle='';StreamURL='test_url';".getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); @@ -81,9 +108,9 @@ public void decode_emptyTitle() { @Test public void decode_semiColonInTitle() { - IcyDecoder decoder = new IcyDecoder(); - String icyContent = "StreamTitle='test; title';StreamURL='test_url';"; - Metadata metadata = decoder.decode(icyContent); + byte[] icyContent = "StreamTitle='test; title';StreamURL='test_url';".getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); @@ -94,9 +121,9 @@ public void decode_semiColonInTitle() { @Test public void decode_quoteInTitle() { - IcyDecoder decoder = new IcyDecoder(); - String icyContent = "StreamTitle='test' title';StreamURL='test_url';"; - Metadata metadata = decoder.decode(icyContent); + byte[] icyContent = "StreamTitle='test' title';StreamURL='test_url';".getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); @@ -107,9 +134,9 @@ public void decode_quoteInTitle() { @Test public void decode_lineTerminatorInTitle() { - IcyDecoder decoder = new IcyDecoder(); - String icyContent = "StreamTitle='test\r\ntitle';StreamURL='test_url';"; - Metadata metadata = decoder.decode(icyContent); + byte[] icyContent = "StreamTitle='test\r\ntitle';StreamURL='test_url';".getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); @@ -119,14 +146,50 @@ public void decode_lineTerminatorInTitle() { } @Test - public void decode_noReconisedHeaders() { - IcyDecoder decoder = new IcyDecoder(); - Metadata metadata = decoder.decode("NotIcyData"); + public void decode_iso885911() { + // Create an invalid UTF-8 string by using 'é'. + byte[] icyContent = "StreamTitle='tést';StreamURL='tést_url';".getBytes(ISO_8859_1); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); + + assertThat(metadata.length()).isEqualTo(1); + IcyInfo streamInfo = (IcyInfo) metadata.get(0); + assertThat(streamInfo.rawMetadata).isEqualTo(icyContent); + assertThat(streamInfo.title).isEqualTo("tést"); + assertThat(streamInfo.url).isEqualTo("tést_url"); + } + + @Test + public void decode_unrecognisedEncoding() { + // Create an invalid UTF-8 and ISO-88591-1 string by using 'é'. + byte[] icyContent = "StreamTitle='tést';StreamURL='tést_url';".getBytes(UTF_16); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); assertThat(metadata.length()).isEqualTo(1); IcyInfo streamInfo = (IcyInfo) metadata.get(0); - assertThat(streamInfo.rawMetadata).isEqualTo("NotIcyData"); + assertThat(streamInfo.rawMetadata).isEqualTo(icyContent); assertThat(streamInfo.title).isNull(); assertThat(streamInfo.url).isNull(); } + + @Test + public void decode_noRecognisedHeaders() { + byte[] icyContent = "NotIcyData".getBytes(UTF_8); + + Metadata metadata = decoder.decode(createMetadataInputBuffer(icyContent)); + + assertThat(metadata.length()).isEqualTo(1); + IcyInfo streamInfo = (IcyInfo) metadata.get(0); + assertThat(streamInfo.rawMetadata).isEqualTo(icyContent); + assertThat(streamInfo.title).isNull(); + assertThat(streamInfo.url).isNull(); + } + + private static MetadataInputBuffer createMetadataInputBuffer(byte[] data) { + MetadataInputBuffer metadataInputBuffer = new MetadataInputBuffer(); + metadataInputBuffer.data = ByteBuffer.allocate(data.length).put(data); + metadataInputBuffer.data.flip(); + return metadataInputBuffer; + } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyInfoTest.java b/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyInfoTest.java index 2c8e6616c97..5f9b1a931f9 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyInfoTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/metadata/icy/IcyInfoTest.java @@ -16,6 +16,7 @@ package com.google.android.exoplayer2.metadata.icy; import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; import android.os.Parcel; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -28,7 +29,8 @@ public final class IcyInfoTest { @Test public void parcelEquals() { - IcyInfo streamInfo = new IcyInfo("StreamName='name';StreamUrl='url'", "name", "url"); + IcyInfo streamInfo = + new IcyInfo("StreamName='name';StreamUrl='url'".getBytes(UTF_8), "name", "url"); // Write to parcel. Parcel parcel = Parcel.obtain(); streamInfo.writeToParcel(parcel, 0);