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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

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 @@ -479,6 +479,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

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

if (!oldFormat.initializationDataEquals(newFormat)) {
discardReasons |= DISCARD_REASON_INITIALIZATION_DATA_CHANGED;
}
Expand Down