Do not flush eac3(joc) decoder on reuse #1346
Conversation
|
Hello @dwhea, Thank you for creating this pull request! Just to confirm, may I ask if you have any data backing up that the eac3(joc) has independent samples (like xHE-AAC codec)? |
|
Hi @microkatz, To respond to your query, eac3-joc frames are structured such that each frame represents an independent time slice (e.g. 32ms of audio) that is encoded such that it does not depend on other frames, and is packaged as a single access unit in container formats such as MP4. eac3-joc is encoded via the Enhanced AC-3 bitstream format which is documented here: https://www.etsi.org/deliver/etsi_ts/102300_102399/102366/01.03.01_60/ts_102366v010301p.pdf The core problem this pull request solves is to avoid issuing a flush event to the eac3/eac3-joc MediaCodec component under the following playback conditions:
Please let me know if you have any further queries. Thanks. |
|
Thanks @mzchan1 for responding. |
|
Hi @dwhea, Sorry for the delay. Would it be possible to open a new PR from an individual-owned fork? We can't push changes to organization-owned forks like this one unless we have collaborator access. If that's not possible then we can still merge this PR but it will result in an 'evil' merge. See more info here: https://github.com/androidx/media/blob/release/CONTRIBUTING.md#push-access-to-pr-branches |
|
Hi @microkatz , no problem. We're not able to open PR from individual-owned fork, so merging as an "evil merge" is OK for us. This was discussed between us two parties around January 2024 and we came to that decision then. |
| } | ||
|
|
||
| // For eac3 and eac3-joc formats, adaptation is possible without reconfiguration or flushing. | ||
| if (discardReasons == 0 && (MimeTypes.AUDIO_E_AC3_JOC.equals(mimeType) |
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
There's no initialization data for Dolby codecs.
If you are not opening a PR from an individual-owned fork then it still easier for us if given collaborator access. In that way we can submit commits to this branch during the review process. |
| // For eac3 and eac3-joc formats, adaptation is possible without reconfiguration or flushing. | ||
| if (discardReasons == 0 && (MimeTypes.AUDIO_E_AC3_JOC.equals(mimeType) | ||
| || MimeTypes.AUDIO_E_AC3.equals(mimeType))) { | ||
| return new DecoderReuseEvaluation( |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yes we will add those tests.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Thanks and sorry for long turnaround time. I've rebased and force pushed the update to the DolbyLaboratories:dlb/ddp-gapless branch.
|
I'm somewhat surprised by this change. My team has observed several cases on many devices where E-AC-3 codecs need not only a flush but also a reconfiguration when transitioning between two E-AC-3 tracks of differing bitrates, or E-AC-3 to E-AC-3+JOC. |
Thanks for the query! Although not expressly noted in the description, if you check out the proposed commit, you'll see that the new conditional check to reuse the decoder(ln: 482) occurs after the checks on sampleMimetype(ln: 414) and sampleRate(ln: 451). Does this help alleviate your concerns? |
|
Hey @dwhea. We need more information to resolve this issue but there hasn't been an update in 14 weekdays. I'm marking the issue as stale and if there are no new updates in the next 7 days I will close it automatically. If you have more information that will help us get to the bottom of this, just add a comment! |
dwhea
force-pushed
the
dlb/ddp-gapless
branch
2 times, most recently
from
8d58577 to
f5f1ce9
Compare
|
Since there haven't been any recent updates here, I am going to close this issue. @dwhea if you're still experiencing this problem and want to continue the discussion just leave a comment here and we are happy to re-open this. |
|
Hello, please update this issue. I did respond to this issue in #1346 (comment) 2 weeks ago but this seems to have been ignored. @microkatz could you please take a look? |
dwhea
force-pushed
the
dlb/ddp-gapless
branch
2 times, most recently
from
bc3f515 to
a797ad3
Compare
|
Thanks for the update for AC-4. One last question to be certain. Would the decoders for these formats need reconfiguration or flushing if the profiles or levels are different from the old to new formats? |
eac3(joc) and ac4 decoders (for the same profile and level content) do not need to be flushed to be reused for the next compatible track or across adapative bitrate transitions. This change allows for gapless playback on devices with Dolby decoders that require internal re-initialization on flush. This change introduces a codec check in canReuseCodec, in addition to the existing mimeType check, as within the same mimeType, such as AC-4, there can be codec changes (such as between AC-4 ac-4.02.01.04 for Atmos Objective Based Immersive and ac-4.02.02.00 for AC-4 Immersive Stereo) that may require decoder reinitialization for correct decoding.
It's possible that the AC-4 decoder needs re-initialization on AC-4 profile or level change. Although this is not always the case, to be conservative, I've updated the PR to check the format codec string, which encapsulates the AC-4 content profile and level information, and require re-initialization whenever this changes. |
eac3(joc) and ac4 decoders (for the same profile and level content) do not need to be flushed to be reused for the next compatible track or across adapative bitrate transitions. This change allows for gapless playback on devices with Dolby decoders that require internal re-initialization on flush. This change introduces a codec check in canReuseCodec, in addition to the existing mimeType check, as within the same mimeType, such as AC-4, there can be codec changes (such as between AC-4 ac-4.02.01.04 for Atmos Objective Based Immersive and ac-4.02.02.00 for AC-4 Immersive Stereo) that may require decoder reinitialization for correct decoding. This is a cherry-pick of 9b1f1b1 from androidx#1346 and 9b1f1b1 .
PiperOrigin-RevId: 794961682
5 participants
eac3(joc) decoders do not need to be flushed to be reused for the next compatible track. This change allows for gapless playback on devices with Dolby decoders that require internal re-initialization on flush.