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

Level controller does not select playable H264 stream when HEVC stream comes first in master playlist #3888

Closed
5 tasks done
MarcusWichelmann opened this issue May 12, 2021 · 7 comments · Fixed by #3889
Closed
5 tasks done

Comments

@MarcusWichelmann
Copy link

Hi,

my master.m3u8 provides two video stream variants with the same bitrate and same resolution but different codecs: HEVC for great quality and H264 for compatibility with most web browsers. But when playing this stream with hls.js, it gives me a manifestIncompatibleCodecsError: no level with compatible codecs found in manifest error.

After having a short look into the code, it seems like, when multiple levels/streams have the same bitrate, it only respects the first one and ignores all the following. So it only sees the first HEVC stream, notices that it is not playable and doesn't look further to the H264 stream that would be playable.

Maybe this behaviour could be improved?

What version of Hls.js are you using?

v1.0.3 on https://hls-js.netlify.app/demo/

What browser and OS (including versions) are you using?

Google Chrome 89.0.4389.72 on Fedora Linux

Test stream:

My stream isn't public but here is the relevant part, the master.m3u8:

#EXTM3U
#EXT-X-VERSION:6
#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="group_a0",NAME="audio_2",DEFAULT=YES,LANGUAGE="DE",URI="audio_aac_de.m3u8"
#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="group_a0",NAME="audio_3",DEFAULT=NO,LANGUAGE="FR",URI="audio_aac_fr.m3u8"
#EXT-X-STREAM-INF:BANDWIDTH=5640800,RESOLUTION=1280x720,CODECS="hvc1.1.4.L120.B01,mp4a.40.2",AUDIO="group_a0"
video_hevc.m3u8

#EXT-X-STREAM-INF:BANDWIDTH=5640800,RESOLUTION=1280x720,CODECS="avc1.4d4020,mp4a.40.2",AUDIO="group_a0"
video_h264.m3u8

Configuration:

{
  "debug": true,
  "enableWorker": true,
  "lowLatencyMode": true,
  "backBufferLength": 90
}

Checklist

Steps to reproduce

  1. Play a master.m3u8 that contains two stream variants with the same bitrate, same resolution but different codecs where the unsupported codec comes first
  2. The playback will fail, even though the playlist contains other variant streams that would be playable by hls.js

Expected behavior

The player skips the unsupported streams and selects the playable (H264) stream.

@robwalch
Copy link
Collaborator

HLS.js handles alternate codecs now, but in this case, the two variants are probably treated as redundant levels here (you'll need to verify as I don't have a test stream for this):

https://github.com/video-dev/hls.js/blob/v1.0.3/src/controller/level-controller.ts#L93

@MarcusWichelmann
Copy link
Author

Thanks for your fast response.
Yeah exactly that part of the code is what I meant. Because of the same bitrates, the two levels are grouped together and the h264 one is not recognized as an option anymore:

image

Do you have an idea how this special case could be handled cleanly?

@robwalch
Copy link
Collaborator

There's a hint here:

      levelFromSet = levelSet[levelParsed.bitrate]; // FIXME: we would also have to match the resolution here

so these are being grouped by bitrate. As a workaround, you could change the bitrate slightly that would fix the issue with how HLS.js is handling this.

What we want to do to handle redundancy correctly would be to require that bitrate (BANDWIDTH), RESOLUTION, and CODECS all match for them STERAM-INF variants to be grouped into a single level set here.

@MarcusWichelmann
Copy link
Author

Okay, that's what I thought about, too. I suppose some simple string representation of these three properties could be used as the key for levelSet here.

I will see when I find time to make a PR for that.

Slightly changing the bitrate of one of the streams doesn't look like a clean option to me because I don't want priorization to happen based on one stream having a higher bitrate in this case.

@robwalch
Copy link
Collaborator

@MarcusWichelmann,

I started on these changes (creating key based on attributes for level sets) and see an opportunity to clean up some unit tests as well. I'll put up a PR and link it to this issue so you can take a look.

@robwalch
Copy link
Collaborator

@MarcusWichelmann
Copy link
Author

Very nice! I'll give it a try shortly and comment on the PR after that.

robwalch pushed a commit that referenced this issue May 17, 2021
* master: (23 commits)
  [skip netlify] Update dependency netlify-cli to v3.29.16
  [skip netlify] Lock file maintenance
  [skip netlify] Lock file maintenance
  Remux overlapping AAC samples so that they are appended over earlier samples segment rather than dropping them
  [skip netlify] Update dependency eslint-plugin-import to v2.23.2
  [skip netlify] Update dependency eslint-plugin-import to v2.23.1
  [skip netlify] Update dependency netlify-cli to v3.29.15
  Drop audio frames with overlapping PTS in streams with video and alt audio tracks
  [skip netlify] Update dependency eslint-plugin-import to v2.23.0
  [skip netlify] Update dependency netlify-cli to v3.29.14
  [skip netlify] Update dependency netlify-cli to v3.29.13
  [skip netlify] Update dependency netlify-cli to v3.29.11
  [skip netlify] Update dependency netlify-cli to v3.29.10
  [skip netlify] Update dependency netlify-cli to v3.29.9
  [skip netlify] Update dependency netlify-cli to v3.29.8
  [skip netlify] Update dependency netlify-cli to v3.29.7
  [skip netlify] Update babel monorepo to v7.14.2
  Require redundant levels to have matching RESOLUTION and CODECS attributes Resolves #3888
  [skip netlify] Update dependency netlify-cli to v3.29.6
  [skip netlify] Update dependency netlify-cli to v3.29.5
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Release Planning and Backlog
  
Top priorities
Development

Successfully merging a pull request may close this issue.

2 participants