plugins.twitch: low latency

[bastimeyer]

Maybe this should be done in two separate pull requests, but here we go.
These changes are mainly based on the work of @back-to and his low latency test plugin for Twitch (see here).

Description

• Adds the --hls-segment-stream-data parameter, which makes Streamlink write the HLS segments to the output buffer while they are being downloaded.
• Adds HLS segment prefetching to the custom TwitchM3U8Parser.
• Lowers the playlist refresh time to the duration of the HLS segments (2 seconds).
• Adds the --twitch-low-latency shorthand plugin-parameter, which automatically sets --hls-segment-stream-data to true and limits the value of --hls-live-edge to a maximum of 2, as higher values (default is 3) are pointless for low latency streaming.

Open questions

• Is reducing the playlist refresh time by this much necessary? It's also what Twitch's web player does nowadays for regular and low latency streams and it basically refreshes the playlist after each segment download. I'm not sure if this defeats the segment prefetching idea. It seems to be right though to ignore the targetduration. Maybe a refresh time of number_of(prefetch_segments) * duration(segment) would make sense...
• Regarding ad filtering and additional segment metadata, is it the right approach while prefetching segments to clone the last segment of the sequence and replace its URL?
• Is introducing a shorthand parameter for setting other parameter values a good idea? I will have to reword the parameter description later, as segment prefetching is always being done.
• Tests need to be added...

As for the results of the low latency changes, I'm able to beat Twitch's web player by 0-2 seconds while using MPV without additional player cache. Using a HLS live edge value of 1 sometimes results in micro-buffering though, but this can be fixed by pausing the playback for a short moment.

Resolves #2180, #1676, #2402

[codecov]

Codecov Report

Merging #2513 into master will increase coverage by 0.16%.
The diff coverage is 94.59%.

@@            Coverage Diff             @@
##           master    #2513      +/-   ##
==========================================
+ Coverage   52.78%   52.95%   +0.16%     
==========================================
  Files         248      248              
  Lines       15466    15527      +61     
==========================================
+ Hits         8164     8222      +58     
- Misses       7302     7305       +3     

[bastimeyer]

• added hls-segment-stream-data to the Streamlink session options
• moved the disable_ads and low_latency properties to the TwitchHLSStream class, so that the other classes can access them via the stream property
• passed the stream instance to the TwitchM3U8Parser
• added HLS tag parsing conditions (as simple decorators)
• reverted the aggressive playlist reloading behavior when low_latency is not set
• improved tests and added tests for low latency logic
• changed / fixed plugin parameter description

[bastimeyer]

Btw, while I was adding the tests, I noticed that the plugin parameters do not get initialized properly and I had to set them explicitly. Looks like they only get initialized by streamlink_cli here:
https://github.com/streamlink/streamlink/blob/1.1.1/src/streamlink_cli/main.py#L888-L914

[beardypig]

I think this will do for now :) I have, what I think is, a better solution in the pipeline, but it is a little way off :)

[beardypig]

One note: why not override HLSStreamWorker.valid_sequence instead of introducing the complexity of parse_condition? :)

[bastimeyer]

HLSStreamWorker.valid_sequence

Ideally, this should have been done in the disable-ads PR earlier this year, so that TwitchHLSStreamWriter.write didn't need to be implemented. With these changes though, it doesn't make sense rewriting the entire thing again, unless you want it to be more complex.

Regarding the decorator, I've added the simple if statement to the prefetch tag method first, but it made sense to disable these advertisement tags too when the other parameter is not set, so adding a decorator made things look more simple, as all methods share this same logic. Btw, using HLSStreamWorker.valid_sequence to figure out whether the prefetch segments should be used or not would mean that they would always have to be created by the parser first and then discarded by the worker. A bit unnecessary.

---

I just wanted to get things finally done here with this PR. I know that most changes are not implemented the best way, but doing it right would require a much deeper refactoring of the entire thing and you already said that you had been working on something better a few months ago.

Let me fix the super class method call, though, before this gets merged.

[beardypig]

Fair enough @bastimeyer, I think I was out of the loop on those changes, or not paying close enough attention.
I think the parser should parse all of the segments regardless, it should only have one job - parse, now it has a dependency on the stream. If the worker wants to discard a segment that should be its responsibility, I personally think it makes it more difficult to understand (and less testable) to delegate to the parser, it might be slightly more efficient - but at what cost? Won’t someone think of the children!?

Hopefully the work I’m doing on the stream classes will make this kind of thing easier.

[bastimeyer]

Any further concerns, comments or suggestions?

If refactoring the plugin and removing the stream dependency from the parser is that important, then tell me, and I will change it. Or just push the necessary changes to my branch yourself :)

[beardypig]

As it's the twitch specific parser, I think it's OK. In my WIP branch, I have not used the stream (https://github.com/beardypig/streamlink/blob/stream-latency/src/streamlink/plugins/twitch.py). I haven't managed to get the latency down to faster than the browser though... What mpv cache settings did you use? And did you notice anything weird when reloading the playlist? I seem to get one prefetch segment in the first load, then in the next reload I get that previous prefetch segment as completed one, plus another completed one, and then more prefetch segments...

[bastimeyer]

What mpv cache settings did you use?

This depends on the value of hls-live-edge. If set to 2, then --cache=no is enough to not make it stutter (MPV's demuxer will still cache one additional second). If hls-live-edge is set to 1, then you need to increase MPV's cache a bit by 0.5 secs or so via --demuxer-readahead-secs=1.5 or --cache-secs.

did you notice anything weird when reloading the playlist?

Not really. Each playlist lists two prefetch segments. One of them is the previously completed one (live edge 1), and in the case that the reload/download takes too long, then the two prefetch segments are new ones.

I think using get_segment in your code is not the correct choice here, because the prefetch segments don't have a state which you can pop off the stack. That's why the last regular segment gets cloned for each prefetch segment in my PR.

faster than the browser

streamlink -l debug --twitch-low-latency --hls-live-edge 1 --player mpv --player-args '--cache=no --demuxer-readahead-secs=1.5 {filename}' twitch.tv/CHANNEL best

Test it on a channel where you can see an ingame timer or so (eg. dota2). I'm ahead by 2 seconds with streamlink.

[beardypig]

Thanks for the mpv example. I’ll took again how get_segment and the state work. I need to look at more playlists :)

[bastimeyer]

I've been thinking a little bit about the ads filtering, and I realized that we will probably have to make both parameters mutually exclusive.

As I've said, I don't get any ads on Twitch, so I can't test and verify this, but the way the low latency stuff is currently implemented is that it clones the last regular segment data, replaces the URL with the additional prefetch segment URLs and adds these new segments to the sequence as regular ones. This means that we're also shifting the live-edge here and since we're limiting the live-edge value to 1 or 2 when the parameter gets set, we don't have proper metadata to work with when it comes to ads filtering. This means that once an ad appears in the regular segments, we've already processed it as a prefetch segment without metadata and we're shifting the start/end of an ad-block by the amount of prefetch tags, which is two, or in other words 4 seconds. Since players like VLC don't handle discontinuities well, this breaks the whole ads-filtering implementation and is a problem.

Also, another thing I've realized is that we're also allowing low latency streaming for regular non-low-latency streams without prefetch segments. This isn't wrong per se, as it enables --hls-segment-stream-data and thus improves the latency by at most 2 seconds, but the log output implies that it's a low latency stream on Twitch's end, which isn't true. The log output needs to be changed, so that the user knows whether it's a true low latency stream or not. There's no information available on the Twitch API, so it has to be done by parsing the HLS playlist and checking whether it includes prefetch data.

[thinkpad4]

Has this been added to the nightly build? I tried a nightly from a week ago and couldn't get the --twitch-low-latency parameter to work.

[bastimeyer]

This is an open pull request, so of course not. If you want to try it, install it from the branch of this PR. See #2402 (comment)

[back-to]

• should this be merged or do you want to make more changes?
• should we wait for the updated HLSStream version? (which will also take longer)

[thinkpad4]

Any update on this getting pushed?

[bastimeyer]

Locking this PR now.

I've already said multiple times that it's a WIP and not finished yet. This hasn't changed, so please stop asking, this is just unnecessary noise and not contributing anything. As you can see on my Github profile, I didn't have the time recently to work on any of my projects or pull requests.

I will see if I can add the necessary changes this weekend, but I'm not sure.

There's also @beardypig with his implementation, but I don't know anything about the status of that.

---

What needs to be done?

• Make ad filtering mutually exclusive with low latency streaming
  LLS will be disabled if ad filtering is enabled.
• "Disable" low latency streaming if it's not a low latency stream
  Just prints an info log message if a stream is not low latency (missing prefetch segments).
• Add "proper" documentation

[bastimeyer]

I've rebased the branch to 1.3.0. This doesn't mean though that there's anything new or that I'm confident getting this merged any time soon.

If you're testing the low latency branch, you should reinstall it. Btw, the next Twitch GUI version will require 1.3.0, so if you're using that, you will have to upgrade.

[bastimeyer]

I've force-pushed a rebase from Streamlink's current master branch, so that the MPV fixes are now included and merge conflicts solved. Don't merge though, nothing has changed.

edit
had to rebase again because of the Twitch GUI's bumped Streamlink version requirement

[back-to]

Regarding ad filtering and additional segment metadata, is it the right approach while prefetching segments to clone the last segment of the sequence and replace its URL?

if you could detect the Ad URLs they could be ignored

as you are using the prefetch URLs there shouldn't be an issue,
I don't think there are Ad URLs in the prefetch URLs.

but this is only testable with a larger user base

Add proper documentation

nice to have, but not important for testing purposes

---

i would merge this PR and see what issues occupy on the fly.

[bastimeyer]

I don't think there are Ad URLs in the prefetch URLs.

Hm, that is actually true.

Add proper documentation
i would merge this PR and see what issues occupy on the fly.

Even if the --twitch-low-latency parameter gets set, there is still some tweaking to do by the user on the player side, otherwise the benefits are minimal. I think that should be mentioned at least. Also the live-edge value, which is all tied to it. Not sure if an example parameter string would be a good idea, because it depends on the player that's being used. I also haven't looked into VLC parameters yet, which is problematic, because it's Streamlink's default player. Let me improve the parameter description at least a bit though.

After I've rebased the branch and fixed the merge conflicts, then I think we can merge this. In another thread somewhere here I've said that I don't really feel comfortable with the changes, because there are still a couple of issues with occasional hiccups, but to be honest, I also don't want this PR to be open and stale anymore. Merging this will probably lead to lots of further questions, but so be it then.

edit:
What I forgot to mention are the non-low-latency streams, which can be an issue sometimes if the parameter gets set, because it limits the live-edge value, and that is not beneficial if there are no prefetch segments. The non-LL streams can only be detected if there are no prefetch segments in the playlist, and reverting the live-edge value at this point is too late, which makes me question whether limiting it is the right choice.

[bastimeyer]

After fiddling around a bit more with MPV's caching parameters (and also with python-sphinx), I've decided against adding an example command line, because it's just too variable. It's probably also unnecessary when using the latest MPV version, because not adjusting anything at all didn't cause any major hiccups when I tested it and the latency was still good. This wasn't the case a few months ago.

I've also not changed the mutual exclusivity of the disable-ads parameter and also not mentioned it in the description. Since a message gets logged to info, I don't think it's necessary. We can change this stuff later if it causes issues.