Skip to content

Get regular playlist url properly in ha-hls-player #7417

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

Merged
bramkragten merged 6 commits into from
Nov 10, 2020
Merged

Get regular playlist url properly in ha-hls-player #7417

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
20ba761
Get regular playlist url properly
uvjustin Oct 21, 2020
d5daf64
Only use regular playlist if only one variant
uvjustin Oct 21, 2020
a8efd60
Fix regexp
uvjustin Oct 21, 2020
03d2d28
Use single regexp for master_playlist parsing
uvjustin Oct 23, 2020
1b5ce66
Add comments
uvjustin Oct 23, 2020
0dc5640
Use named groups and matchTwice const for clarity
uvjustin Nov 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/components/ha-hls-player.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ class HaHLSPlayer extends LitElement {

private async _startHls(): Promise<void> {
const videoEl = this._videoEl;
const playlist_url = this.url.replace("master_playlist", "playlist");
const useExoPlayerPromise = this._getUseExoPlayer();
const masterPlaylistPromise = fetch(this.url);

Expand All @@ -126,13 +125,19 @@ class HaHLSPlayer extends LitElement {
}

this._useExoPlayer = await useExoPlayerPromise;
let hevcRegexp: RegExp;
let masterPlaylist: string;
if (this._useExoPlayer) {
hevcRegexp = /CODECS=".*?((hev1)|(hvc1))\..*?"/;
masterPlaylist = await (await masterPlaylistPromise).text();
const masterPlaylist = await (await masterPlaylistPromise).text();

// use regular playlist instead of master playlist if possible
let playlist_url: string;
const playlistRegexp = /#EXT-X-STREAM-INF:.*?(?:CODECS=".*?(hev1|hvc1)?\..*?".*?)?(?:\n|\r\n)(.+)/g;
const match = playlistRegexp.exec(masterPlaylist);
if (match !== null && playlistRegexp.exec(masterPlaylist) === null) {

@bramkragten bramkragten Nov 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still is not clear what we are doing here, why do we do the regex a second time and why should it not match?

@uvjustin uvjustin Nov 10, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second regex is to test whether there's another #EXT-X-STREAM-INF line which would mean that our master playlist has more than one stream. We should actually not encounter this case currently as we are currently only include one stream in our master playlist, but in case we change things, this safely sends the master_playlist instead of skipping to the regular playlist when there is more than one stream available.

@bramkragten bramkragten Nov 10, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comment for that, can we also give the various match part logical variable names? like:
const isHevc = match && match[1];

@uvjustin uvjustin Nov 10, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a change, does that look better? I haven't had a chance to test it yet

@bramkragten bramkragten Nov 10, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that looks better. Let me know when you tested it :-)

@uvjustin uvjustin Nov 10, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested it and seems to be working fine

playlist_url = new URL(match[2], this.url).href;

@bramkragten bramkragten Oct 23, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain to me, and in a comment in the code, what we are doing here.

@uvjustin uvjustin Oct 23, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I added comments to the code which should help clarify things.
HLS has two types of playlists, a master playlist and regular playlists. The master playlist usually contains multiple variant streams and metadata which describes them (including bandwidth and resolution). The players can then choose between the variants depending on available resources.
The stream component originally just used a single regular playlist since we only have one stream variant. In 0.115, we made several changes to stream, including not including an audio stream with the video stream when there is no audio. However, this made chromecast devices not play the stream. We realized that we could fix around this problem by making a master playlist with only our single variant regular playlist with metadata which indicated that the stream had only video and no audio.
After this change, the stream component now sends a master playlist instead of a regular playlist. This is slightly less efficient since a master playlist must be loaded and parsed before the regular playlist can be loaded and played. Using a master playlist did help us by giving the ability to check the codec of the stream so we could restrict ExoPlayer use to only videos using the H.265 (HEVC) codec. But this actually resulted in 2 loads of the master playlist before the regular playlist load - once by ha-hls-player to check the codec, then once by the player itself. So we effectively went from one load (the player loading the regular playlist) to three loads (ha-hls-player loading the master playlist, the player loading the master playlist, then the player loading the regular playlist).
We previously reduced these 3 loads to 2 loads by passing the regular playlist directly to the player, helping the player avoid loading the master playlist. However, we did this by a simple string replace. @hunterjm noted that this might be undesirable because makes the frontend rely on the path of the variant stream coded in the backend.
This PR just gets the regular playlist correctly by parsing the regular playlist url from the master playlist.

} else {
playlist_url = this.url;
}
if (this._useExoPlayer && hevcRegexp!.test(masterPlaylist!)) {

if (this._useExoPlayer && match !== null && match[1] !== undefined) {
this._renderHLSExoPlayer(playlist_url);
} else if (hls.isSupported()) {
this._renderHLSPolyfill(videoEl, hls, playlist_url);
Expand Down