diff --git a/RELEASENOTES.md b/RELEASENOTES.md index da99134082e..5d11f7770f2 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -112,6 +112,10 @@ be changed during playback with `Player.replaceMediaItem(int, MediaItem)` without interrupting playback ([#1976](https://github.com/androidx/media/issues/1976)). + * SubRip: Add support for injecting a `CharsetDetector` into + `DefaultSubtitleParserFactory` to detect the character encoding of + standalone SubRip subtitles without a byte order mark + ([#2247](https://github.com/androidx/media/issues/2247)). * Metadata: * Image: * DataSource: diff --git a/libraries/common/src/main/java/androidx/media3/common/util/ParsableByteArray.java b/libraries/common/src/main/java/androidx/media3/common/util/ParsableByteArray.java index c936a14f9bc..921f01e2e34 100644 --- a/libraries/common/src/main/java/androidx/media3/common/util/ParsableByteArray.java +++ b/libraries/common/src/main/java/androidx/media3/common/util/ParsableByteArray.java @@ -16,6 +16,7 @@ package androidx.media3.common.util; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; @@ -783,6 +784,14 @@ public Charset readUtfCharsetFromBom() { return null; } + /** + * Returns whether the provided {@link Charset} is supported when passed to methods like {@link + * #peekChar(Charset)} and {@link #readLine(Charset)}. + */ + public static boolean isCharsetSupported(Charset charset) { + return SUPPORTED_CHARSETS_FOR_READLINE.contains(checkNotNull(charset)); + } + /** * Sets whether all read/peek methods should enforce that {@link #getPosition()} never exceeds * {@link #limit()}. diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/CharsetDetector.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/CharsetDetector.java new file mode 100644 index 00000000000..0d6fb82d701 --- /dev/null +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/CharsetDetector.java @@ -0,0 +1,38 @@ +/* + * Copyright 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package androidx.media3.extractor.text; + +import androidx.annotation.Nullable; +import androidx.media3.common.util.UnstableApi; +import java.nio.charset.Charset; + +/** Detects the character encoding of subtitle byte data. */ +@UnstableApi +public interface CharsetDetector { + + /** + * Detects the character encoding of the requested range of {@code data}. + * + *

The requested range is not guaranteed to contain a complete subtitle file. + * + * @param data The subtitle byte data. + * @param offset The start offset in {@code data}. + * @param length The number of bytes to inspect. + * @return The detected character encoding, or {@code null} if it could not be determined. + */ + @Nullable + Charset detect(byte[] data, int offset, int length); +} diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/DefaultSubtitleParserFactory.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/DefaultSubtitleParserFactory.java index b2413e3be55..60ac5a90bf8 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/DefaultSubtitleParserFactory.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/DefaultSubtitleParserFactory.java @@ -50,6 +50,22 @@ @UnstableApi public final class DefaultSubtitleParserFactory implements SubtitleParser.Factory { + @Nullable private final CharsetDetector charsetDetector; + + /** Creates an instance. */ + public DefaultSubtitleParserFactory() { + this(/* charsetDetector= */ null); + } + + /** + * Creates an instance that passes {@code charsetDetector} to delegate factories (where relevant). + * + * @param charsetDetector The detector to use, or {@code null}. + */ + public DefaultSubtitleParserFactory(@Nullable CharsetDetector charsetDetector) { + this.charsetDetector = charsetDetector; + } + @Override public boolean supportsFormat(Format format) { @Nullable String mimeType = format.sampleMimeType; @@ -106,7 +122,7 @@ public SubtitleParser create(Format format) { case MimeTypes.APPLICATION_MP4VTT: return new Mp4WebvttParser(); case MimeTypes.APPLICATION_SUBRIP: - return new SubripParser(); + return new SubripParser(charsetDetector, format); case MimeTypes.APPLICATION_TX3G: return new Tx3gParser(format.initializationData); case MimeTypes.APPLICATION_PGS: diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/subrip/SubripParser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/subrip/SubripParser.java index 8f607ed6357..77be5ce7679 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/subrip/SubripParser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/subrip/SubripParser.java @@ -26,11 +26,13 @@ import androidx.media3.common.C; import androidx.media3.common.Format; import androidx.media3.common.Format.CueReplacementBehavior; +import androidx.media3.common.MimeTypes; import androidx.media3.common.text.Cue; import androidx.media3.common.util.Consumer; import androidx.media3.common.util.Log; import androidx.media3.common.util.ParsableByteArray; import androidx.media3.common.util.UnstableApi; +import androidx.media3.extractor.text.CharsetDetector; import androidx.media3.extractor.text.CuesWithTiming; import androidx.media3.extractor.text.SubtitleParser; import com.google.common.collect.ImmutableList; @@ -82,11 +84,28 @@ public final class SubripParser implements SubtitleParser { private final StringBuilder textBuilder; private final ArrayList tags; private final ParsableByteArray parsableByteArray; + @Nullable private final CharsetDetector charsetDetector; + private final Format format; + /** Creates an instance. */ public SubripParser() { + this(/* charsetDetector= */ null, new Format.Builder().build()); + } + + /** + * Creates an instance that uses {@code charsetDetector} when the charset of the input is + * ambiguous. + * + * @param charsetDetector The detector to use, or {@code null} to default to UTF-8 when the + * charset of the input can't be determined by other means. + * @param format The format of the subrip track. + */ + public SubripParser(@Nullable CharsetDetector charsetDetector, Format format) { textBuilder = new StringBuilder(); tags = new ArrayList<>(); parsableByteArray = new ParsableByteArray(); + this.charsetDetector = charsetDetector; + this.format = format; } @Override @@ -103,7 +122,19 @@ public void parse( Consumer output) { parsableByteArray.reset(data, /* limit= */ offset + length); parsableByteArray.setPosition(offset); - Charset charset = detectUtfCharset(parsableByteArray); + Charset charset = detectCharset(); + if (!ParsableByteArray.isCharsetSupported(charset)) { + // ParsableByteArray doesn't support directly reading the detected charset, so we transcode + // the data to UTF-8 (which is supported). + parsableByteArray.reset( + new String( + parsableByteArray.getData(), + parsableByteArray.getPosition(), + parsableByteArray.bytesLeft(), + charset) + .getBytes(StandardCharsets.UTF_8)); + charset = StandardCharsets.UTF_8; + } @Nullable List cuesWithTimingBeforeRequestedStartTimeUs = @@ -188,11 +219,35 @@ public void parse( } /** - * Determine UTF encoding of the byte array from a byte order mark (BOM), defaulting to UTF-8 if - * no BOM is found. + * Returns the charset to use for line parsing of {@link #parsableByteArray}. + * + *

The result is derived from, in order: + * + *

    + *
  1. Content from a Matroska container is assumed to always be UTF-8 (per spec). + *
  2. A byte order mark in the data, if present. + *
  3. The result of calling {@link #charsetDetector}, if non-null. + *
  4. Fallback to assuming UTF-8. + *
*/ - private Charset detectUtfCharset(ParsableByteArray data) { - @Nullable Charset charset = data.readUtfCharsetFromBom(); + private Charset detectCharset() { + if (MimeTypes.isMatroska(format.containerMimeType)) { + // Subrip muxed into a Matroska container is always UTF-8. + return StandardCharsets.UTF_8; + } + @Nullable Charset utfCharset = parsableByteArray.readUtfCharsetFromBom(); + if (utfCharset != null) { + return utfCharset; + } + if (charsetDetector == null) { + return StandardCharsets.UTF_8; + } + @Nullable + Charset charset = + charsetDetector.detect( + parsableByteArray.getData(), + parsableByteArray.getPosition(), + parsableByteArray.bytesLeft()); return charset != null ? charset : StandardCharsets.UTF_8; } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/subrip/SubripParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/subrip/SubripParserTest.java index b8cab4f943c..6ef7d5a6ac9 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/subrip/SubripParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/subrip/SubripParserTest.java @@ -17,24 +17,39 @@ import static androidx.media3.common.Format.CUE_REPLACEMENT_BEHAVIOR_MERGE; import static com.google.common.truth.Truth.assertThat; - +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import androidx.media3.common.Format; +import androidx.media3.common.MimeTypes; import androidx.media3.common.text.Cue; +import androidx.media3.extractor.text.CharsetDetector; import androidx.media3.extractor.text.CuesWithTiming; import androidx.media3.extractor.text.SubtitleParser; import androidx.media3.extractor.text.SubtitleParser.OutputOptions; import androidx.media3.test.utils.TestUtil; import androidx.test.core.app.ApplicationProvider; -import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import com.google.common.primitives.Bytes; +import com.google.testing.junit.testparameterinjector.TestParameter; import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestParameterInjector; /** Unit test for {@link SubripParser}. */ -@RunWith(AndroidJUnit4.class) +@RunWith(RobolectricTestParameterInjector.class) public final class SubripParserTest { private static final String EMPTY_FILE = "media/subrip/empty"; @@ -54,6 +69,9 @@ public final class SubripParserTest { "media/subrip/typical_no_hours_and_millis"; private static final String TYPICAL_BAD_TIMESTAMPS = "media/subrip/typical_bad_timestamps"; + private static final Format STANDALONE_SUBRIP_FORMAT = + new Format.Builder().setContainerMimeType(MimeTypes.APPLICATION_SUBRIP).build(); + @Test public void cueReplacementBehaviorIsMerge() { SubripParser parser = new SubripParser(); @@ -132,8 +150,46 @@ public void parseTypical_cuesAfterTimeThenCuesBefore() throws IOException { } @Test - public void parseTypicalWithByteOrderMark() throws IOException { - SubripParser parser = new SubripParser(); + public void parseTypical_charsetDetectorReturnsNull_assumesUtf8() throws IOException { + SubripParser parser = + new SubripParser( + /* charsetDetector= */ (data, offset, length) -> null, STANDALONE_SUBRIP_FORMAT); + byte[] bytes = TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), TYPICAL_FILE); + + ImmutableList allCues = parseAllCues(parser, bytes); + + assertThat(allCues).hasSize(3); + assertTypicalCue1(allCues.get(0)); + assertTypicalCue2(allCues.get(1)); + assertTypicalCue3(allCues.get(2)); + } + + @Test + public void parseTypicalWithByteOrderMark_doesNotCallCharsetDetector() throws IOException { + CharsetDetector charsetDetector = mock(CharsetDetector.class); + SubripParser parser = new SubripParser(charsetDetector, STANDALONE_SUBRIP_FORMAT); + byte[] bytes = + TestUtil.getByteArray( + ApplicationProvider.getApplicationContext(), TYPICAL_WITH_BYTE_ORDER_MARK); + + ImmutableList allCues = parseAllCues(parser, bytes); + + assertThat(allCues).hasSize(3); + assertTypicalCue1(allCues.get(0)); + assertTypicalCue2(allCues.get(1)); + assertTypicalCue3(allCues.get(2)); + verify(charsetDetector, never()) + .detect(/* data= */ any(), /* offset= */ anyInt(), /* length= */ anyInt()); + } + + @Test + public void parseTypicalWithByteOrderMark_inMkvContainer_doesNotCallCharsetDetector() + throws IOException { + CharsetDetector charsetDetector = mock(CharsetDetector.class); + SubripParser parser = + new SubripParser( + charsetDetector, + new Format.Builder().setContainerMimeType(MimeTypes.VIDEO_MATROSKA).build()); byte[] bytes = TestUtil.getByteArray( ApplicationProvider.getApplicationContext(), TYPICAL_WITH_BYTE_ORDER_MARK); @@ -144,6 +200,8 @@ public void parseTypicalWithByteOrderMark() throws IOException { assertTypicalCue1(allCues.get(0)); assertTypicalCue2(allCues.get(1)); assertTypicalCue3(allCues.get(2)); + verify(charsetDetector, never()) + .detect(/* data= */ any(), /* offset= */ anyInt(), /* length= */ anyInt()); } @Test @@ -233,6 +291,28 @@ public void parseTypicalUtf16LittleEndian() throws IOException { assertTypicalCue3(allCues.get(2)); } + @Test + public void parseTypicalUtf16LittleEndian_inMkv_doesNotCallCharsetDetector_producesGarbage() + throws IOException { + CharsetDetector charsetDetector = mock(CharsetDetector.class); + SubripParser parser = + new SubripParser( + charsetDetector, + new Format.Builder().setContainerMimeType(MimeTypes.VIDEO_MATROSKA).build()); + byte[] bytes = + TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), TYPICAL_UTF16LE); + + ImmutableList _ = parseAllCues(parser, bytes); + + // Don't assert the specific output, since it's effectively undefined behaviour. This test + // exists to assert we don't call CharsetDetector for every subtitle sample from a container + // where the charset is defined by the spec - because most CharsetDetector implementations + // are statistical (and so rely on longer documents than a single subtitle) and also potentially + // slow (so we don't want to call it for every text sample). + verify(charsetDetector, never()) + .detect(/* data= */ any(), /* offset= */ anyInt(), /* length= */ anyInt()); + } + @Test public void parseTypicalUtf16BigEndian() throws IOException { SubripParser parser = new SubripParser(); @@ -303,6 +383,64 @@ public void parseTypicalBadTimestamps() throws IOException { assertTypicalCue1(Iterables.getOnlyElement(allCues)); } + private enum CharsetTestCase { + GB18030("起来 快起来", Charset.forName("GB18030")), + EUC_KR("이것은 한국어 자막 테스트입니다.", Charset.forName("EUC-KR")), + SHIFT_JIS("これは日本語の字幕テストです。", Charset.forName("Shift_JIS")), + UTF_8("This is a UTF-8 subtitle.", StandardCharsets.UTF_8), + US_ASCII("This is an ASCII subtitle.", StandardCharsets.US_ASCII); + + private final Charset charset; + private final String text; + + private CharsetTestCase(String text, Charset charset) { + this.charset = charset; + this.text = text; + } + } + + @Test + public void parse_withCharsetDetector_outputsDecodedText( + @TestParameter CharsetTestCase testCase) { + byte[] bytes = + ("1\r\n" + "00:00:00,000 --> 00:00:05,000\r\n" + testCase.text + "\r\n") + .getBytes(testCase.charset); + SubripParser parser = + new SubripParser( + /* charsetDetector= */ (data, offset, length) -> testCase.charset, + STANDALONE_SUBRIP_FORMAT); + + ImmutableList allCues = parseAllCues(parser, bytes); + + assertThat(allCues).hasSize(1); + assertThat(allCues.get(0).cues.get(0).text.toString()).isEqualTo(testCase.text); + } + + @Test + public void parse_atOffsetWithCharsetDetector_passesRequestedRangeToDetector() { + Charset charset = Charset.forName("GB18030"); + int offset = 5; + int padding = 7; + String text = "这是一个字幕测试。"; + byte[] bytes = + Bytes.concat( + new byte[offset], + ("1\r\n" + "00:00:00,000 --> 00:00:05,000\r\n" + text + "\r\n").getBytes(charset), + new byte[padding]); + int subtitleLength = bytes.length - offset - padding; + CharsetDetector charsetDetector = mock(CharsetDetector.class); + when(charsetDetector.detect(/* data= */ any(), /* offset= */ anyInt(), /* length= */ anyInt())) + .thenReturn(charset); + SubripParser parser = new SubripParser(charsetDetector, STANDALONE_SUBRIP_FORMAT); + + List allCues = new ArrayList<>(); + parser.parse(bytes, offset, subtitleLength, OutputOptions.allCues(), allCues::add); + + assertThat(allCues).hasSize(1); + assertThat(allCues.get(0).cues.get(0).text.toString()).isEqualTo(text); + verify(charsetDetector).detect(same(bytes), eq(offset), eq(subtitleLength)); + } + private static ImmutableList parseAllCues(SubtitleParser parser, byte[] data) { ImmutableList.Builder cues = ImmutableList.builder(); parser.parse(data, OutputOptions.allCues(), cues::add);