Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replacing label with a list of Label elements in Format. #1054

Merged
merged 8 commits into from
Mar 27, 2024
Merged

Replacing label with a list of Label elements in Format. #1054

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
df76322
Replacing label with a list of Label elements in Format.
jekopena Jan 30, 2024
63fb68e
Reverting breaking changes by keeping label and adding labels.
jekopena Jan 31, 2024
e9e47f4
Removing id from Label and ensuring label/labels consistency in Forma…
jekopena Mar 12, 2024
5910200
Adding IllegalStateException to Format build and unit tests.
jekopena Mar 13, 2024
0a04e79
Addressing PR feedback.
jekopena Mar 14, 2024
54c23e2
Reverting changes in gradle-wrapper.properties
jekopena Mar 14, 2024
8673e6d
Removing label from toLogString.
jekopena Mar 18, 2024
73c0ebb
Additional clean-up and formatting
tonihei Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Removing id from Label and ensuring label/labels consistency in Forma… 
…t.Builder.
  • Loading branch information
@jekopena @tonihei
jekopena authored and tonihei committed Mar 25, 2024
commit e9e47f4fe694613f82c56b1c3fae48367867c271
32 changes: 23 additions & 9 deletions libraries/common/src/main/java/androidx/media3/common/Format.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static java.lang.annotation.ElementType.TYPE_USE;

import android.os.Bundle;
import android.text.TextUtils;
import androidx.annotation.IntDef;
import androidx.annotation.Nullable;
import androidx.media3.common.util.BundleCollectionUtil;
Expand Down Expand Up @@ -137,7 +138,6 @@ public final class Format implements Bundleable {
public static final class Builder {

@Nullable private String id;

@Nullable private String label;
@Nullable private List<Label> labels;
@Nullable private String language;
Expand Down Expand Up @@ -756,12 +756,8 @@ public Format build() {
/** An identifier for the format, or null if unknown or not applicable. */
@Nullable public final String id;

/**
* The human readable label, or null if unknown or not applicable.
*
* @deprecated Use {@link #labels} instead.
*/
@Deprecated @Nullable public final String label;
/** The human readable label, or null if unknown or not applicable. */
@Nullable public final String label;

/** The human readable list of labels, or null if unknown or not applicable. */
@UnstableApi public final List<Label> labels;
Expand Down Expand Up @@ -954,9 +950,13 @@ public Format build() {

private Format(Builder builder) {
id = builder.id;
label = builder.label;
labels = builder.labels == null ? Collections.emptyList() : builder.labels;
language = Util.normalizeLanguageCode(builder.language);
@Nullable String tmpLabel = builder.label;
labels = builder.labels == null ? new ArrayList<>() : builder.labels;
if (labels.isEmpty() && tmpLabel != null && !tmpLabel.isEmpty()) {
labels.add(new Label(language, tmpLabel));
}
label = makeLabelIfNeeded(tmpLabel, labels);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the logic flow easier to read, could it add an else if (!labels.isEmpty() && tmpLabel == null) to call this method and rename the method to getDefaultLabel?

selectionFlags = builder.selectionFlags;
roleFlags = builder.roleFlags;
averageBitrate = builder.averageBitrate;
Expand Down Expand Up @@ -1004,6 +1004,20 @@ private Format(Builder builder) {
}
}

private @Nullable String makeLabelIfNeeded(@Nullable String label, List<Label> labels) {
if (label == null || label.isEmpty()) {
for (Label l : labels) {
if (TextUtils.equals(l.lang, language)) {
return l.value;
}
}
if (!labels.isEmpty()) {
return labels.get(0).value;
}
}
return label;
}

/** Returns a {@link Format.Builder} initialized with the values of this instance. */
@UnstableApi
public Builder buildUpon() {
Expand Down
13 changes: 2 additions & 11 deletions libraries/common/src/main/java/androidx/media3/common/Label.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,22 @@
/** A Label, as defined by ISO 23009-1, 4th edition, 5.3.7.2. */
@UnstableApi
public class Label implements Parcelable {
/** The Label identifier, if one exists. */
@Nullable public final String id;

/** Declares the language code(s) for this Label. */
@Nullable public final String lang;

/** The value for this Label. */
public final String value;

/**
* @param id The id.
* @param lang The lang code.
* @param value The value.
*/
public Label(@Nullable String id, @Nullable String lang, String value) {
this.id = id;
public Label(@Nullable String lang, String value) {
this.lang = lang;
this.value = value;
}

/* package */ Label(Parcel in) {
id = in.readString();
lang = in.readString();
value = in.readString();
}
Expand All @@ -41,15 +35,13 @@ public boolean equals(@Nullable Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Label label = (Label) o;
return Util.areEqual(id, label.id)
&& Util.areEqual(lang, label.lang)
return Util.areEqual(lang, label.lang)
&& Util.areEqual(value, label.value);
}

@Override
public int hashCode() {
int result = value.hashCode();
result = 31 * result + (id != null ? id.hashCode() : 0);
result = 31 * result + (lang != null ? lang.hashCode() : 0);
return result;
}
Expand All @@ -61,7 +53,6 @@ public int describeContents() {

@Override
public void writeToParcel(@NonNull Parcel dest, int flags) {
dest.writeString(id);
dest.writeString(lang);
dest.writeString(value);
}
Expand Down
2 changes: 1 addition & 1 deletion libraries/common/src/test/java/androidx/media3/common/FormatTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ private static Format createTestFormat() {
.build();

List<Label> labels = new ArrayList<>();
labels.add(new Label("id", "en", "label"));
labels.add(new Label("en", "label"));
return new Format.Builder()
.setId("id")
.setLabel("label")
Expand Down
11 changes: 1 addition & 10 deletions ...s/exoplayer/src/main/java/androidx/media3/exoplayer/source/DefaultMediaSourceFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import androidx.media3.common.AdViewProvider;
import androidx.media3.common.C;
import androidx.media3.common.Format;
import androidx.media3.common.Label;
import androidx.media3.common.MediaItem;
import androidx.media3.common.MimeTypes;
import androidx.media3.common.util.Assertions;
Expand Down Expand Up @@ -59,7 +58,6 @@
import com.google.common.primitives.Ints;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -515,20 +513,13 @@ public MediaSource createMediaSource(MediaItem mediaItem) {
mediaSources[0] = mediaSource;
for (int i = 0; i < subtitleConfigurations.size(); i++) {
if (parseSubtitlesDuringExtraction) {
List<Label> labels = new ArrayList<>();
String label = subtitleConfigurations.get(i).label;
if (label != null) {
labels.add(new Label(null, null, label));
}

Format format =
new Format.Builder()
.setSampleMimeType(subtitleConfigurations.get(i).mimeType)
.setLanguage(subtitleConfigurations.get(i).language)
.setSelectionFlags(subtitleConfigurations.get(i).selectionFlags)
.setRoleFlags(subtitleConfigurations.get(i).roleFlags)
.setLabel(label)
.setLabels(labels)
.setLabel(subtitleConfigurations.get(i).label)
.setId(subtitleConfigurations.get(i).id)
.build();
ExtractorsFactory extractorsFactory =
Expand Down
11 changes: 1 addition & 10 deletions ...ies/exoplayer/src/main/java/androidx/media3/exoplayer/source/SingleSampleMediaSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import android.net.Uri;
import androidx.annotation.Nullable;
import androidx.media3.common.Format;
import androidx.media3.common.Label;
import androidx.media3.common.MediaItem;
import androidx.media3.common.MimeTypes;
import androidx.media3.common.Timeline;
Expand All @@ -34,8 +33,6 @@
import androidx.media3.exoplayer.upstream.LoadErrorHandlingPolicy;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.ArrayList;
import java.util.List;

/**
* Loads data at a given {@link Uri} as a single sample belonging to a single {@link MediaPeriod}.
Expand Down Expand Up @@ -173,19 +170,13 @@ private SingleSampleMediaSource(
.setSubtitleConfigurations(ImmutableList.of(subtitleConfiguration))
.setTag(tag)
.build();
List<Label> labels = new ArrayList<>();
String label = subtitleConfiguration.label;
if (label != null) {
labels.add(new Label(null, null, label));
}
this.format =
new Format.Builder()
.setSampleMimeType(firstNonNull(subtitleConfiguration.mimeType, MimeTypes.TEXT_UNKNOWN))
.setLanguage(subtitleConfiguration.language)
.setSelectionFlags(subtitleConfiguration.selectionFlags)
.setRoleFlags(subtitleConfiguration.roleFlags)
.setLabel(label)
.setLabels(labels)
.setLabel(subtitleConfiguration.label)
.setId(subtitleConfiguration.id != null ? subtitleConfiguration.id : trackId)
.build();
this.dataSpec =
Expand Down
9 changes: 3 additions & 6 deletions ...player_dash/src/main/java/androidx/media3/exoplayer/dash/manifest/DashManifestParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,9 +506,7 @@ protected AdaptationSet parseAdaptationSet(
} else if (XmlPullParserUtil.isStartTag(xpp, "InbandEventStream")) {
inbandEventStreams.add(parseDescriptor(xpp, "InbandEventStream"));
} else if (XmlPullParserUtil.isStartTag(xpp, "Label")) {
Label parsedLabel = parseLabel(xpp);
label = parsedLabel.value;
labels.add(parsedLabel);
labels.add(parseLabel(xpp));
} else if (XmlPullParserUtil.isStartTag(xpp)) {
parseAdaptationSetChild(xpp);
}
Expand Down Expand Up @@ -861,7 +859,7 @@ protected Format buildFormat(
protected Representation buildRepresentation(
RepresentationInfo representationInfo,
@Nullable String label,
@Nullable List<Label> labels,
List<Label> labels,
@Nullable String extraDrmSchemeType,
ArrayList<SchemeData> extraDrmSchemeDatas,
ArrayList<Descriptor> extraInbandEventStreams) {
Expand Down Expand Up @@ -1413,10 +1411,9 @@ protected ProgramInformation parseProgramInformation(XmlPullParser xpp)
* @return The parsed label.
*/
protected Label parseLabel(XmlPullParser xpp) throws XmlPullParserException, IOException {
String id = xpp.getAttributeValue(null, "id");
String lang = xpp.getAttributeValue(null, "lang");
String value = parseText(xpp, "Label");
return new Label(id, lang, value);
return new Label(lang, value);
}

/**
Expand Down
15 changes: 1 addition & 14 deletions ...er_dash/src/test/java/androidx/media3/exoplayer/dash/manifest/DashManifestParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,28 +437,15 @@ public void parseSegmentTimeline_timeOffsetsAndUndefinedRepeatCount() throws Exc
public void parseLabel() throws Exception {
DashManifestParser parser = new DashManifestParser();
XmlPullParser xpp = XmlPullParserFactory.newInstance().newPullParser();
xpp.setInput(new StringReader("<Label id=\"1\" lang=\"en\">test label</Label>" + NEXT_TAG));
xpp.setInput(new StringReader("<Label lang=\"en\">test label</Label>" + NEXT_TAG));
xpp.next();

Label label = parser.parseLabel(xpp);
assertThat(label.id).isEqualTo("1");
assertThat(label.lang).isEqualTo("en");
assertThat(label.value).isEqualTo("test label");
assertNextTag(xpp);
}

@Test
public void parseLabel_noId() throws Exception {
DashManifestParser parser = new DashManifestParser();
XmlPullParser xpp = XmlPullParserFactory.newInstance().newPullParser();
xpp.setInput(new StringReader("<Label lang=\"en\">test label</Label>" + NEXT_TAG));
xpp.next();

Label label = parser.parseLabel(xpp);
assertThat(label.id).isEqualTo(null);
assertNextTag(xpp);
}

@Test
public void parseLabel_noLang() throws Exception {
DashManifestParser parser = new DashManifestParser();
Expand Down
4 changes: 0 additions & 4 deletions ...exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsPlaylistParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import androidx.media3.common.DrmInitData;
import androidx.media3.common.DrmInitData.SchemeData;
import androidx.media3.common.Format;
import androidx.media3.common.Label;
import androidx.media3.common.Metadata;
import androidx.media3.common.MimeTypes;
import androidx.media3.common.ParserException;
Expand Down Expand Up @@ -472,13 +471,10 @@ private static HlsMultivariantPlaylist parseMultivariantPlaylist(
line = mediaTags.get(i);
String groupId = parseStringAttr(line, REGEX_GROUP_ID, variableDefinitions);
String name = parseStringAttr(line, REGEX_NAME, variableDefinitions);
List<Label> labels = new ArrayList<>();
labels.add(new Label(null, null, name));
Format.Builder formatBuilder =
new Format.Builder()
.setId(groupId + ":" + name)
.setLabel(name)
.setLabels(labels)
.setContainerMimeType(MimeTypes.APPLICATION_M3U8)
.setSelectionFlags(parseSelectionFlags(line))
.setRoleFlags(parseRoleFlags(line, variableDefinitions))
Expand Down
9 changes: 1 addition & 8 deletions ...ng/src/main/java/androidx/media3/exoplayer/smoothstreaming/manifest/SsManifestParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import androidx.media3.common.DrmInitData;
import androidx.media3.common.DrmInitData.SchemeData;
import androidx.media3.common.Format;
import androidx.media3.common.Label;
import androidx.media3.common.MimeTypes;
import androidx.media3.common.ParserException;
import androidx.media3.common.util.Assertions;
Expand Down Expand Up @@ -741,16 +740,10 @@ public void parseStartTag(XmlPullParser parser) throws ParserException {
formatBuilder.setContainerMimeType(MimeTypes.APPLICATION_MP4);
}

List<Label> labels = new ArrayList<>();
String label = (String) getNormalizedAttribute(KEY_NAME);
if (label != null) {
labels.add(new Label(null, null, label));
}
format =
formatBuilder
.setId(parser.getAttributeValue(null, KEY_INDEX))
.setLabel(label)
.setLabels(labels)
.setLabel((String) getNormalizedAttribute(KEY_NAME))
.setSampleMimeType(sampleMimeType)
.setAverageBitrate(parseRequiredInt(parser, KEY_BITRATE))
.setLanguage((String) getNormalizedAttribute(KEY_LANGUAGE))
Expand Down
9 changes: 1 addition & 8 deletions libraries/extractor/src/main/java/androidx/media3/extractor/avi/AviExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import androidx.annotation.Nullable;
import androidx.media3.common.C;
import androidx.media3.common.Format;
import androidx.media3.common.Label;
import androidx.media3.common.MimeTypes;
import androidx.media3.common.ParserException;
import androidx.media3.common.util.Assertions;
Expand All @@ -43,7 +42,6 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.ArrayList;
import java.util.List;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;

/**
Expand Down Expand Up @@ -516,12 +514,7 @@ private ChunkReader processStreamList(ListChunk streamList, int streamId) {
}
StreamNameChunk streamName = streamList.getChild(StreamNameChunk.class);
if (streamName != null) {
String label = streamName.name;
builder.setLabel(label);

List<Label> labels = new ArrayList<>();
labels.add(new Label(null, null, label));
builder.setLabels(labels);
builder.setLabel(streamName.name);
}
int trackType = MimeTypes.getTrackType(streamFormat.sampleMimeType);
if (trackType == C.TRACK_TYPE_AUDIO || trackType == C.TRACK_TYPE_VIDEO) {
Expand Down
5 changes: 0 additions & 5 deletions libraries/extractor/src/main/java/androidx/media3/extractor/mkv/MatroskaExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import androidx.media3.common.DrmInitData;
import androidx.media3.common.DrmInitData.SchemeData;
import androidx.media3.common.Format;
import androidx.media3.common.Label;
import androidx.media3.common.MimeTypes;
import androidx.media3.common.ParserException;
import androidx.media3.common.util.Log;
Expand Down Expand Up @@ -2425,10 +2424,6 @@ public void initializeOutput(ExtractorOutput output, int trackId) throws ParserE

if (name != null && !TRACK_NAME_TO_ROTATION_DEGREES.containsKey(name)) {
formatBuilder.setLabel(name);

List<Label> labels = new ArrayList<>();
labels.add(new Label(null, null, name));
formatBuilder.setLabels(labels);
}

Format format =
Expand Down
1 change: 1 addition & 0 deletions ...s/test_data/src/test/assets/extractordumps/mkv/sample_with_null_terminated_srt.mkv.0.dump
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ track 3:
language = en
label = Subs Label
labels:
lang = en
value = Subs Label
sample 0:
time = 0
Expand Down
1 change: 1 addition & 0 deletions ...s/test_data/src/test/assets/extractordumps/mkv/sample_with_null_terminated_srt.mkv.1.dump
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ track 3:
language = en
label = Subs Label
labels:
lang = en
value = Subs Label
sample 0:
time = 0
Expand Down
1 change: 1 addition & 0 deletions ...s/test_data/src/test/assets/extractordumps/mkv/sample_with_null_terminated_srt.mkv.2.dump
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ track 3:
language = en
label = Subs Label
labels:
lang = en
value = Subs Label
sample 0:
time = 0
Expand Down
1 change: 1 addition & 0 deletions ...s/test_data/src/test/assets/extractordumps/mkv/sample_with_null_terminated_srt.mkv.3.dump
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ track 3:
language = en
label = Subs Label
labels:
lang = en
value = Subs Label
sample 0:
time = 0
Expand Down
Loading