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.

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

Fix cue render order for ass/ssa. #2137

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions demos/main/src/main/assets/media.exolist.json
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,13 @@
"subtitle_mime_type": "text/x-ssa",
"subtitle_language": "en"
},
{
"name": "SubStation Layer Render",
"uri": "https://storage.googleapis.com/exoplayer-test-media-1/gen-3/screens/dash-vod-single-segment/video-avc-baseline-480.mp4",
"subtitle_uri": "https://storage.googleapis.com/exoplayer-test-media-1/ssa/test-subs-layer.ass",
"subtitle_mime_type": "text/x-ssa",
"subtitle_language": "en"
},
{
"name": "MPEG-4 Timed Text",
"uri": "https://storage.googleapis.com/exoplayer-test-media-1/mp4/dizzy-with-tx3g.mp4"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,11 @@ public final class Cue {
*/
public final float shearDegrees;

/**
* The z index for cue, the larger index will render above the smaller index.
*/
public final int zIndex;

private Cue(
@Nullable CharSequence text,
@Nullable Alignment textAlignment,
Expand All @@ -326,7 +331,8 @@ private Cue(
boolean windowColorSet,
int windowColor,
@VerticalType int verticalType,
float shearDegrees) {
float shearDegrees,
int zIndex) {
// Exactly one of text or bitmap should be set.
if (text == null) {
Assertions.checkNotNull(bitmap);
Expand Down Expand Up @@ -356,6 +362,7 @@ private Cue(
this.textSize = textSize;
this.verticalType = verticalType;
this.shearDegrees = shearDegrees;
this.zIndex = zIndex;
}

/** Returns a new {@link Cue.Builder} initialized with the same values as this Cue. */
Expand Down Expand Up @@ -391,7 +398,8 @@ public boolean equals(@Nullable Object obj) {
&& textSizeType == that.textSizeType
&& textSize == that.textSize
&& verticalType == that.verticalType
&& shearDegrees == that.shearDegrees;
&& shearDegrees == that.shearDegrees
&& zIndex == that.zIndex;
}

@Override
Expand All @@ -413,7 +421,8 @@ public int hashCode() {
textSizeType,
textSize,
verticalType,
shearDegrees);
shearDegrees,
zIndex);
}

/** A builder for {@link Cue} objects. */
Expand All @@ -436,6 +445,7 @@ public static final class Builder {
@ColorInt private int windowColor;
private @VerticalType int verticalType;
private float shearDegrees;
private int zIndex;

public Builder() {
text = null;
Expand Down Expand Up @@ -474,6 +484,7 @@ private Builder(Cue cue) {
windowColor = cue.windowColor;
verticalType = cue.verticalType;
shearDegrees = cue.shearDegrees;
zIndex = cue.zIndex;
}

/**
Expand Down Expand Up @@ -806,6 +817,21 @@ public Builder setShearDegrees(float shearDegrees) {
return verticalType;
}

/** Sets the zIndex for this Cue. */
@CanIgnoreReturnValue
public Builder setZIndex(int zIndex) {
this.zIndex = zIndex;
return this;
}

/**
* Gets the zIndex for this Cue.
*/
@Pure
public int getZIndex() {
return zIndex;
}

/** Build the cue. */
public Cue build() {
return new Cue(
Expand All @@ -825,7 +851,8 @@ public Cue build() {
windowColorSet,
windowColor,
verticalType,
shearDegrees);
shearDegrees,
zIndex);
}
}

Expand All @@ -848,6 +875,7 @@ public Cue build() {
private static final String FIELD_WINDOW_COLOR_SET = Util.intToStringMaxRadix(14);
private static final String FIELD_VERTICAL_TYPE = Util.intToStringMaxRadix(15);
private static final String FIELD_SHEAR_DEGREES = Util.intToStringMaxRadix(16);
private static final String FIELD_Z_INDEX = Util.intToStringMaxRadix(19);

/**
* Returns a {@link Bundle} that can be serialized to bytes.
Expand Down Expand Up @@ -923,6 +951,7 @@ private Bundle toBundleWithoutBitmap() {
bundle.putInt(FIELD_WINDOW_COLOR, windowColor);
bundle.putInt(FIELD_VERTICAL_TYPE, verticalType);
bundle.putFloat(FIELD_SHEAR_DEGREES, shearDegrees);
bundle.putInt(FIELD_Z_INDEX, zIndex);
return bundle;
}

Expand Down Expand Up @@ -995,6 +1024,9 @@ public static Cue fromBundle(Bundle bundle) {
if (bundle.containsKey(FIELD_SHEAR_DEGREES)) {
builder.setShearDegrees(bundle.getFloat(FIELD_SHEAR_DEGREES));
}
if (bundle.containsKey(FIELD_Z_INDEX)) {
builder.setZIndex(bundle.getInt(FIELD_Z_INDEX));
}
return builder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,19 @@
import androidx.media3.common.util.UnstableApi;
import androidx.media3.common.util.Util;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Ordering;
import java.util.ArrayList;
import java.util.List;

/** Class to represent the state of active {@link Cue Cues} at a particular time. */
public final class CueGroup {

/**
* An {@link Ordering} which sorts cues in ascending zIndex priority
*/
private static final Ordering<Cue> CUES_PRIORITY_COMPARATOR =
Ordering.<Integer>natural().onResultOf(c -> c.zIndex);

/** An empty group with no {@link Cue Cues} and presentation time of zero. */
@UnstableApi
public static final CueGroup EMPTY_TIME_ZERO =
Expand All @@ -54,7 +61,7 @@ public final class CueGroup {
/** Creates a CueGroup. */
@UnstableApi
public CueGroup(List<Cue> cues, long presentationTimeUs) {
this.cues = ImmutableList.copyOf(cues);
this.cues = ImmutableList.sortedCopyOf(CUES_PRIORITY_COMPARATOR, cues);
this.presentationTimeUs = presentationTimeUs;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
/** A list of {@link Cue} instances with a start time and duration. */
@UnstableApi
public class CuesWithTiming {

/** The cues to show on screen. */
public final ImmutableList<Cue> cues;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,16 @@
*/
/* package */ final class SsaDialogueFormat {

public final int layerIndex;
public final int startTimeIndex;
public final int endTimeIndex;
public final int styleIndex;
public final int textIndex;
public final int length;

private SsaDialogueFormat(
int startTimeIndex, int endTimeIndex, int styleIndex, int textIndex, int length) {
int layerIndex, int startTimeIndex, int endTimeIndex, int styleIndex, int textIndex, int length) {
this.layerIndex = layerIndex;
this.startTimeIndex = startTimeIndex;
this.endTimeIndex = endTimeIndex;
this.styleIndex = styleIndex;
Expand All @@ -54,6 +56,7 @@ private SsaDialogueFormat(
*/
@Nullable
public static SsaDialogueFormat fromFormatLine(String formatLine) {
int layerIndex = C.INDEX_UNSET;
int startTimeIndex = C.INDEX_UNSET;
int endTimeIndex = C.INDEX_UNSET;
int styleIndex = C.INDEX_UNSET;
Expand All @@ -62,6 +65,9 @@ public static SsaDialogueFormat fromFormatLine(String formatLine) {
String[] keys = TextUtils.split(formatLine.substring(FORMAT_LINE_PREFIX.length()), ",");
for (int i = 0; i < keys.length; i++) {
switch (Ascii.toLowerCase(keys[i].trim())) {
case "layer":
layerIndex = i;
break;
case "start":
startTimeIndex = i;
break;
Expand All @@ -79,7 +85,7 @@ public static SsaDialogueFormat fromFormatLine(String formatLine) {
return (startTimeIndex != C.INDEX_UNSET
&& endTimeIndex != C.INDEX_UNSET
&& textIndex != C.INDEX_UNSET)
? new SsaDialogueFormat(startTimeIndex, endTimeIndex, styleIndex, textIndex, keys.length)
? new SsaDialogueFormat(layerIndex, startTimeIndex, endTimeIndex, styleIndex, textIndex, keys.length)
: null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,14 @@ private void parseDialogueLine(
return;
}

int layer = 0;
if (format.layerIndex != C.INDEX_UNSET) {
layer = parseInt(lineValues[format.layerIndex]);
if (layer == C.LENGTH_UNSET) {
layer = 0;
}
}

long startTimeUs = parseTimecodeUs(lineValues[format.startTimeIndex]);
if (startTimeUs == C.TIME_UNSET) {
Log.w(TAG, "Skipping invalid timing: " + dialogueLine);
Expand All @@ -351,7 +359,7 @@ private void parseDialogueLine(
.replace("\\N", "\n")
.replace("\\n", "\n")
.replace("\\h", "\u00A0");
Cue cue = createCue(text, style, styleOverrides, screenWidth, screenHeight);
Cue cue = createCue(text, layer, style, styleOverrides, screenWidth, screenHeight);

int startTimeIndex = addCuePlacerholderByTime(startTimeUs, cueTimesUs, cues);
int endTimeIndex = addCuePlacerholderByTime(endTimeUs, cueTimesUs, cues);
Expand All @@ -361,6 +369,19 @@ private void parseDialogueLine(
}
}

/**
* Parse int in SSA.
* @param intString The string to parse.
* @return The parsed int.
*/
private static int parseInt(String intString) {
try {
return Integer.parseInt(intString.trim());
} catch (Exception exception) {
return C.LENGTH_UNSET;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should default to 0 here for layer, same as Cue

That probably means you need to either return null here, or take a default value as a parameter, or don't define this general purpose function and just in-line it for the layer case.

I'd suggest in-lining

Copy link
Author

Choose a reason for hiding this comment

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

parseInt is a general method, and I will set layer to 0 when the result is unset in line 333.

}
}

/**
* Parses an SSA timecode string.
*
Expand All @@ -382,12 +403,15 @@ private static long parseTimecodeUs(String timeString) {

private static Cue createCue(
String text,
int layer,
@Nullable SsaStyle style,
SsaStyle.Overrides styleOverrides,
float screenWidth,
float screenHeight) {
SpannableString spannableText = new SpannableString(text);
Cue.Builder cue = new Cue.Builder().setText(spannableText);
Cue.Builder cue = new Cue.Builder()
.setText(spannableText)
.setZIndex(layer);

if (style != null) {
if (style.primaryColor != null) {
Expand Down