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

Presentation time offset for MPEG DASH manifests not applied correctly for xHE-AAC and AC4 streams #1440

Closed
1 task done
ronak2121 opened this issue Jun 10, 2024 · 14 comments
Assignees
Labels

Comments

@ronak2121
Copy link

ronak2121 commented Jun 10, 2024

Version

Media3 1.3.0

More version details

No response

Devices that reproduce the issue

Samsung S21+ with Android 14

Devices that do not reproduce the issue

None

Reproducible in the demo app?

Yes

Reproduction steps

Play attached media
Wait for the first period to finish
Watch the second period not honor the offset value (effectively restart playback from the beginning of the content)

Expected result

Media plays correctly - the first period and the second, honoring the offset correctly

Actual result

Playback starts, plays the first period, and then plays the second one, without honoring the presentationTimeOffset.

Media

We actively worked with the Exoplayer team here: google/ExoPlayer#11000 to fix discontinuities with AC-4 streams. We just adopted the fix, but it looks to have broken applying the presentationTimeOffset correctly for both xHE-AAC and AC-4 audio streams. For some reason, it looks to be working with AAC-LC (which doesn't have I-Frames).

Based on our debugging, the commit the Exoplayer team made for our above issue is what caused this. Can you please look into what's going on, and please apply the correct fix?

We're not sure if you still have our AC-4 stream from the past, but you can still use that. We will also email you links to three clear streams - an AAC one (where presentationTimeOffset is being honored) and xHE-AAC and AC-4 where they are not anymore.

Bug Report

@ronak2121
Copy link
Author

FYI @rohitjoins and @tonihei

@ronak2121
Copy link
Author

FYI we've emailed the adb bug report to the above email address. We will send you sample audio later today. Would appreciate a set of eyes from your team on this soon.

We still think our original proposed patch from: google/ExoPlayer#11000 is the actual solution to the original problem rather than messing with timestamps....but open to hearing your guidance as well.

@ronak2121 ronak2121 changed the title Presentation Time Offset for MPEG DASH manifests not applied correctly for xHE-AAC and AC4 streams Presentation time offset for MPEG DASH manifests not applied correctly for xHE-AAC and AC4 streams Jun 12, 2024
@ronak2121
Copy link
Author

We've emailed an XHE-AAC stream as promised. An AC4 one will be forthcoming.

@ronak2121
Copy link
Author

@rohitjoins, can you please advise on the next steps for this issue?

@ronak2121
Copy link
Author

Hi @rohitjoins , is there any update?

@rohitjoins rohitjoins assigned tonihei and unassigned rohitjoins Jun 25, 2024
@tonihei
Copy link
Collaborator

tonihei commented Jun 25, 2024

@ronak2121 The xHE-AAC link you shared via email results in HTTP 403 errors for me. Could you check that it's accessible or provide more details on how to access it? Not sure what the link is targeting, but the most important thing to have for testing is the actual DASH manifest referencing real media files that show the problem.

@ronak2121
Copy link
Author

Just emailed you guys again with a new link.

@tonihei
Copy link
Collaborator

tonihei commented Jun 28, 2024

The link still gives 403 errors I'm afraid. Not sure if it expired or if there is another problem, but we can't really debug and look into the issue if it's not accessible to us.

@ronak2121
Copy link
Author

Hi I emailed you a new link a few weeks ago. Please try it out.

@tonihei
Copy link
Collaborator

tonihei commented Jul 19, 2024

Thanks for the new stream, I was now able to access it. We don't monitor this email address directly, so please let us know here is you send new information to make us aware.

I was able to reproduce the issue and it is caused by the fact that the second period starts in the middle of a segment. This requires the player to preroll the data from the beginning of the segment. For formats without specific keyframes, there is an optimization that skips the samples early (for AAC-LC), so it seems to work correctly. For other formats like xHE-AAC and AC-4, we need to start decoding at earlier samples and can't skip them directly. Usually they would be skipped later, after decoding, but our DASH implementation didn't expect that a period starts in the middle of a segment and thus simply played out the entire segment again.

We can look into solving this, by note that even if we report it correctly in our pipelines, the player likely enters a very short buffering state at the transition where it needs to flush and restart the codec.

@ronak2121
Copy link
Author

ronak2121 commented Jul 19, 2024 via email

@tonihei
Copy link
Collaborator

tonihei commented Jul 19, 2024

The first segment has a duration of about 20 seconds and is supposed to start at around 10 seconds. The presentation time offset is applied correctly in some parts of the pipeline, but we don't tell the decoder that it needs to discard some of the data again. This is because of the wrong assumption that every DASH period starts at the beginning of a segment.

We have I-Frames on every ~1s fragment so I’m not following what you’re referring to there.

Each fragment has its own sample index table, so the player still needs to parse through the entire file to find the right point.

@ronak2121
Copy link
Author

Thanks Tony. I get it now. Please let us know when this is closer to being fixed so we can test it out with our streams.

copybara-service bot pushed a commit that referenced this issue Aug 29, 2024
DASH periods don't have to start at the beginning of a segment. In
these cases, they should report an initial discontinuity to let the
player know it needs to expect preroll data (e.g. to flush renderers)

This information is only available in the ChunkSampleStream after
loading the initialization data, so we need to check the sample
streams and tell them to only report discontinuities at the very
beginning of playback. All other position resets are triggered by
the player itself and don't need this method.

Issue: #1440
PiperOrigin-RevId: 668831563
@tonihei
Copy link
Collaborator

tonihei commented Aug 29, 2024

We submitted a fix for the main problem now (see commit above). But as highlighted earlier, there will still be a tiny buffering state at the transition point where ExoPlayer needs time to go through the decode-only samples it won't need for playback. Closing this issue as the remaining buffering is a wider performance improvement related to how we handle pre-roll samples at transition points.

@tonihei tonihei closed this as completed Aug 29, 2024
@androidx androidx locked and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants