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

FMP4 extractor can output metadata in the order if media contains v0 and v1 emsg atoms #9996

Closed
Koster35 opened this issue Feb 22, 2022 · 4 comments
Closed

FMP4 extractor can output metadata in the order if media contains v0 and v1 emsg atoms #9996

Koster35 opened this issue Feb 22, 2022 · 4 comments
Assignees
@ojw28
Labels
bug

Comments

@Koster35
Copy link

Hello! We are having a playback issue with our multi-period DASH livestreams. To summarize, when the ExoPlayer encounters small gaps between two periods in a DASH stream, the ExoPlayer throws an exception and playback stops.

Example Stack-trace:

2022-02-18 13:03:09.166 18017-18017/com.google.android.exoplayer2.demo E/EventLogger: playerFailed [eventTime=32.08, mediaPos=31.08, window=0, period=1
      com.google.android.exoplayer2.ExoPlaybackException: Source error
        at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:554)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.os.HandlerThread.run(HandlerThread.java:67)
     Caused by: com.google.android.exoplayer2.upstream.Loader$UnexpectedLoaderException: Unexpected IllegalArgumentException: null
        at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:436)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:920)
     Caused by: java.lang.IllegalArgumentException
        at com.google.android.exoplayer2.util.Assertions.checkArgument(Assertions.java:39)
        at com.google.android.exoplayer2.source.SampleQueue.commitSample(SampleQueue.java:739)
        at com.google.android.exoplayer2.source.SampleQueue.sampleMetadata(SampleQueue.java:583)
        at com.google.android.exoplayer2.source.chunk.BundledChunkExtractor$BindingTrackOutput.sampleMetadata(BundledChunkExtractor.java:211)
        at com.google.android.exoplayer2.extractor.mp4.FragmentedMp4Extractor.outputPendingMetadataSamples(FragmentedMp4Extractor.java:1443)
        at com.google.android.exoplayer2.extractor.mp4.FragmentedMp4Extractor.readSample(FragmentedMp4Extractor.java:1426)
        at com.google.android.exoplayer2.extractor.mp4.FragmentedMp4Extractor.read(FragmentedMp4Extractor.java:346)
        at com.google.android.exoplayer2.source.chunk.BundledChunkExtractor.read(BundledChunkExtractor.java:114)
        at com.google.android.exoplayer2.source.chunk.ContainerMediaChunk.load(ContainerMediaChunk.java:129)
        at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:415)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) 
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) 
        at java.lang.Thread.run(Thread.java:920) 
    ]

ExoPlayer Versions affected
This issue affects all recent releases of the ExoPlayer starting with v2.12.0. I confirmed it also affects the latest release v2.16.1. It does not affect ExoPlayer v2.11.8.

This issue is device independent, so it seemingly affects all devices.

Reproduction Steps
We have a sample video harvested directly from one of our DASH livestreams that can be viewed here: https://d2aieihupeq7i1.cloudfront.net/multi-period-issue/index.mpd

To reproduce the issue, play that video in the ExoPlayer Demo app and observe. Playback will stop at ~31 seconds.

ADB Bug Report
Google Pixel 5, Android 12.0
bugreport-redfin-SP1A.211105.003-2022-02-18-12-13-39.zip

Notes
It appears this is a regression that started with ExoPlayer v2.12.0 due to code added in this commit: a6be8ee

In that commit, an assertion was added to the beginning of SampleQueue#commitSample():

  private synchronized void commitSample(
      long timeUs,
      @C.BufferFlags int sampleFlags,
      long offset,
      int size,
      @Nullable CryptoData cryptoData) {
    if (length > 0) {
      // Ensure sample data doesn't overlap.
      int previousSampleRelativeIndex = getRelativeIndex(length - 1);
      checkArgument(
          offsets[previousSampleRelativeIndex] + sizes[previousSampleRelativeIndex] <= offset);
    }
      // ...
  }

As you can see by the stack-trace, this is where the IllegalArgumentException is thrown.
The commit message says this about the assertion:

Also add an assertion to SampleQueue that prevents the hard-to-detect failure mode of overlapping sample byte ranges.

The rest of the commit seems to be aimed at fixing an extractor issue related to HLS playback. However, this is breaking playback of our DASH streams, which I'd guess was not intended.

We have some upcoming deadlines that will likely make us unable to wait for a fix to be implemented. It appears that DASH playback works fine when I just comment out this assertion, so while we wait for a fix, we may fork the ExoPlayer and have this commitSample() method skip the assertion when DASH content is being played. Does that seem like a reasonable solution in the interim or do you have another suggested workaround?

Let me know if I can provide any other information. Thank you!
@christosts christosts self-assigned this Feb 23, 2022
@christosts
Copy link
Contributor

I can verify the issue occurs on 2.16.1 with the dash link provided. @ojw28 can you take a look from the DASH perspective? For reference, the commit in question (a6be8ee) was submitted to address #7690.

@christosts christosts assigned ojw28 and unassigned christosts Feb 23, 2022
@ojw28
Copy link
Contributor

ojw28 commented Feb 23, 2022 •
edited

Thanks for the detailed report! This isn't actually related to the manifest having multiple periods. The problem is caused by media segments that contain both version 0 and version 1 emsg atoms, which I think is true for the first video segment in the second period.

Version 0 emsg atoms are complicated because they contain presentation timestamps that are deltas relative to the timestamp of the first access unit in the segment. If we don't know the timestamp of the first access unit yet, we need to defer outputting the sample metadata corresponding to emsg atoms until we do, since we need to be able to calculate the absolute presentation timestamps to include in the metadata. Conversely, version 1 emsg atoms simply contain absolute presentation timestamps, and so the sample metadata can be output immediately. If you're interested, you can see where this happens in code here:

https://github.com/google/ExoPlayer/blob/r2.16.1/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/mp4/FragmentedMp4Extractor.java#L664

The mixture of version 0 and version 1 emsg atoms is a problem in the provided media because we encounter a version 0 emsg and defer writing the metadata, then encounter a version 1 emsg and output the metadata immediately, and then output the deferred metadata for the first emsg atom once we learn the first access unit timestamp. As a result the metadata corresponding to the samples is written in the wrong order. To fix this, we should make FragmentedMp4Extractor defer outputting metadata for version 1 emsg samples if there are already version 0 emsg samples that have been deferred, to ensure that the metadata is still output in the correct order. Marking as a bug to track fixing this (it should be fairly straightforward).

Regarding your workaround: It might work, but it doesn't feel like the safest thing to be doing. As per the description of the commit that added the assertion, the assertion prevents a "hard-to-detect failure mode". It would be safer to make a change in FragmentedMp4Extractor to fix the issue described above. We will provide a fix shortly, which you could cherry-pick if you needed to.

@ojw28 ojw28 changed the title Multi-period DASH playback issue FMP4 extractor can output metadata in the order if media contains v0 and v1 emsg atoms Feb 23, 2022
@Koster35
Copy link
Author

Thank you for the clear explanation @ojw28. I appreciate the details as I try to learn more about these different video specs.

I will pass along this information about the mixture of version 0 and version 1 emsg atoms to my team so we can investigate our DASH streams further.

@ojw28
Copy link
Contributor

ojw28 commented Feb 23, 2022

It looks there are two types of metadata in the segment, where one is using v1 and the other v0. I don't think mixing the two is invalid. That said, it does seem a bit unusual. You'd probably want to use v0 atoms everywhere (if you need to support clients that don't support v1 atoms), or you'd want to use v1 atoms everywhere because they're easier for clients to handle (if supported)!

We will push a fix to dev-v2 in the next couple of days. Thanks!

icbaker pushed a commit to androidx/media that referenced this issue Mar 1, 2022
@ojw28 @icbaker
FMP4: Fix output of mixed v0 and v1 emsg samples
5a304fd 
Issue: google/ExoPlayer#9996
#minor-release
PiperOrigin-RevId: 430773329
icbaker pushed a commit that referenced this issue Mar 1, 2022
@ojw28 @icbaker
FMP4: Fix output of mixed v0 and v1 emsg samples
1bc4ba2 
Issue: #9996
#minor-release
PiperOrigin-RevId: 430773329
@ojw28 ojw28 closed this as completed Mar 6, 2022
icbaker pushed a commit that referenced this issue Mar 9, 2022
@ojw28 @icbaker
FMP4: Fix output of mixed v0 and v1 emsg samples
fbb2bcb 
Issue: #9996
PiperOrigin-RevId: 430773329
(cherry picked from commit 1bc4ba2)
icbaker pushed a commit to androidx/media that referenced this issue Mar 9, 2022
@ojw28 @icbaker
FMP4: Fix output of mixed v0 and v1 emsg samples
d107004 
Issue: google/ExoPlayer#9996
#minor-release
PiperOrigin-RevId: 430773329
(cherry picked from commit 5a304fd)
@google google locked and limited conversation to collaborators May 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ojw28 ojw28

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ojw28 @Koster35 @christosts