Skip to content

Do not flush eac3(joc) decoder on reuse #1346

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+98 −0
Open

Do not flush eac3(joc) decoder on reuse #1346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,17 @@ public DecoderReuseEvaluation canReuseCodec(Format oldFormat, Format newFormat)
}
}

// For eac3 and eac3-joc formats, adaptation is possible without reconfiguration or flushing.
if (discardReasons == 0 && (MimeTypes.AUDIO_E_AC3_JOC.equals(mimeType)
Copy link
Contributor

@microkatz microkatz Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@dwhea
Do Dolby Audio decoders use any initalization data provided by the stream? Just checking if this check needs to occur after the initialization data comparison right below.

Copy link
Contributor Author

@dwhea dwhea Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There's no initialization data for Dolby codecs.

|| MimeTypes.AUDIO_E_AC3.equals(mimeType))) {
return new DecoderReuseEvaluation(
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be able to add a unit test to the MediaCodecInfoTest file that given a call of canReuseCodec for dolby content of the same mimetype that REUSE_RESULT_YES_WITHOUT_RECONFIGURATION is returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we will add those tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I force pushed a commit to DolbyLaboratories@ade6e4f which rebased to Google upstream main branch, and added the requested tests.

I'm not sure how to update this pull request with those changes. Please advise if I need to do anything, or whether you can pull that new commit into this pull request (or create a new pull request).

Copy link
Contributor

@microkatz microkatz Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I believe that you need to make the commits to your topic branch, DolbyLaboratories:dlb/ddp-gapless, and then rebase the topic branch back onto main. With a git --force push command that should update the PR with your requested commits and update the PR's base commit as main head.

Without the collaborator access I'm unable to do this for you.

I also made a comment in the referenced commit as well.
DolbyLaboratories@ade6e4f#r145885998

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks and sorry for long turnaround time. I've rebased and force pushed the update to the DolbyLaboratories:dlb/ddp-gapless branch.

name,
oldFormat,
newFormat,
REUSE_RESULT_YES_WITHOUT_RECONFIGURATION,
/* discardReasons= */ 0);
}

if (!oldFormat.initializationDataEquals(newFormat)) {
discardReasons |= DISCARD_REASON_INITIALIZATION_DATA_CHANGED;
}
Expand Down
87 changes: 87 additions & 0 deletions ...ries/exoplayer/src/test/java/androidx/media3/exoplayer/mediacodec/MediaCodecInfoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package androidx.media3.exoplayer.mediacodec;

import static androidx.media3.common.MimeTypes.AUDIO_AAC;
import static androidx.media3.common.MimeTypes.AUDIO_E_AC3;
import static androidx.media3.common.MimeTypes.AUDIO_E_AC3_JOC;
import static androidx.media3.common.MimeTypes.VIDEO_AV1;
import static androidx.media3.common.MimeTypes.VIDEO_H264;
import static androidx.media3.exoplayer.DecoderReuseEvaluation.DISCARD_REASON_AUDIO_CHANNEL_COUNT_CHANGED;
Expand Down Expand Up @@ -73,6 +75,24 @@ public final class MediaCodecInfoTest {
.setInitializationData(ImmutableList.of(new byte[] {4, 4, 1, 0, 0}))
.build();

private static final Format FORMAT_EAC3 =
new Format.Builder()
.setSampleMimeType(AUDIO_E_AC3)
.setChannelCount(6)
.setSampleRate(48000)
.setAverageBitrate(5000)
.setInitializationData(ImmutableList.of(new byte[] {4, 4, 1, 0, 0}))
.build();

private static final Format FORMAT_EAC3JOC =
new Format.Builder()
.setSampleMimeType(AUDIO_E_AC3_JOC)
.setChannelCount(12)
.setSampleRate(48000)
.setAverageBitrate(5000)
.setInitializationData(ImmutableList.of(new byte[] {4, 4, 1, 0, 0}))
.build();

@Test
public void canReuseCodec_withDifferentMimeType_returnsNo() {
MediaCodecInfo codecInfo = buildH264CodecInfo(/* adaptive= */ true);
Expand Down Expand Up @@ -299,6 +319,43 @@ public void canReuseCodec_audioWithDifferentInitializationData_returnsNo() {
DISCARD_REASON_INITIALIZATION_DATA_CHANGED));
}

@Test
public void canReuseCodec_eac3_returnsYesWithoutReconfiguration() {
MediaCodecInfo codecInfo = buildEac3CodecInfo();

Format variantFormat =
FORMAT_EAC3
.buildUpon()
.setInitializationData(ImmutableList.of(new byte[] {0}))
.build();
assertThat(codecInfo.canReuseCodec(FORMAT_EAC3, variantFormat))
.isEqualTo(
new DecoderReuseEvaluation(
codecInfo.name,
FORMAT_EAC3,
variantFormat,
DecoderReuseEvaluation.REUSE_RESULT_YES_WITHOUT_RECONFIGURATION,
/* discardReasons= */ 0));
}
@Test
public void canReuseCodec_eac3joc_returnsYesWithoutReconfiguration() {
MediaCodecInfo codecInfo = buildEac3JocCodecInfo();

Format variantFormat =
FORMAT_EAC3JOC
.buildUpon()
.setInitializationData(ImmutableList.of(new byte[] {0}))
.build();
assertThat(codecInfo.canReuseCodec(FORMAT_EAC3JOC, variantFormat))
.isEqualTo(
new DecoderReuseEvaluation(
codecInfo.name,
FORMAT_EAC3JOC,
variantFormat,
DecoderReuseEvaluation.REUSE_RESULT_YES_WITHOUT_RECONFIGURATION,
/* discardReasons= */ 0));
}

private static MediaCodecInfo buildH264CodecInfo(boolean adaptive) {
return new MediaCodecInfo(
"h264",
Expand Down Expand Up @@ -329,6 +386,36 @@ private static MediaCodecInfo buildAacCodecInfo() {
/* detachedSurfaceSupported= */ false);
}

private static MediaCodecInfo buildEac3CodecInfo() {
return new MediaCodecInfo(
"eac3joc",
AUDIO_E_AC3,
AUDIO_E_AC3,
/* capabilities= */ null,
/* hardwareAccelerated= */ false,
/* softwareOnly= */ true,
/* vendor= */ true,
/* adaptive= */ false,
/* tunneling= */ false,
/* secure= */ false,
/* detachedSurfaceSupported= */ false);
}

private static MediaCodecInfo buildEac3JocCodecInfo() {
return new MediaCodecInfo(
"eac3joc",
AUDIO_E_AC3_JOC,
AUDIO_E_AC3_JOC,
/* capabilities= */ null,
/* hardwareAccelerated= */ false,
/* softwareOnly= */ true,
/* vendor= */ true,
/* adaptive= */ false,
/* tunneling= */ false,
/* secure= */ false,
/* detachedSurfaceSupported= */ false);
}

private static ColorInfo buildHdrColorInfo(@C.ColorSpace int colorSpace) {
return new ColorInfo.Builder()
.setColorSpace(colorSpace)
Expand Down