Buffering and adaptive bitrate switching #732
Conversation
|
TODO/issues:
|
glennguy
force-pushed
the
rework-new
branch
4 times, most recently
from
138fb1c
to
8d89ff4
Compare
|
Hey guys, thanks a lot for this rework. I hope this solves most of the buffering issues. A question, was also Live Dash streaming like Zattoo was taken into consideration? |
|
@popy2k14 No idea about 'Zattoo' but streams that work at the moment should work in this. Nothing has changed with regards to parsing particular types of manifests, just how downloading segments and storing them in a buffer along with switching quality to suit the current bandwidth. |
|
ok, thx, is there an android TV (arm7) testbuild for kodi 18.9? (yes my family dont want me to upgrade :-( ) |
|
I'm sorry to say the code is only for Kodi 19 at the moment. Also, this will be released for Kodi 20 first, and possibly be backported to Kodi 19 afterwards. Currently 19 and 20 binary addons are compatible but that is likely to change sometime in the near future. |
|
ok, thanks for clearing things up. Thank you |
|
Sure thing, current builds are here: https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Finputstream.adaptive/detail/PR-732/15/artifacts |
glennguy
added
v20 Nexus
Issue Type: New feature
|
Should all commits be reviewed or just yours @glennguy? Also, there are a number of merge commits. In a PR they should not be required., or the PR should be cleared up to not depend on them. If that's just way too much work and we should just get this in please let me know. |
|
Just my commits, I can't speak to the ones by peak and spyderboy. I'll see if I can rebase and get rid of the merge commits to clear it up. |
Check the tree hasn't been recently updated when adding new segments to the buffer
With TS streams the elapsed time would be calculated incorrectly as during the tree refresh, nextSegment would be deleted by the FreeSegments/newsegments swap. Do this now before the tree refresh. Also, when reopening a stream (switching reps) the elapsed time would be incorrectly set until the second segment plays, now force a correct calculation at the start of the stream.
Scenario: Multi period stream starts, and due to previous bandwidth conditions the stream starts with a low bandwidth. Some small time passes and we're on the best rep available. When we transition to the next period we get given the low bandwidth again. This change will make sure that before we get the first segment that we select an appropriate representation based on the download speeds so far.
Because with HLS multiperiod the first period ID will always be 1 (therefore streams 1001, 1002 etc) we need to make sure that is always returned for that period, even though ID will be whatever the disco seq is + 1 - Kodi will never be able to know the real ID. Otherwise when reopening stream we will use the period ID, not 1 and stream will fail. Worked fine previously (pre rework) as same stream was never reopened
... if a new representation is chosen at the start of a new period otherwise start_stream is working with an empty rep. Correct `update` flag is also passed into prepareRepresentation after choosing new rep in ensureSegment
Previous behavior with live streams was to always start a new stream ~12 seconds from the live edge (except multiperiod - only last period starts this way). It does this by selecting `current_segment_` when starting the stream. However we now need to keep the same position for `current_segment_` when reopening a stream that has switched reps as the new rep will not have or at least an up-to-date `current_segment_`.
As with the PTS offset issue, `nextSegment` has been deleted by way of refreshing the tree. Since this becomes the new `current_segment_` we run into trouble. Affected live streams.
Will make testing AdaptiveStream possible
Tests won't compile if anything needs a reference to the addon instance
Switching audio representations is not overly helpful and can cause audio glitches or no audio at all.
Downloaded urls are now kept as a list to be cleared after each test. This list also includes the initialization segment so tests are updated accordingly. The amount of segments that the worker thread downloads is not reliably predictable. Sometimes it will get only the amount of segments that we ask to 'read' but other times it will run away and start filling the available buffers on top of the reads requested, making the previous use of `downloadedUrls.back()` unreliable.
Changing reps on the last segment of a period causes issues with fmp4 streams: due to design elsewhere the init segment will never be downloaded; the `stream_changed_` check has no effect as Kodi has already opened the new stream to feed its buffer.
glennguy
force-pushed
the
rework-new
branch
from
c514f7a
to
d69eb7f
Compare
|
I've installed this on a Shield Pro and have been using it with @matthuisman's SkySportNow addon and Youtube, here's by 2 cents. There doesn't seem to be a way to disable the adaptive switching. The stream selection option "manually select all streams" doesn't actually disable the switching. It just allows you to manually select a stream which may be overwritten by the adaptive function shortly afterwards. The screen flashes black whenever the stream is changed, can be annoying as the streams change very frequently some times. The screen sometimes remains black for a few seconds and then the video resumes and fast-forwards to catchup with the audio, which usually doesn't cut out. Overall it seems very good, thanks for the great work. |
|
Installed on Archlinux, Matrix. Used your personal branch (https://github.com/glennguy/inputstream.adaptive/tree/rework-new) |
|
@kontell Thanks for the feedback. I'll note the issue with not being able to force a manual selection. This PR is for the minimum viable product i.e it works, and most users will want to just leave it on default. WRT the screen blanking on Android - good to know, it works well on Windows with only a tiny amount of resyncing so maybe there's some platform specific issues with stream switching. The issue may lie on Kodi's side of the border @bibitocarlos thanks as well for trying it out. The gray bar on the OSD is reporting Kodi's internal 8 second buffer for streams and is unaware of the buffer internal to inputstream.adaptive. Maybe in would be a feature that could be added to the inputstream API to query buffer size and fullness. Perhaps something useful that could be added on this side would be some debug logging of buffer health. |
Adaptive does not support the times interface for inputstreams yet. I.e. |
|
Does local buffering is really implemented in Inputstream Without Inputstrem
With Inputstream |
|
kodi core currently dont have an implementation to show internal buffer of binary addons to the GUI |
|
It does not show in progress bar also like "without Inputstream"? Reason is, sometimes, I face buffering problem in case of Inputstream. Is there any specific settings to implement it?
|
|
no that buffering show in your picture is the Kodi VP buffer and not the ISA buffer (assuming that its not a streaming provider problem) if depends from your network that is not capable to fullfill the VP+ISA buffer then you can force limit bitrate from ISA settings and/or set a stream selection type "ask" in to ISA settings to select manually a lower bitrate if you have buffering problems with a specific manifest type, please open a new Issue by filling the template to provide all required info |
|
In normal case, while you are facing problem from ISP, if you pause for some 10-15 minutes and play. It start working smoothly but in case of Inputstream, it play for 1-2 second and then again start buffering no matter how long you pause the video. Mostly using MPD link. |
|
I believe that adaptive could implement the iTimes interface. I tried to do it a couple of years back. I’m sure the PR is still about somewhere. |
i dont have encoutered this behaviour, there are many types of manifests then so without knowing you can't get any help, please stop use PR's pages like this to talk about problems, these are development threads only, Issue threads are done for them |
Description
This continues on the great work started last year by @spyderboy92 and @peak3d to bring this feature to this add-on.
Motivation and context
The complaint of no real buffering feature comes up constantly in this repo and on forums. Users are usually unaware of the limitation and end up frustrated when changing buffering settings in advancedsettings.xml doesn't improve the performance. The GSOC 2020 project of implementing a solution to this gave some hope however due to personal commitments @peak3d wasn't able finalise and bring the changes in.
Adaptive bitrate switching is also a much requested feature, I know in my home country where internet connections are mediocre at best and many still on ADSL2 this would be a much welcomed improvement. Add-on authors have incorporated features like stream selection dialog and logic such as in @matthuisman's add-ons to help the user get the correct stream for their needs however a solution where the stream will adapt over playback to the conditions is what's really needed.
From the starting point in #506 multiple improvements and fixes have been made. I've documented briefly in each commit for myself and others to aid future contributions.
How has this been tested?
The bulk of change in the code is in the AdaptiveStream class, and so the tests have been updated to run correctly.
There is an outstanding issue with how the thread locks are structured and acquired. So far in my testing it behaves fine at runtime but under automated test it caused issues. There is a workaround in place of sleeping the testing thread for a brief amount of time to avoid deadlocking, I hope that in the short term future myself or another can rewrite the affected area to acquire and release locks in a consistent order.
What is the effect on users?
Users should have a pleasant experience hopefully devoid of any buffering pauses, as well as seeing the best quality that's possible on their connection.
Types of change
Checklist:
Note:
The codebase as a whole is full of mixed conventions and doesn't conform to the same unified standard as required in xbmc. The plan is to follow this PR with a big cleanup involving renaming variables to be consistent and fixing formatting issues.
Fixes #726, #392, #141, #14, #8