From d6f08a6237ae8fdb2af1ebf2ac5bca5f0eb619ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sikora?= Date: Sat, 24 Aug 2024 01:44:31 +0200 Subject: [PATCH 1/8] Add VTT voice spans to cues --- .../media3/common/text/CustomSpanBundler.java | 11 +- .../media3/common/text/VoiceSpan.java | 62 +++++++++ .../common/text/CustomCueBundlerTest.java | 9 +- .../text/webvtt/WebvttCueParser.java | 10 +- .../text/webvtt/WebvttCueParserTest.java | 56 +++++++- .../test/utils/truth/SpannedSubject.java | 126 ++++++++++++++++++ .../test/utils/truth/SpannedSubjectTest.java | 81 +++++++++++ 7 files changed, 351 insertions(+), 4 deletions(-) create mode 100644 libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java diff --git a/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java b/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java index edcda586d2..d5f7d15e32 100644 --- a/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java +++ b/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java @@ -58,7 +58,7 @@ import java.util.ArrayList; @Documented @Retention(RetentionPolicy.SOURCE) @Target({TYPE_USE}) - @IntDef({UNKNOWN, RUBY, TEXT_EMPHASIS, HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT}) + @IntDef({UNKNOWN, RUBY, TEXT_EMPHASIS, HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT, VOICE}) private @interface CustomSpanType {} private static final int UNKNOWN = -1; @@ -69,6 +69,8 @@ import java.util.ArrayList; private static final int HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT = 3; + private static final int VOICE = 4; + private static final String FIELD_START_INDEX = Util.intToStringMaxRadix(0); private static final String FIELD_END_INDEX = Util.intToStringMaxRadix(1); private static final String FIELD_FLAGS = Util.intToStringMaxRadix(2); @@ -94,6 +96,10 @@ import java.util.ArrayList; text, span, /* spanType= */ HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT, /* params= */ null); bundledCustomSpans.add(bundle); } + for (VoiceSpan span : text.getSpans(0, text.length(), VoiceSpan.class)) { + Bundle bundle = spanToBundle(text, span, /* spanType= */ VOICE, /* params= */ span.toBundle()); + bundledCustomSpans.add(bundle); + } return bundledCustomSpans; } @@ -113,6 +119,9 @@ import java.util.ArrayList; case HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT: text.setSpan(new HorizontalTextInVerticalContextSpan(), start, end, flags); break; + case VOICE: + text.setSpan(VoiceSpan.fromBundle(checkNotNull(span)), start, end, flags); + break; default: break; } diff --git a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java new file mode 100644 index 0000000000..66f74b1004 --- /dev/null +++ b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2024 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.common.text; + +import static androidx.media3.common.util.Assertions.checkNotNull; + +import android.os.Bundle; +import androidx.media3.common.util.UnstableApi; +import androidx.media3.common.util.Util; +import com.google.common.collect.ImmutableSet; +import java.util.Set; + +/** + * A span representing a speaker. + * + *

More information on + * voice spans. + */ +@UnstableApi +public final class VoiceSpan implements LanguageFeatureSpan { + + /** The speaker name. */ + public final String speakerName; + + /** The classes associated with the text. It can specify things like "first", "loud", etc. */ + public final Set classes; + + private static final String FIELD_NAME = Util.intToStringMaxRadix(0); + private static final String FIELD_CLASSES = Util.intToStringMaxRadix(1); + + public VoiceSpan(String speakerName, Set classes) { + this.speakerName = speakerName; + this.classes = classes; + } + + public Bundle toBundle() { + Bundle bundle = new Bundle(); + bundle.putString(FIELD_NAME, speakerName); + bundle.putStringArray(FIELD_CLASSES, classes.toArray(new String[0])); + return bundle; + } + + public static VoiceSpan fromBundle(Bundle bundle) { + return new VoiceSpan( + /* speakerName = */ checkNotNull(bundle.getString(FIELD_NAME)), + /* classes = */ ImmutableSet.copyOf(checkNotNull(bundle.getStringArray(FIELD_CLASSES)))); + } +} diff --git a/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java b/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java index 45a8d2ab1e..a2b01343f9 100644 --- a/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java @@ -42,6 +42,7 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public class CustomCueBundlerTest { + private static final VoiceSpan VOICE_SPAN = new VoiceSpan("name", Set.of("first", "loud")); private static final RubySpan RUBY_SPAN = new RubySpan("ruby text", TextAnnotation.POSITION_AFTER); private static final TextEmphasisSpan TEXT_EMPHASIS_SPAN = @@ -55,7 +56,8 @@ public class CustomCueBundlerTest { ImmutableMap.of( RUBY_SPAN, new Pair<>(1, 2), TEXT_EMPHASIS_SPAN, new Pair<>(2, 3), - HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT_SPAN, new Pair<>(5, 7)); + HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT_SPAN, new Pair<>(5, 7), + VOICE_SPAN, new Pair<>(8, 10)); @Test public void serializingSpannableWithAllCustomSpans() { @@ -92,6 +94,11 @@ public class CustomCueBundlerTest { .hasHorizontalTextInVerticalContextSpanBetween( ALL_SPANS_TO_START_END_INDEX.get(HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT_SPAN).first, ALL_SPANS_TO_START_END_INDEX.get(HORIZONTAL_TEXT_IN_VERTICAL_CONTEXT_SPAN).second); + SpannedSubject.assertThat(result) + .hasVoiceSpanBetween( + ALL_SPANS_TO_START_END_INDEX.get(VOICE_SPAN).first, + ALL_SPANS_TO_START_END_INDEX.get(VOICE_SPAN).second) + .withSpeakerNameAndClasses(VOICE_SPAN.speakerName, VOICE_SPAN.classes); } @Test diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java index cbf44a94b6..b2a5527db5 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java @@ -43,6 +43,7 @@ import androidx.media3.common.text.HorizontalTextInVerticalContextSpan; import androidx.media3.common.text.RubySpan; import androidx.media3.common.text.SpanUtil; import androidx.media3.common.text.TextAnnotation; +import androidx.media3.common.text.VoiceSpan; import androidx.media3.common.util.Assertions; import androidx.media3.common.util.Log; import androidx.media3.common.util.ParsableByteArray; @@ -555,8 +556,10 @@ public final class WebvttCueParser { case TAG_CLASS: applyDefaultColors(text, startTag.classes, start, end); break; - case TAG_LANG: case TAG_VOICE: + applyVoiceSpan(text, startTag.voice, startTag.classes, start, end); + break; + case TAG_LANG: case "": // Case of the "whole cue" virtual tag. break; default: @@ -658,6 +661,11 @@ public final class WebvttCueParser { } } + private static void applyVoiceSpan( + SpannableStringBuilder text, String voice, Set classes, int start, int end) { + text.setSpan(new VoiceSpan(voice, classes), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + } + private static void applyStyleToText( SpannableStringBuilder spannedText, WebvttCssStyle style, int start, int end) { if (style == null) { diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java index e49bc8604b..0fd5eda714 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java @@ -22,6 +22,7 @@ import android.graphics.Color; import android.text.Spanned; import androidx.test.ext.junit.runners.AndroidJUnit4; import java.util.Collections; +import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; @@ -46,7 +47,7 @@ public final class WebvttCueParserTest { public void parseStrictValidUnsupportedTagsStrippedOut() throws Exception { Spanned text = parseCueText( - "This is text with " + "This is text with " + "html tags"); assertThat(text.toString()).isEqualTo("This is text with html tags"); @@ -242,6 +243,59 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("&&&&&&&"); } + @Test + public void parseEmptyVoiceSpan() throws Exception { + Spanned text = parseCueText("Text with a single voice span"); + + assertThat(text.toString()).isEqualTo("Text with a single voice span"); + assertThat(text) + .hasVoiceSpanBetween(0, "Text with a single voice span".length()) + .withSpeakerNameAndClasses("", Collections.emptySet()); + } + + @Test + public void parseVoiceSpanWithName() throws Exception { + Spanned text = parseCueText("Text with a single voice span"); + + assertThat(text.toString()).isEqualTo("Text with a single voice span"); + assertThat(text) + .hasVoiceSpanBetween(0, "Text with a single voice span".length()) + .withSpeakerNameAndClasses("Esme", Collections.emptySet()); + } + + @Test + public void parseVoiceSpanWithClasses() throws Exception { + Spanned text = parseCueText("Text with a single voice span"); + + assertThat(text.toString()).isEqualTo("Text with a single voice span"); + assertThat(text) + .hasVoiceSpanBetween(0, "Text with a single voice span".length()) + .withSpeakerNameAndClasses("", Set.of("first", "loud")); + } + + @Test + public void parseVoiceSpanWithNameAndClasses() throws Exception { + Spanned text = parseCueText("Text with a single voice span"); + + assertThat(text.toString()).isEqualTo("Text with a single voice span"); + assertThat(text) + .hasVoiceSpanBetween(0, "Text with a single voice span".length()) + .withSpeakerNameAndClasses("Esme", Set.of("first", "loud")); + } + + @Test + public void parseMultipleVoiceSpans() throws Exception { + Spanned text = parseCueText("Text with multiple voice spans"); + + assertThat(text.toString()).isEqualTo("Text with multiple voice spans"); + assertThat(text) + .hasVoiceSpanBetween(0, "Text with ".length()) + .withSpeakerNameAndClasses("Esme", Set.of("loud")); + assertThat(text) + .hasVoiceSpanBetween("Text with ".length(), "Text with multiple voice spans".length()) + .withSpeakerNameAndClasses("Mary", Set.of("quiet")); + } + private static Spanned parseCueText(String string) { return WebvttCueParser.parseCueText( /* id= */ null, string, /* styles= */ Collections.emptyList()); diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java index 43d7b9533e..a05c6fde82 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java @@ -42,6 +42,7 @@ import androidx.media3.common.text.HorizontalTextInVerticalContextSpan; import androidx.media3.common.text.RubySpan; import androidx.media3.common.text.TextAnnotation; import androidx.media3.common.text.TextEmphasisSpan; +import androidx.media3.common.text.VoiceSpan; import androidx.media3.common.util.NullableType; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; @@ -52,6 +53,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Objects; +import java.util.Set; import org.checkerframework.checker.nullness.qual.RequiresNonNull; /** A Truth {@link Subject} for assertions on {@link Spanned} instances containing text styling. */ @@ -634,6 +637,42 @@ public final class SpannedSubject extends Subject { hasNoSpansOfTypeBetween(HorizontalTextInVerticalContextSpan.class, start, end); } + /** + * Checks that the subject has an {@link VoiceSpan} from {@code start} to {@code end}. + * + * @param start The start of the expected span. + * @param end The end of the expected span. + * @return A {@link VoiceSpan} object for optional additional assertions on the flags. + */ + public VoiceText hasVoiceSpanBetween(int start, int end) { + if (actual == null) { + failWithoutActual(simpleFact("Spanned must not be null")); + return ALREADY_FAILED_WITH_NAME_AND_CLASSES; + } + + List voiceSpans = findMatchingSpans(start, end, VoiceSpan.class); + if (voiceSpans.size() == 1) { + return check("VoiceSpan (start=%s,end=%s)", start, end) + .about(voiceSpanSubjects(actual)) + .that(voiceSpans); + } + failWithExpectedSpan(start, end, VoiceSpan.class, actual.toString().substring(start, end)); + return ALREADY_FAILED_WITH_NAME_AND_CLASSES; + } + + /** + * Checks that the subject has no {@link VoiceSpan}s on any of the text + * between {@code start} and {@code end}. + * + *

This fails even if the start and end indexes don't exactly match. + * + * @param start The start index to start searching for spans. + * @param end The end index to stop searching for spans. + */ + public void hasNoVoiceSpanBetween(int start, int end) { + hasNoSpansOfTypeBetween(VoiceSpan.class, start, end); + } + /** * Checks that the subject has no spans of type {@code spanClazz} on any of the text between * {@code start} and {@code end}. @@ -1272,4 +1311,91 @@ public final class SpannedSubject extends Subject { } } } + + /** Allows assertions about a span's voice its position. */ + public interface VoiceText { + /** + * Checks that at least one of the matched spans has the expected {@code name} and {@code + * classes}. + * + * @param name The expected name of the voice. + * @param classes The classes used to style the voice. + * @return A {@link AndSpanFlags} object for optional additional assertions on the flags. + */ + AndSpanFlags withSpeakerNameAndClasses(String name, Set classes); + } + + private static final VoiceText ALREADY_FAILED_WITH_NAME_AND_CLASSES = + (name, classes) -> ALREADY_FAILED_AND_FLAGS; + + private static Factory> voiceSpanSubjects( + Spanned actualSpanned) { + return (FailureMetadata metadata, @Nullable List spans) -> + new VoiceSpanSubject(metadata, spans, actualSpanned); + } + + private static final class VoiceSpanSubject extends Subject implements VoiceText { + + @Nullable private final List actualSpans; + private final Spanned actualSpanned; + + private VoiceSpanSubject( + FailureMetadata metadata, + @Nullable List actualSpans, + Spanned actualSpanned) { + super(metadata, actualSpans); + this.actualSpans = actualSpans; + this.actualSpanned = actualSpanned; + } + + @Override + public AndSpanFlags withSpeakerNameAndClasses(String name, Set classes) { + List matchingSpanFlags = new ArrayList<>(); + List voiceSpeakerNameAndClasses = new ArrayList<>(); + for (VoiceSpan span : checkNotNull(actualSpans)) { + voiceSpeakerNameAndClasses.add(new SpeakerNameAndClasses(span.speakerName, span.classes)); + if (span.speakerName.equals(name) && span.classes.equals(classes)) { + matchingSpanFlags.add(actualSpanned.getSpanFlags(span)); + } + } + check("voiceSpeakerNameAndClasses") + .that(voiceSpeakerNameAndClasses) + .containsExactly(new SpeakerNameAndClasses(name, classes)); + return check("flags").about(spanFlags()).that(matchingSpanFlags); + } + + private static final class SpeakerNameAndClasses { + + private final String speakerName; + private final Set classes; + + private SpeakerNameAndClasses(String name, Set classes) { + this.speakerName = name; + this.classes = classes; + } + + @Override + public boolean equals(@Nullable Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + SpeakerNameAndClasses that = (SpeakerNameAndClasses) o; + return (speakerName.equals(that.speakerName)) && classes.equals(that.classes); + } + + @Override + public int hashCode() { + return Objects.hash(speakerName, classes); + } + + @Override + public String toString() { + return String.format("{speakerName=%s,classes=%s}", speakerName, classes); + } + } + } } diff --git a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java index 57ef16713b..c15ac26596 100644 --- a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java +++ b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java @@ -40,11 +40,13 @@ import androidx.media3.common.text.HorizontalTextInVerticalContextSpan; import androidx.media3.common.text.RubySpan; import androidx.media3.common.text.TextAnnotation; import androidx.media3.common.text.TextEmphasisSpan; +import androidx.media3.common.text.VoiceSpan; import androidx.media3.common.util.Util; import androidx.media3.test.utils.truth.SpannedSubject.AndSpanFlags; import androidx.media3.test.utils.truth.SpannedSubject.WithSpanFlags; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.truth.ExpectFailure; +import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; @@ -902,6 +904,85 @@ public class SpannedSubjectTest { SpannedSubject::hasNoHorizontalTextInVerticalContextSpanBetween); } + @Test + public void voiceSpan_success() { + SpannableString spannable = + createSpannable( + new VoiceSpan("speaker", Set.of("quiet")), + Spanned.SPAN_INCLUSIVE_EXCLUSIVE); + + assertThat(spannable) + .hasVoiceSpanBetween(SPAN_START, SPAN_END) + .withSpeakerNameAndClasses("speaker", Set.of("quiet")) + .andFlags(Spanned.SPAN_INCLUSIVE_EXCLUSIVE); + } + + @Test + public void voiceSpan_wrongEndIndex() { + checkHasSpanFailsDueToIndexMismatch( + new VoiceSpan("speaker", Set.of("quiet")), + SpannedSubject::hasVoiceSpanBetween); + } + + @Test + public void voiceSpan_wrongSpeakerName() { + SpannableString spannable = createSpannable(new VoiceSpan("speaker", Set.of("quiet"))); + + AssertionError expected = + expectFailure( + whenTesting -> + whenTesting + .that(spannable) + .hasVoiceSpanBetween(SPAN_START, SPAN_END) + .withSpeakerNameAndClasses("different speaker", Set.of("quiet"))); + + assertThat(expected).factValue("value of").contains("voiceSpeakerNameAndClasses"); + assertThat(expected).factValue("expected").contains("speakerName=different speaker"); + assertThat(expected).factValue("but was").contains("speakerName=speaker"); + } + + @Test + public void voiceSpan_wrongClasses() { + SpannableString spannable = createSpannable(new VoiceSpan("speaker", Set.of("quiet"))); + + AssertionError expected = + expectFailure( + whenTesting -> + whenTesting + .that(spannable) + .hasVoiceSpanBetween(SPAN_START, SPAN_END) + .withSpeakerNameAndClasses("speaker", Set.of("loud"))); + + assertThat(expected).factValue("value of").contains("voiceSpeakerNameAndClasses"); + assertThat(expected).factValue("expected").contains("classes=[loud]"); + assertThat(expected).factValue("but was").contains("classes=[quiet]"); + } + + @Test + public void voiceSpan_wrongFlags() { + checkHasSpanFailsDueToFlagMismatch( + new VoiceSpan("speaker", Set.of("quiet")), + (subject, start, end) -> + subject + .hasVoiceSpanBetween(start, end) + .withSpeakerNameAndClasses("speaker", Set.of("quiet"))); + } + + @Test + public void noVoiceSpan_success() { + SpannableString spannable = + createSpannableWithUnrelatedSpanAnd(new VoiceSpan("speaker", Set.of("quiet"))); + + assertThat(spannable).hasNoVoiceSpanBetween(UNRELATED_SPAN_START, UNRELATED_SPAN_END); + } + + @Test + public void noVoiceSpan_failure() { + checkHasNoSpanFails( + new VoiceSpan("speaker", Set.of("quiet")), + SpannedSubject::hasNoVoiceSpanBetween); + } + private interface HasSpanFunction { T call(SpannedSubject s, int start, int end); } From 108a5ca2f55784d5bb05a43ede8ff965752f582f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sikora?= Date: Sat, 24 Aug 2024 01:57:17 +0200 Subject: [PATCH 2/8] Remove language span marker interface --- .../src/main/java/androidx/media3/common/text/VoiceSpan.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java index 66f74b1004..570e770dda 100644 --- a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java +++ b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java @@ -31,7 +31,7 @@ import java.util.Set; * voice spans. */ @UnstableApi -public final class VoiceSpan implements LanguageFeatureSpan { +public final class VoiceSpan { /** The speaker name. */ public final String speakerName; From ce52fc77ecdc71aae141f570a8b96b7f719264d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sikora?= Date: Tue, 27 Aug 2024 19:27:57 +0200 Subject: [PATCH 3/8] Remove VTT voice span classes --- .../media3/common/text/VoiceSpan.java | 12 +----- .../common/text/CustomCueBundlerTest.java | 4 +- .../text/webvtt/WebvttCueParser.java | 6 +-- .../text/webvtt/WebvttCueParserTest.java | 17 ++++----- .../test/utils/truth/SpannedSubject.java | 35 ++++++++---------- .../test/utils/truth/SpannedSubjectTest.java | 37 +++++-------------- 6 files changed, 39 insertions(+), 72 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java index 570e770dda..6e05b58714 100644 --- a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java +++ b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java @@ -36,27 +36,19 @@ public final class VoiceSpan { /** The speaker name. */ public final String speakerName; - /** The classes associated with the text. It can specify things like "first", "loud", etc. */ - public final Set classes; - private static final String FIELD_NAME = Util.intToStringMaxRadix(0); - private static final String FIELD_CLASSES = Util.intToStringMaxRadix(1); - public VoiceSpan(String speakerName, Set classes) { + public VoiceSpan(String speakerName) { this.speakerName = speakerName; - this.classes = classes; } public Bundle toBundle() { Bundle bundle = new Bundle(); bundle.putString(FIELD_NAME, speakerName); - bundle.putStringArray(FIELD_CLASSES, classes.toArray(new String[0])); return bundle; } public static VoiceSpan fromBundle(Bundle bundle) { - return new VoiceSpan( - /* speakerName = */ checkNotNull(bundle.getString(FIELD_NAME)), - /* classes = */ ImmutableSet.copyOf(checkNotNull(bundle.getStringArray(FIELD_CLASSES)))); + return new VoiceSpan(/* speakerName = */ checkNotNull(bundle.getString(FIELD_NAME))); } } diff --git a/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java b/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java index a2b01343f9..e795929278 100644 --- a/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java @@ -42,7 +42,7 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public class CustomCueBundlerTest { - private static final VoiceSpan VOICE_SPAN = new VoiceSpan("name", Set.of("first", "loud")); + private static final VoiceSpan VOICE_SPAN = new VoiceSpan("name"); private static final RubySpan RUBY_SPAN = new RubySpan("ruby text", TextAnnotation.POSITION_AFTER); private static final TextEmphasisSpan TEXT_EMPHASIS_SPAN = @@ -98,7 +98,7 @@ public class CustomCueBundlerTest { .hasVoiceSpanBetween( ALL_SPANS_TO_START_END_INDEX.get(VOICE_SPAN).first, ALL_SPANS_TO_START_END_INDEX.get(VOICE_SPAN).second) - .withSpeakerNameAndClasses(VOICE_SPAN.speakerName, VOICE_SPAN.classes); + .withSpeakerName(VOICE_SPAN.speakerName); } @Test diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java index b2a5527db5..3b9d048b1f 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttCueParser.java @@ -557,7 +557,7 @@ public final class WebvttCueParser { applyDefaultColors(text, startTag.classes, start, end); break; case TAG_VOICE: - applyVoiceSpan(text, startTag.voice, startTag.classes, start, end); + applyVoiceSpan(text, startTag.voice, start, end); break; case TAG_LANG: case "": // Case of the "whole cue" virtual tag. @@ -662,8 +662,8 @@ public final class WebvttCueParser { } private static void applyVoiceSpan( - SpannableStringBuilder text, String voice, Set classes, int start, int end) { - text.setSpan(new VoiceSpan(voice, classes), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + SpannableStringBuilder text, String voice, int start, int end) { + text.setSpan(new VoiceSpan(voice), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); } private static void applyStyleToText( diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java index 0fd5eda714..86e239b454 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java @@ -22,7 +22,6 @@ import android.graphics.Color; import android.text.Spanned; import androidx.test.ext.junit.runners.AndroidJUnit4; import java.util.Collections; -import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; @@ -250,7 +249,7 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerNameAndClasses("", Collections.emptySet()); + .withSpeakerName(""); } @Test @@ -260,7 +259,7 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerNameAndClasses("Esme", Collections.emptySet()); + .withSpeakerName("Esme"); } @Test @@ -270,17 +269,17 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerNameAndClasses("", Set.of("first", "loud")); + .withSpeakerName(""); } @Test - public void parseVoiceSpanWithNameAndClasses() throws Exception { + public void ignoreVoiceSpanClasses() throws Exception { Spanned text = parseCueText("Text with a single voice span"); assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerNameAndClasses("Esme", Set.of("first", "loud")); + .withSpeakerName("Esme"); } @Test @@ -288,12 +287,10 @@ public final class WebvttCueParserTest { Spanned text = parseCueText("Text with multiple voice spans"); assertThat(text.toString()).isEqualTo("Text with multiple voice spans"); - assertThat(text) - .hasVoiceSpanBetween(0, "Text with ".length()) - .withSpeakerNameAndClasses("Esme", Set.of("loud")); + assertThat(text).hasVoiceSpanBetween(0, "Text with ".length()).withSpeakerName("Esme"); assertThat(text) .hasVoiceSpanBetween("Text with ".length(), "Text with multiple voice spans".length()) - .withSpeakerNameAndClasses("Mary", Set.of("quiet")); + .withSpeakerName("Mary"); } private static Spanned parseCueText(String string) { diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java index a05c6fde82..70d3745de0 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java @@ -1315,18 +1315,15 @@ public final class SpannedSubject extends Subject { /** Allows assertions about a span's voice its position. */ public interface VoiceText { /** - * Checks that at least one of the matched spans has the expected {@code name} and {@code - * classes}. + * Checks that at least one of the matched spans has the expected {@code name}. * * @param name The expected name of the voice. - * @param classes The classes used to style the voice. * @return A {@link AndSpanFlags} object for optional additional assertions on the flags. */ - AndSpanFlags withSpeakerNameAndClasses(String name, Set classes); + AndSpanFlags withSpeakerName(String name); } - private static final VoiceText ALREADY_FAILED_WITH_NAME_AND_CLASSES = - (name, classes) -> ALREADY_FAILED_AND_FLAGS; + private static final VoiceText ALREADY_FAILED_WITH_NAME_AND_CLASSES = (name) -> ALREADY_FAILED_AND_FLAGS; private static Factory> voiceSpanSubjects( Spanned actualSpanned) { @@ -1349,29 +1346,27 @@ public final class SpannedSubject extends Subject { } @Override - public AndSpanFlags withSpeakerNameAndClasses(String name, Set classes) { + public AndSpanFlags withSpeakerName(String name) { List matchingSpanFlags = new ArrayList<>(); - List voiceSpeakerNameAndClasses = new ArrayList<>(); + List voiceSpeakerName = new ArrayList<>(); for (VoiceSpan span : checkNotNull(actualSpans)) { - voiceSpeakerNameAndClasses.add(new SpeakerNameAndClasses(span.speakerName, span.classes)); - if (span.speakerName.equals(name) && span.classes.equals(classes)) { + voiceSpeakerNameAndClasses.add(new SpeakerName(span.speakerName)); + if (span.speakerName.equals(name)) { matchingSpanFlags.add(actualSpanned.getSpanFlags(span)); } } - check("voiceSpeakerNameAndClasses") + check("voiceSpeakerName") .that(voiceSpeakerNameAndClasses) - .containsExactly(new SpeakerNameAndClasses(name, classes)); + .containsExactly(new SpeakerName(name)); return check("flags").about(spanFlags()).that(matchingSpanFlags); } - private static final class SpeakerNameAndClasses { + private static final class SpeakerName { private final String speakerName; - private final Set classes; - private SpeakerNameAndClasses(String name, Set classes) { + private SpeakerName(String name) { this.speakerName = name; - this.classes = classes; } @Override @@ -1383,18 +1378,18 @@ public final class SpannedSubject extends Subject { return false; } - SpeakerNameAndClasses that = (SpeakerNameAndClasses) o; - return (speakerName.equals(that.speakerName)) && classes.equals(that.classes); + SpeakerName that = (SpeakerName) o; + return name.equals(that.name); } @Override public int hashCode() { - return Objects.hash(speakerName, classes); + return Objects.hash(speakerName); } @Override public String toString() { - return String.format("{speakerName=%s,classes=%s}", speakerName, classes); + return String.format("{speakerName=%s}", speakerName); } } } diff --git a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java index c15ac26596..5ca83c9a30 100644 --- a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java +++ b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java @@ -908,25 +908,25 @@ public class SpannedSubjectTest { public void voiceSpan_success() { SpannableString spannable = createSpannable( - new VoiceSpan("speaker", Set.of("quiet")), + new VoiceSpan("speaker"), Spanned.SPAN_INCLUSIVE_EXCLUSIVE); assertThat(spannable) .hasVoiceSpanBetween(SPAN_START, SPAN_END) - .withSpeakerNameAndClasses("speaker", Set.of("quiet")) + .withSpeakerName("speaker") .andFlags(Spanned.SPAN_INCLUSIVE_EXCLUSIVE); } @Test public void voiceSpan_wrongEndIndex() { checkHasSpanFailsDueToIndexMismatch( - new VoiceSpan("speaker", Set.of("quiet")), + new VoiceSpan("speaker"), SpannedSubject::hasVoiceSpanBetween); } @Test public void voiceSpan_wrongSpeakerName() { - SpannableString spannable = createSpannable(new VoiceSpan("speaker", Set.of("quiet"))); + SpannableString spannable = createSpannable(new VoiceSpan("speaker")); AssertionError expected = expectFailure( @@ -934,44 +934,27 @@ public class SpannedSubjectTest { whenTesting .that(spannable) .hasVoiceSpanBetween(SPAN_START, SPAN_END) - .withSpeakerNameAndClasses("different speaker", Set.of("quiet"))); + .withSpeakerName("different speaker")); - assertThat(expected).factValue("value of").contains("voiceSpeakerNameAndClasses"); + assertThat(expected).factValue("value of").contains("voiceSpeakerName"); assertThat(expected).factValue("expected").contains("speakerName=different speaker"); assertThat(expected).factValue("but was").contains("speakerName=speaker"); } - @Test - public void voiceSpan_wrongClasses() { - SpannableString spannable = createSpannable(new VoiceSpan("speaker", Set.of("quiet"))); - - AssertionError expected = - expectFailure( - whenTesting -> - whenTesting - .that(spannable) - .hasVoiceSpanBetween(SPAN_START, SPAN_END) - .withSpeakerNameAndClasses("speaker", Set.of("loud"))); - - assertThat(expected).factValue("value of").contains("voiceSpeakerNameAndClasses"); - assertThat(expected).factValue("expected").contains("classes=[loud]"); - assertThat(expected).factValue("but was").contains("classes=[quiet]"); - } - @Test public void voiceSpan_wrongFlags() { checkHasSpanFailsDueToFlagMismatch( - new VoiceSpan("speaker", Set.of("quiet")), + new VoiceSpan("speaker"), (subject, start, end) -> subject .hasVoiceSpanBetween(start, end) - .withSpeakerNameAndClasses("speaker", Set.of("quiet"))); + .withSpeakerName("speaker")); } @Test public void noVoiceSpan_success() { SpannableString spannable = - createSpannableWithUnrelatedSpanAnd(new VoiceSpan("speaker", Set.of("quiet"))); + createSpannableWithUnrelatedSpanAnd(new VoiceSpan("speaker")); assertThat(spannable).hasNoVoiceSpanBetween(UNRELATED_SPAN_START, UNRELATED_SPAN_END); } @@ -979,7 +962,7 @@ public class SpannedSubjectTest { @Test public void noVoiceSpan_failure() { checkHasNoSpanFails( - new VoiceSpan("speaker", Set.of("quiet")), + new VoiceSpan("speaker"), SpannedSubject::hasNoVoiceSpanBetween); } From 24bbe6d92119db5db7b157640d80122b2e3cb9c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sikora?= Date: Tue, 27 Aug 2024 19:36:29 +0200 Subject: [PATCH 4/8] Rename VTT voice span speakerName to name --- .../media3/common/text/VoiceSpan.java | 14 +++---- .../common/text/CustomCueBundlerTest.java | 2 +- .../text/webvtt/WebvttCueParserTest.java | 12 +++--- .../test/utils/truth/SpannedSubject.java | 40 +++++++++---------- .../test/utils/truth/SpannedSubjectTest.java | 15 ++++--- 5 files changed, 38 insertions(+), 45 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java index 6e05b58714..8dc3619436 100644 --- a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java +++ b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java @@ -21,8 +21,6 @@ import static androidx.media3.common.util.Assertions.checkNotNull; import android.os.Bundle; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; -import com.google.common.collect.ImmutableSet; -import java.util.Set; /** * A span representing a speaker. @@ -33,22 +31,22 @@ import java.util.Set; @UnstableApi public final class VoiceSpan { - /** The speaker name. */ - public final String speakerName; + /** The voice name. */ + public final String name; private static final String FIELD_NAME = Util.intToStringMaxRadix(0); - public VoiceSpan(String speakerName) { - this.speakerName = speakerName; + public VoiceSpan(String name) { + this.name = name; } public Bundle toBundle() { Bundle bundle = new Bundle(); - bundle.putString(FIELD_NAME, speakerName); + bundle.putString(FIELD_NAME, name); return bundle; } public static VoiceSpan fromBundle(Bundle bundle) { - return new VoiceSpan(/* speakerName = */ checkNotNull(bundle.getString(FIELD_NAME))); + return new VoiceSpan(/* name = */ checkNotNull(bundle.getString(FIELD_NAME))); } } diff --git a/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java b/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java index e795929278..5f7a1bd08a 100644 --- a/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/text/CustomCueBundlerTest.java @@ -98,7 +98,7 @@ public class CustomCueBundlerTest { .hasVoiceSpanBetween( ALL_SPANS_TO_START_END_INDEX.get(VOICE_SPAN).first, ALL_SPANS_TO_START_END_INDEX.get(VOICE_SPAN).second) - .withSpeakerName(VOICE_SPAN.speakerName); + .withName(VOICE_SPAN.name); } @Test diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java index 86e239b454..896a939b5f 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java @@ -249,7 +249,7 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerName(""); + .withName(""); } @Test @@ -259,7 +259,7 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerName("Esme"); + .withName("Esme"); } @Test @@ -269,7 +269,7 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerName(""); + .withName(""); } @Test @@ -279,7 +279,7 @@ public final class WebvttCueParserTest { assertThat(text.toString()).isEqualTo("Text with a single voice span"); assertThat(text) .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withSpeakerName("Esme"); + .withName("Esme"); } @Test @@ -287,10 +287,10 @@ public final class WebvttCueParserTest { Spanned text = parseCueText("Text with multiple voice spans"); assertThat(text.toString()).isEqualTo("Text with multiple voice spans"); - assertThat(text).hasVoiceSpanBetween(0, "Text with ".length()).withSpeakerName("Esme"); + assertThat(text).hasVoiceSpanBetween(0, "Text with ".length()).withName("Esme"); assertThat(text) .hasVoiceSpanBetween("Text with ".length(), "Text with multiple voice spans".length()) - .withSpeakerName("Mary"); + .withName("Mary"); } private static Spanned parseCueText(String string) { diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java index 70d3745de0..9b2ee2fd82 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java @@ -54,7 +54,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; -import java.util.Set; import org.checkerframework.checker.nullness.qual.RequiresNonNull; /** A Truth {@link Subject} for assertions on {@link Spanned} instances containing text styling. */ @@ -661,8 +660,8 @@ public final class SpannedSubject extends Subject { } /** - * Checks that the subject has no {@link VoiceSpan}s on any of the text - * between {@code start} and {@code end}. + * Checks that the subject has no {@link VoiceSpan}s on any of the text between {@code start} and + * {@code end}. * *

This fails even if the start and end indexes don't exactly match. * @@ -1320,10 +1319,11 @@ public final class SpannedSubject extends Subject { * @param name The expected name of the voice. * @return A {@link AndSpanFlags} object for optional additional assertions on the flags. */ - AndSpanFlags withSpeakerName(String name); + AndSpanFlags withName(String name); } - private static final VoiceText ALREADY_FAILED_WITH_NAME_AND_CLASSES = (name) -> ALREADY_FAILED_AND_FLAGS; + private static final VoiceText ALREADY_FAILED_WITH_NAME_AND_CLASSES = + (name) -> ALREADY_FAILED_AND_FLAGS; private static Factory> voiceSpanSubjects( Spanned actualSpanned) { @@ -1337,36 +1337,32 @@ public final class SpannedSubject extends Subject { private final Spanned actualSpanned; private VoiceSpanSubject( - FailureMetadata metadata, - @Nullable List actualSpans, - Spanned actualSpanned) { + FailureMetadata metadata, @Nullable List actualSpans, Spanned actualSpanned) { super(metadata, actualSpans); this.actualSpans = actualSpans; this.actualSpanned = actualSpanned; } @Override - public AndSpanFlags withSpeakerName(String name) { + public AndSpanFlags withName(String name) { List matchingSpanFlags = new ArrayList<>(); - List voiceSpeakerName = new ArrayList<>(); + List voiceName = new ArrayList<>(); for (VoiceSpan span : checkNotNull(actualSpans)) { - voiceSpeakerNameAndClasses.add(new SpeakerName(span.speakerName)); - if (span.speakerName.equals(name)) { + voiceName.add(new Name(span.name)); + if (span.name.equals(name)) { matchingSpanFlags.add(actualSpanned.getSpanFlags(span)); } } - check("voiceSpeakerName") - .that(voiceSpeakerNameAndClasses) - .containsExactly(new SpeakerName(name)); + check("voiceName").that(voiceName).containsExactly(new Name(name)); return check("flags").about(spanFlags()).that(matchingSpanFlags); } - private static final class SpeakerName { + private static final class Name { - private final String speakerName; + private final String name; - private SpeakerName(String name) { - this.speakerName = name; + private Name(String name) { + this.name = name; } @Override @@ -1378,18 +1374,18 @@ public final class SpannedSubject extends Subject { return false; } - SpeakerName that = (SpeakerName) o; + Name that = (Name) o; return name.equals(that.name); } @Override public int hashCode() { - return Objects.hash(speakerName); + return Objects.hash(name); } @Override public String toString() { - return String.format("{speakerName=%s}", speakerName); + return String.format("{name=%s}", name); } } } diff --git a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java index 5ca83c9a30..c916955054 100644 --- a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java +++ b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java @@ -46,7 +46,6 @@ import androidx.media3.test.utils.truth.SpannedSubject.AndSpanFlags; import androidx.media3.test.utils.truth.SpannedSubject.WithSpanFlags; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.truth.ExpectFailure; -import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; @@ -913,7 +912,7 @@ public class SpannedSubjectTest { assertThat(spannable) .hasVoiceSpanBetween(SPAN_START, SPAN_END) - .withSpeakerName("speaker") + .withName("speaker") .andFlags(Spanned.SPAN_INCLUSIVE_EXCLUSIVE); } @@ -925,7 +924,7 @@ public class SpannedSubjectTest { } @Test - public void voiceSpan_wrongSpeakerName() { + public void voiceSpan_wrongName() { SpannableString spannable = createSpannable(new VoiceSpan("speaker")); AssertionError expected = @@ -934,11 +933,11 @@ public class SpannedSubjectTest { whenTesting .that(spannable) .hasVoiceSpanBetween(SPAN_START, SPAN_END) - .withSpeakerName("different speaker")); + .withName("different speaker")); - assertThat(expected).factValue("value of").contains("voiceSpeakerName"); - assertThat(expected).factValue("expected").contains("speakerName=different speaker"); - assertThat(expected).factValue("but was").contains("speakerName=speaker"); + assertThat(expected).factValue("value of").contains("voiceName"); + assertThat(expected).factValue("expected").contains("name=different speaker"); + assertThat(expected).factValue("but was").contains("name=speaker"); } @Test @@ -948,7 +947,7 @@ public class SpannedSubjectTest { (subject, start, end) -> subject .hasVoiceSpanBetween(start, end) - .withSpeakerName("speaker")); + .withName("speaker")); } @Test From cd47e2a134ab282a1c199e20d4f877a79844c15b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sikora?= Date: Wed, 28 Aug 2024 16:15:44 +0200 Subject: [PATCH 5/8] Remove redundant test --- .../extractor/text/webvtt/WebvttCueParserTest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java index 896a939b5f..7475516ade 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java @@ -262,16 +262,6 @@ public final class WebvttCueParserTest { .withName("Esme"); } - @Test - public void parseVoiceSpanWithClasses() throws Exception { - Spanned text = parseCueText("Text with a single voice span"); - - assertThat(text.toString()).isEqualTo("Text with a single voice span"); - assertThat(text) - .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withName(""); - } - @Test public void ignoreVoiceSpanClasses() throws Exception { Spanned text = parseCueText("Text with a single voice span"); From a4aa975a262a34721cf99d24120c55811d5a3216 Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 29 Aug 2024 10:24:54 +0100 Subject: [PATCH 6/8] Reformat and tweak javadoc --- .../media3/common/text/CustomSpanBundler.java | 3 ++- .../media3/common/text/VoiceSpan.java | 8 ++++---- .../text/webvtt/WebvttCueParserTest.java | 4 +--- .../test/utils/truth/SpannedSubject.java | 3 +-- .../test/utils/truth/SpannedSubjectTest.java | 19 +++++-------------- 5 files changed, 13 insertions(+), 24 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java b/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java index d5f7d15e32..6f24add352 100644 --- a/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java +++ b/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java @@ -97,7 +97,8 @@ import java.util.ArrayList; bundledCustomSpans.add(bundle); } for (VoiceSpan span : text.getSpans(0, text.length(), VoiceSpan.class)) { - Bundle bundle = spanToBundle(text, span, /* spanType= */ VOICE, /* params= */ span.toBundle()); + Bundle bundle = + spanToBundle(text, span, /* spanType= */ VOICE, /* params= */ span.toBundle()); bundledCustomSpans.add(bundle); } return bundledCustomSpans; diff --git a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java index 8dc3619436..fc73efb14e 100644 --- a/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java +++ b/libraries/common/src/main/java/androidx/media3/common/text/VoiceSpan.java @@ -23,10 +23,10 @@ import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; /** - * A span representing a speaker. + * A span representing the speaker of the spanned text. * - *

More information on - * voice spans. + *

For example a WebVTT voice + * span. */ @UnstableApi public final class VoiceSpan { @@ -47,6 +47,6 @@ public final class VoiceSpan { } public static VoiceSpan fromBundle(Bundle bundle) { - return new VoiceSpan(/* name = */ checkNotNull(bundle.getString(FIELD_NAME))); + return new VoiceSpan(checkNotNull(bundle.getString(FIELD_NAME))); } } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java index 7475516ade..14cf6e55dd 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttCueParserTest.java @@ -247,9 +247,7 @@ public final class WebvttCueParserTest { Spanned text = parseCueText("Text with a single voice span"); assertThat(text.toString()).isEqualTo("Text with a single voice span"); - assertThat(text) - .hasVoiceSpanBetween(0, "Text with a single voice span".length()) - .withName(""); + assertThat(text).hasVoiceSpanBetween(0, "Text with a single voice span".length()).withName(""); } @Test diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java index 9b2ee2fd82..3c3a5c1c0c 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/truth/SpannedSubject.java @@ -53,7 +53,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; import org.checkerframework.checker.nullness.qual.RequiresNonNull; /** A Truth {@link Subject} for assertions on {@link Spanned} instances containing text styling. */ @@ -1380,7 +1379,7 @@ public final class SpannedSubject extends Subject { @Override public int hashCode() { - return Objects.hash(name); + return name.hashCode(); } @Override diff --git a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java index c916955054..dc7b2fca39 100644 --- a/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java +++ b/libraries/test_utils/src/test/java/androidx/media3/test/utils/truth/SpannedSubjectTest.java @@ -906,9 +906,7 @@ public class SpannedSubjectTest { @Test public void voiceSpan_success() { SpannableString spannable = - createSpannable( - new VoiceSpan("speaker"), - Spanned.SPAN_INCLUSIVE_EXCLUSIVE); + createSpannable(new VoiceSpan("speaker"), Spanned.SPAN_INCLUSIVE_EXCLUSIVE); assertThat(spannable) .hasVoiceSpanBetween(SPAN_START, SPAN_END) @@ -919,8 +917,7 @@ public class SpannedSubjectTest { @Test public void voiceSpan_wrongEndIndex() { checkHasSpanFailsDueToIndexMismatch( - new VoiceSpan("speaker"), - SpannedSubject::hasVoiceSpanBetween); + new VoiceSpan("speaker"), SpannedSubject::hasVoiceSpanBetween); } @Test @@ -944,25 +941,19 @@ public class SpannedSubjectTest { public void voiceSpan_wrongFlags() { checkHasSpanFailsDueToFlagMismatch( new VoiceSpan("speaker"), - (subject, start, end) -> - subject - .hasVoiceSpanBetween(start, end) - .withName("speaker")); + (subject, start, end) -> subject.hasVoiceSpanBetween(start, end).withName("speaker")); } @Test public void noVoiceSpan_success() { - SpannableString spannable = - createSpannableWithUnrelatedSpanAnd(new VoiceSpan("speaker")); + SpannableString spannable = createSpannableWithUnrelatedSpanAnd(new VoiceSpan("speaker")); assertThat(spannable).hasNoVoiceSpanBetween(UNRELATED_SPAN_START, UNRELATED_SPAN_END); } @Test public void noVoiceSpan_failure() { - checkHasNoSpanFails( - new VoiceSpan("speaker"), - SpannedSubject::hasNoVoiceSpanBetween); + checkHasNoSpanFails(new VoiceSpan("speaker"), SpannedSubject::hasNoVoiceSpanBetween); } private interface HasSpanFunction { From 4d9ad5a6e03379598824de57c28dc432e21f7d2d Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 29 Aug 2024 10:27:10 +0100 Subject: [PATCH 7/8] Add release note --- RELEASENOTES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 872ab9349e..711b3c4be0 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -67,6 +67,9 @@ to remove a previously set `Surface` if the codec supports this (`MediaCodecInfo.detachedSurfaceSupported`). * Text: + * Add a custom `VoiceSpan` and populate it for + [WebVTT voice spans](https://www.w3.org/TR/webvtt1/#webvtt-cue-voice-span) + ([#1632](https://github.com/androidx/media/issues/1632)). * Metadata: * Image: * Add `ExternallyLoadedImageDecoder` for simplified integration with From c78ac6e784003fc45ee2356388fabd4883117ab2 Mon Sep 17 00:00:00 2001 From: Ian Baker Date: Thu, 29 Aug 2024 16:44:50 +0100 Subject: [PATCH 8/8] Remove intdef value list from javadoc --- .../media3/common/text/CustomSpanBundler.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java b/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java index 6f24add352..73a64fc9fa 100644 --- a/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java +++ b/libraries/common/src/main/java/androidx/media3/common/text/CustomSpanBundler.java @@ -45,16 +45,7 @@ import java.util.ArrayList; */ /* package */ final class CustomSpanBundler { - /** - * Media3 custom span implementations. One of the following: - * - *

- */ + /** Media3 custom span implementations. */ @Documented @Retention(RetentionPolicy.SOURCE) @Target({TYPE_USE})