shaka-player icon indicating copy to clipboard operation
shaka-player copied to clipboard

Error MANIFEST.PERIOD_FLATTENING_FAILED for MPD with multiple audio tracks

Open mjneil opened this issue 3 years ago • 1 comments

Have you read the FAQ and checked for duplicate open issues? yes

What version of Shaka Player are you using? shaka-player demo page, v3.0.10 at time of writing

Can you reproduce the issue with our latest release version? yes

Can you reproduce the issue with the latest code from master? yes

Are you using the demo app or your own custom app? demo

If custom app, can you reproduce the issue using our demo app?

What browser and OS are you using? chrome on mac

For embedded devices (smart TVs, etc.), what model and firmware version are you using?

What are the manifest and license server URIs? Here is an example, The segment urls themselves have been modified so it won't play content, but it should fail on manifest parse with MANIFEST.PERIOD_FLATTENING_FAILED https://gist.githubusercontent.com/mjneil/e6d1de78f91b8b25b07135bde62fc4ef/raw/debf2766d4cacd45992341a37a48fa78965402fd/shaka.mpd

What configuration are you using? What is the output of player.getConfiguration()? default demo

What did you do? load manifest

What did you expect to happen? It to play

What actually happened? MANIFEST.PERIOD_FLATTENING_FAILED error for manifest with multiple audio tracks. Seems to be an edge case when you have multiple tracks of the same language reliant on ordering. In my case, two English tracks, the primary track and a commentary track. The commentary track is listed first in the manifest, so during the period combine algorithm, when comparing each unused stream with the outputStream for a best match the main English track, which is listed second in the manifest is incorrectly decided to be the best match because the languages match and it is the primary track https://github.com/google/shaka-player/blob/master/lib/util/periods.js#L1053..L1071

I think the fix is simple. An additional check at the top of this best match method https://github.com/google/shaka-player/blob/master/lib/util/periods.js#L1042

    // If the output stream was based on the best stream, then we already
    // have the best match. We can check this by comparing their ids.
    if (outputStream.id == best.id) {
      return false;
    }

I've tested this modification in chrome dev tools and it seems to work. I think you would have a similar problem with video and text tracks, but I did not verify this.

mjneil avatar Apr 12 '21 22:04 mjneil

Confirmed the bug. First, there is a problem with this line where we iterate over a list but also mutate the list. This causes us to skip elements. It also seems like you're correct that we're selecting the wrong stream.

TheModMaker avatar Apr 27 '21 20:04 TheModMaker

We recently ran into the same issue when adding accessibility sub/audio tracks to our streams, any chance this could be bumped in priority?

We have working and non-working manifest samples (depending on track order) that could be emailed for testing, should you require more material to test with.

martinstark avatar Oct 13 '22 13:10 martinstark

@martinstark are you interested on send a PR?

avelad avatar Feb 09 '23 09:02 avelad

@avelad I'll see if I can refresh myself on the issue and find stream samples to test the suggested fix on, my current assignment migrated Slack organizations without keeping any history :sweat:.

martinstark avatar Feb 09 '23 10:02 martinstark

@avelad @mjneil Using the manifest in the report I can reproduce the issue on 3.0.10, but I cannot reproduce the issue on 3.3.17, 4.2.4, or 4.3.4. It may take me some time to get access to the manifests I used to reproduce the issue on 4.2.X back in October, due to lost history and people being on holiday.

Newer versions of shaka stop on BAD_HTTP_STATUS when attempting to fetch the segments in the modified manifest above.

martinstark avatar Feb 09 '23 12:02 martinstark

I have no way of reproducing this since our new packager won't put audio variants at the top of the manifest. To reproduce it the first two tracks need to be audio, and the very first needs to be type="commentary".

Can someone possibly help me with setting up a test stream (with working segments)? If so I'm happy work on a fix.

martinstark avatar Feb 17 '23 14:02 martinstark

I can confirm that the error no longer occurs in new versions.

avelad avatar Sep 25 '23 11:09 avelad