Skip to content

Commit

Permalink
Split Cue.toBundle into serializable and binder-based variants
Browse files Browse the repository at this point in the history 
The serializable form is used when we need to serialize the result into
bytes in the sample queue. The binder-based (ultimately
filedescriptor-based) form is used for
session/controller IPC, in order to avoid sending the bitmap bytes over
the IPC.

Issue: #836

#minor-release

PiperOrigin-RevId: 588420836
  • Loading branch information
@icbaker @copybara-github
icbaker authored and copybara-github committed Dec 6, 2023
1 parent a98052b commit bd19953
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 26 deletions.
4 changes: 4 additions & 0 deletions RELEASENOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
marked as unsupported
([#693](https://github.com/androidx/media/issues/693)).
* Text:
* Fix serialization of bitmap cues to resolve `Tried to marshall a Parcel
that contained Binder objects` error when using
`DefaultExtractorsFactory.setTextTrackTranscodingEnabled`
([#836](https://github.com/androidx/media/issues/836)).
* Metadata:
* DRM:
* Extend workaround for spurious ClearKey `https://default.url` license
Expand Down
65 changes: 60 additions & 5 deletions libraries/common/src/main/java/androidx/media3/common/text/Cue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@
package androidx.media3.common.text;

import static androidx.media3.common.text.CustomSpanBundler.bundleCustomSpans;
import static androidx.media3.common.util.Assertions.checkState;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.ElementType.TYPE_USE;

import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.graphics.Color;
import android.os.Binder;
import android.os.Bundle;
import android.text.Layout;
import android.text.Layout.Alignment;
Expand All @@ -40,6 +43,7 @@
import androidx.media3.common.util.Util;
import com.google.common.base.Objects;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.ByteArrayOutputStream;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
Expand Down Expand Up @@ -832,7 +836,8 @@ public Cue build() {
private static final String FIELD_CUSTOM_SPANS = Util.intToStringMaxRadix(17);
private static final String FIELD_TEXT_ALIGNMENT = Util.intToStringMaxRadix(1);
private static final String FIELD_MULTI_ROW_ALIGNMENT = Util.intToStringMaxRadix(2);
private static final String FIELD_BITMAP = Util.intToStringMaxRadix(3);
private static final String FIELD_BITMAP_PARCELABLE = Util.intToStringMaxRadix(3);
private static final String FIELD_BITMAP_BYTES = Util.intToStringMaxRadix(18);
private static final String FIELD_LINE = Util.intToStringMaxRadix(4);
private static final String FIELD_LINE_TYPE = Util.intToStringMaxRadix(5);
private static final String FIELD_LINE_ANCHOR = Util.intToStringMaxRadix(6);
Expand All @@ -847,9 +852,56 @@ public Cue build() {
private static final String FIELD_VERTICAL_TYPE = Util.intToStringMaxRadix(15);
private static final String FIELD_SHEAR_DEGREES = Util.intToStringMaxRadix(16);

/**
* Returns a {@link Bundle} that can be serialized to bytes.
*
* <p>Prefer the more efficient {@link #toBinderBasedBundle()} if the result doesn't need to be
* serialized.
*
* <p>The {@link Bundle} returned from this method must not be passed to other processes that
* might be using a different version of the media3 library.
*/
@UnstableApi
public Bundle toSerializableBundle() {
Bundle bundle = toBundleWithoutBitmap();
if (bitmap != null) {
ByteArrayOutputStream output = new ByteArrayOutputStream();
// The PNG format is lossless, and the quality parameter is ignored.
checkState(bitmap.compress(Bitmap.CompressFormat.PNG, /* quality= */ 0, output));
bundle.putByteArray(FIELD_BITMAP_BYTES, output.toByteArray());
}
return bundle;
}

/**
* Returns a {@link Bundle} that may contain {@link Binder} references, meaning it cannot be
* safely serialized to bytes.
*
* <p>The {@link Bundle} returned from this method can be safely sent between processes and parsed
* by older versions of the media3 library.
*
* <p>Use {@link #toSerializableBundle()} to get a {@link Bundle} that can be safely serialized.
*/
@UnstableApi
public Bundle toBinderBasedBundle() {
Bundle bundle = toBundleWithoutBitmap();
if (bitmap != null) {
bundle.putParcelable(FIELD_BITMAP_PARCELABLE, bitmap);
}
return bundle;
}

/**
* @deprecated Use {@link #toSerializableBundle()} or {@link #toBinderBasedBundle()} instead.
*/
@UnstableApi
@Override
@Deprecated
public Bundle toBundle() {
return toBinderBasedBundle();
}

private Bundle toBundleWithoutBitmap() {
Bundle bundle = new Bundle();
if (text != null) {
bundle.putCharSequence(FIELD_TEXT, text);
Expand All @@ -862,9 +914,6 @@ public Bundle toBundle() {
}
bundle.putSerializable(FIELD_TEXT_ALIGNMENT, textAlignment);
bundle.putSerializable(FIELD_MULTI_ROW_ALIGNMENT, multiRowAlignment);
if (bitmap != null) {
bundle.putParcelable(FIELD_BITMAP, bitmap);
}
bundle.putFloat(FIELD_LINE, line);
bundle.putInt(FIELD_LINE_TYPE, lineType);
bundle.putInt(FIELD_LINE_ANCHOR, lineAnchor);
Expand Down Expand Up @@ -915,9 +964,15 @@ public static Cue fromBundle(Bundle bundle) {
if (multiRowAlignment != null) {
builder.setMultiRowAlignment(multiRowAlignment);
}
@Nullable Bitmap bitmap = bundle.getParcelable(FIELD_BITMAP);
@Nullable Bitmap bitmap = bundle.getParcelable(FIELD_BITMAP_PARCELABLE);
if (bitmap != null) {
builder.setBitmap(bitmap);
} else {
@Nullable byte[] bitmapBytes = bundle.getByteArray(FIELD_BITMAP_BYTES);
if (bitmapBytes != null) {
builder.setBitmap(
BitmapFactory.decodeByteArray(bitmapBytes, /* offset= */ 0, bitmapBytes.length));
}
}
if (bundle.containsKey(FIELD_LINE) && bundle.containsKey(FIELD_LINE_TYPE)) {
builder.setLine(bundle.getFloat(FIELD_LINE), bundle.getInt(FIELD_LINE_TYPE));
Expand Down
3 changes: 2 additions & 1 deletion libraries/common/src/main/java/androidx/media3/common/text/CueGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public Bundle toBundle() {
Bundle bundle = new Bundle();
bundle.putParcelableArrayList(
FIELD_CUES,
BundleCollectionUtil.toBundleArrayList(filterOutBitmapCues(cues), Cue::toBundle));
BundleCollectionUtil.toBundleArrayList(
filterOutBitmapCues(cues), Cue::toBinderBasedBundle));
bundle.putLong(FIELD_PRESENTATION_TIME_US, presentationTimeUs);
return bundle;
}
Expand Down
81 changes: 65 additions & 16 deletions libraries/common/src/test/java/androidx/media3/common/text/CueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

import android.graphics.Bitmap;
import android.graphics.Color;
import android.os.Bundle;
import android.os.Parcel;
import android.text.Layout;
import android.text.SpannedString;
import androidx.test.ext.junit.runners.AndroidJUnit4;
Expand Down Expand Up @@ -109,6 +107,51 @@ public void buildWithBothTextAndBitmapFails() {
}

@Test
public void roundTripViaBinderBasedBundle_yieldsEqualInstance() {
Cue cue =
new Cue.Builder()
.setText(SpannedString.valueOf("text"))
.setTextAlignment(Layout.Alignment.ALIGN_CENTER)
.setMultiRowAlignment(Layout.Alignment.ALIGN_NORMAL)
.setLine(5, Cue.LINE_TYPE_NUMBER)
.setLineAnchor(Cue.ANCHOR_TYPE_END)
.setPosition(0.4f)
.setPositionAnchor(Cue.ANCHOR_TYPE_MIDDLE)
.setTextSize(0.2f, Cue.TEXT_SIZE_TYPE_FRACTIONAL)
.setSize(0.8f)
.setWindowColor(Color.CYAN)
.setVerticalType(Cue.VERTICAL_TYPE_RL)
.setShearDegrees(-15f)
.build();
Cue modifiedCue = Cue.fromBundle(cue.toBinderBasedBundle());

assertThat(modifiedCue).isEqualTo(cue);
}

@Test
public void roundTripViaSerializableBundle_yieldsEqualInstance() {
Cue cue =
new Cue.Builder()
.setText(SpannedString.valueOf("text"))
.setTextAlignment(Layout.Alignment.ALIGN_CENTER)
.setMultiRowAlignment(Layout.Alignment.ALIGN_NORMAL)
.setLine(5, Cue.LINE_TYPE_NUMBER)
.setLineAnchor(Cue.ANCHOR_TYPE_END)
.setPosition(0.4f)
.setPositionAnchor(Cue.ANCHOR_TYPE_MIDDLE)
.setTextSize(0.2f, Cue.TEXT_SIZE_TYPE_FRACTIONAL)
.setSize(0.8f)
.setWindowColor(Color.CYAN)
.setVerticalType(Cue.VERTICAL_TYPE_RL)
.setShearDegrees(-15f)
.build();
Cue modifiedCue = Cue.fromBundle(cue.toSerializableBundle());

assertThat(modifiedCue).isEqualTo(cue);
}

@Test
@SuppressWarnings("deprecation") // Testing deprecated Cue.toBundle() method
public void roundTripViaBundle_yieldsEqualInstance() {
Cue cue =
new Cue.Builder()
Expand All @@ -125,30 +168,36 @@ public void roundTripViaBundle_yieldsEqualInstance() {
.setVerticalType(Cue.VERTICAL_TYPE_RL)
.setShearDegrees(-15f)
.build();
Cue modifiedCue = parcelAndUnParcelCue(cue);
Cue modifiedCue = Cue.fromBundle(cue.toBundle());

assertThat(modifiedCue).isEqualTo(cue);
}

@Test
public void roundTripViaBundle_withBitmap_yieldsEqualInstance() {
public void roundTripViaBinderBasedBundle_withBitmap_yieldsEqualInstance() {
Cue cue =
new Cue.Builder().setBitmap(Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888)).build();
Cue modifiedCue = parcelAndUnParcelCue(cue);
Cue modifiedCue = Cue.fromBundle(cue.toBinderBasedBundle());

assertThat(modifiedCue).isEqualTo(cue);
}

private static Cue parcelAndUnParcelCue(Cue cue) {
Parcel parcel = Parcel.obtain();
try {
parcel.writeBundle(cue.toBundle());
parcel.setDataPosition(0);

Bundle bundle = parcel.readBundle();
return Cue.fromBundle(bundle);
} finally {
parcel.recycle();
}
@Test
public void roundTripViaSerializableBundle_withBitmap_yieldsEqualInstance() {
Cue cue =
new Cue.Builder().setBitmap(Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888)).build();
Cue modifiedCue = Cue.fromBundle(cue.toSerializableBundle());

assertThat(modifiedCue).isEqualTo(cue);
}

@Test
@SuppressWarnings("deprecation") // Testing deprecated Cue.toBundle() method
public void roundTripViaBundle_withBitmap_yieldsEqualInstance() {
Cue cue =
new Cue.Builder().setBitmap(Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888)).build();
Cue modifiedCue = Cue.fromBundle(cue.toBundle());

assertThat(modifiedCue).isEqualTo(cue);
}
}
11 changes: 8 additions & 3 deletions ...s/extractor/src/androidTest/java/androidx/media3/extractor/text/CueSerializationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.graphics.Color;
import android.graphics.ColorSpace;
import android.os.Bundle;
import android.text.Layout;
import android.text.Spannable;
Expand All @@ -35,7 +36,6 @@
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.common.collect.ImmutableList;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;

Expand Down Expand Up @@ -96,7 +96,6 @@ public void serializingCueWithoutSpans() {
}

@Test
@Ignore("Currently broken: https://github.com/androidx/media/issues/836")
public void serializingBitmapCue() throws Exception {
CueEncoder encoder = new CueEncoder();
CueDecoder decoder = new CueDecoder();
Expand All @@ -105,7 +104,13 @@ public void serializingBitmapCue() throws Exception {
TestUtil.getByteArray(
ApplicationProvider.getApplicationContext(),
"media/png/non-motion-photo-shortened.png");
Bitmap bitmap = BitmapFactory.decodeByteArray(imageData, 0, imageData.length);
BitmapFactory.Options options = new BitmapFactory.Options();
// Without this hint BitmapFactory reads an 'unknown' RGB color space from the file, which
// then causes spurious comparison failures later. Using a named RGB color space allows the
// Bitmap.isSameAs comparison to succeed.
options.inPreferredColorSpace = ColorSpace.get(ColorSpace.Named.SRGB);
Bitmap bitmap =
BitmapFactory.decodeByteArray(imageData, /* offset= */ 0, imageData.length, options);
Cue bitmapCue = new Cue.Builder().setBitmap(bitmap).build();

// encoding and decoding
Expand Down
3 changes: 2 additions & 1 deletion libraries/extractor/src/main/java/androidx/media3/extractor/text/CueEncoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public final class CueEncoder {
* @return The serialized byte array.
*/
public byte[] encode(List<Cue> cues, long durationUs) {
ArrayList<Bundle> bundledCues = BundleCollectionUtil.toBundleArrayList(cues, Cue::toBundle);
ArrayList<Bundle> bundledCues =
BundleCollectionUtil.toBundleArrayList(cues, Cue::toSerializableBundle);
Bundle allCuesBundle = new Bundle();
allCuesBundle.putParcelableArrayList(CueDecoder.BUNDLE_FIELD_CUES, bundledCues);
allCuesBundle.putLong(CueDecoder.BUNDLE_FIELD_DURATION_US, durationUs);
Expand Down

0 comments on commit bd19953

Please sign in to comment.