From 7b450f0d0a75f14b2bea3d975671e6f5d2948e15 Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 1 Jan 2024 20:18:34 -0800 Subject: [PATCH] Fix limit calculation to include `offset` in `Cea6/708Parser.parse` PiperOrigin-RevId: 595007390 --- .../extractor/text/cea/Cea608Parser.java | 2 +- .../extractor/text/cea/Cea708Parser.java | 2 +- .../extractor/text/cea/Cea608ParserTest.java | 59 ++++++++++++++++++- .../extractor/text/cea/Cea708ParserTest.java | 48 ++++++++++++--- 4 files changed, 101 insertions(+), 10 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Parser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Parser.java index 5d24fbe9a3..0489383b79 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Parser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Parser.java @@ -439,7 +439,7 @@ public final class Cea608Parser implements SubtitleParser { int length, OutputOptions outputOptions, Consumer output) { - ccData.reset(data, length); + ccData.reset(data, offset + length); ccData.setPosition(offset); boolean captionDataProcessed = false; while (ccData.bytesLeft() >= packetLength) { diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java index 72e9e3194b..e5ff146f52 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea708Parser.java @@ -215,7 +215,7 @@ public final class Cea708Parser implements SubtitleParser { int length, OutputOptions outputOptions, Consumer output) { - ccData.reset(data, length); + ccData.reset(data, offset + length); ccData.setPosition(offset); while (ccData.bytesLeft() >= 3) { int ccTypeAndValid = (ccData.readUnsignedByte() & 0x07); diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea608ParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea608ParserTest.java index 21d3ccde9b..98e81999c5 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea608ParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea608ParserTest.java @@ -75,6 +75,53 @@ public class Cea608ParserTest { .isEqualTo("test subtitle, spans 2 samples"); } + @Test + public void paintOnEmitsSubtitlesImmediately_respectsOffsetAndLimit() throws Exception { + Cea608Parser cea608Parser = + new Cea608Parser( + MimeTypes.APPLICATION_CEA608, + /* accessibilityChannel= */ 1, + Cea608Parser.MIN_DATA_CHANNEL_TIMEOUT_MS); + byte[] sample1 = + Bytes.concat( + // 'paint on' control character + createPacket(0xFC, 0x14, 0x29), + createPacket(0xFC, 't', 'e'), + createPacket(0xFC, 's', 't'), + createPacket(0xFC, ' ', 's'), + createPacket(0xFC, 'u', 'b'), + createPacket(0xFC, 't', 'i'), + createPacket(0xFC, 't', 'l'), + createPacket(0xFC, 'e', ','), + createPacket(0xFC, ' ', 's'), + createPacket(0xFC, 'p', 'a')); + byte[] sample2 = + Bytes.concat( + createPacket(0xFC, 'n', 's'), + createPacket(0xFC, ' ', '2'), + createPacket(0xFC, ' ', 's'), + createPacket(0xFC, 'a', 'm'), + createPacket(0xFC, 'p', 'l'), + createPacket(0xFC, 'e', 's')); + byte[] bothSamples = Bytes.concat(sample1, sample2); + + CuesWithTiming firstCues = + checkNotNull( + parseSample(cea608Parser, bothSamples, /* offset= */ 0, /* length= */ sample1.length)); + CuesWithTiming secondCues = + checkNotNull( + parseSample( + cea608Parser, + bothSamples, + /* offset= */ sample1.length, + /* length= */ sample2.length)); + + assertThat(Iterables.getOnlyElement(firstCues.cues).text.toString()) + .isEqualTo("test subtitle, spa"); + assertThat(Iterables.getOnlyElement(secondCues.cues).text.toString()) + .isEqualTo("test subtitle, spans 2 samples"); + } + @Test public void rollUpEmitsSubtitlesImmediately() throws Exception { Cea608Parser cea608Parser = @@ -335,8 +382,18 @@ public class Cea608ParserTest { @Nullable private static CuesWithTiming parseSample(Cea608Parser parser, byte[] sample) throws SubtitleDecoderException { + return parseSample(parser, sample, /* offset= */ 0, /* length= */ sample.length); + } + + /** + * Passes {@code sample} to {@link Cea608Parser#parse} and returns either the emitted {@link + * CuesWithTiming} or null if none was emitted. + */ + @Nullable + private static CuesWithTiming parseSample( + Cea608Parser parser, byte[] sample, int offset, int length) { List result = new ArrayList<>(); - parser.parse(sample, OutputOptions.allCues(), result::add); + parser.parse(sample, offset, length, OutputOptions.allCues(), result::add); return result.isEmpty() ? null : Iterables.getOnlyElement(result); } } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java index cc5dfdac4f..3910fa0093 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/cea/Cea708ParserTest.java @@ -31,10 +31,8 @@ import com.google.common.primitives.Bytes; import com.google.common.primitives.UnsignedBytes; import java.util.ArrayList; import java.util.List; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.robolectric.shadows.ShadowLog; /** Tests for {@link Cea708Parser}. */ @RunWith(AndroidJUnit4.class) @@ -44,11 +42,6 @@ public class Cea708ParserTest { private static final byte CHANNEL_PACKET_DATA = 0x6; private static final byte CHANNEL_PACKET_END = 0x2; - @Before - public void setupLogging() { - ShadowLog.stream = System.out; - } - @Test public void singleServiceAndWindowDefinition() throws Exception { Cea708Parser cea708Parser = @@ -81,6 +74,47 @@ public class Cea708ParserTest { .isEqualTo("test subtitle"); } + @Test + public void singleServiceAndWindowDefinition_respectsOffsetAndLimit() throws Exception { + Cea708Parser cea708Parser = + new Cea708Parser( + /* accessibilityChannel= */ Format.NO_VALUE, /* initializationData= */ null); + byte[] windowDefinition = + TestUtil.createByteArray( + 0x98, // DF0 command (define window 0) + 0b0010_0000, // visible=true, row lock and column lock disabled, priority=0 + 0xF0 | 50, // relative positioning, anchor vertical + 50, // anchor horizontal + 10, // anchor point = 0, row count = 10 + 30, // column count = 30 + 0b0000_1001); // window style = 1, pen style = 1 + byte[] setCurrentWindow = TestUtil.createByteArray(0x80); // CW0 (set current window to 0) + byte[] subtitleData = + encodePacketIntoBytePairs( + createPacket( + /* sequenceNumber= */ 0, + createServiceBlock( + Bytes.concat( + windowDefinition, + setCurrentWindow, + "test subtitle".getBytes(Charsets.UTF_8))))); + byte[] garbagePrefix = TestUtil.buildTestData(subtitleData.length * 2); + byte[] garbageSuffix = TestUtil.buildTestData(10); + byte[] subtitleDataWithGarbagePrefixAndSuffix = + Bytes.concat(garbagePrefix, subtitleData, garbageSuffix); + + List result = new ArrayList<>(); + cea708Parser.parse( + subtitleDataWithGarbagePrefixAndSuffix, + garbagePrefix.length, + subtitleData.length, + SubtitleParser.OutputOptions.allCues(), + result::add); + + assertThat(Iterables.getOnlyElement(Iterables.getOnlyElement(result).cues).text.toString()) + .isEqualTo("test subtitle"); + } + /** See section 4.4.1 of the CEA-708-B spec. */ private static byte[] encodePacketIntoBytePairs(byte[] packet) { checkState(packet.length % 2 == 0);