Skip to content

stream.dash: respect the manifest's minBufferTime #5610

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

Conversation

bastimeyer
Copy link
Member

Resolves #5608

Make `MPDParsers.duration()` return a callable which always transforms
`isodate.Duration` objects (year or month durations)
to `datetime.timedelta` objects by using a `datetime.datetime` anchor.

Use the manifest's `publishTime` as anchor if it's set, otherwise
use the current time.
Don't allow the manifest's `suggestedPresentationDelay` to be lower than
the `minBufferTime` value.
@bastimeyer
Copy link
Member Author

@michaelarnauts You can give this branch a try and provide feedback if you want. Thanks.

Copy link
Member

@gravyboat gravyboat left a comment

Choose a reason for hiding this comment

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

Approved pending confirmation from issue reporter.

@michaelarnauts
Copy link
Contributor

It actually still freezes for a second after playing a segment even with this change. When I use self.minBufferTime.total_seconds() * 2 instead it works fine.

I've tried increasing the DEFAULT_MINBUFFERTIME one by one, and when I pick 9, it plays smooth, so maybe a good value is to use the minBufferSize * 2?

I wonder if the suggestedPresentationDelay should actually be twice the segment size as suggested here: https://docs.unified-streaming.com/documentation/live/configure-dynamic-mpd.html#mpd-suggestedpresentationdelay

@bastimeyer
Copy link
Member Author

The manifest you've posted has segment durations of about 4 seconds (both video and audio adaptation sets), while it sets the minBufferTime to 5 seconds. With this PR, the suggestedPresentationDelay must be at least 5 seconds now instead of the previously hardcoded value of 3 (still not configurable), which means that the live-edge will be at least two segments now, depending on the moment when you start the stream. So after starting the stream, the DASHStreamWorker can queue up the following segments after the starting segments, meaning you have an initial time window of 8 seconds for it to queue and download new segments. Whether it buffers in your player depends on how fast the download is and how fast FFmpeg can mux the individual streams.

Without a config option, I don't want to set a static multiplication factor for increasing the suggestedPresentationDelay. This would break streams with lower latency, just for the almost negligible issue of buffering at the start.

The link you've posted explains the suggestedPresentationDelay wrong. This delay does not protect against 404 segment requests (when they were made too early). This is what the timeline does by explicitly and exactly defining the access times, in addition to the offset values. The presentation delay is merely used for client buffering. But as said, even though there are or might be some issues left with initial buffering, I don't want to change or hardcode any values here. This can be resolved by adding a live-edge value eventually, similar to HLS streams.

Just to be sure, are actually on this branch (version must be streamlink 6.2.1+28.g495da28d)?

@michaelarnauts
Copy link
Contributor

I understand your hesitance to just add a * 2 multiplier. I'm just curious where this freeze is coming from. I'm wondering if the sleeper is waiting to long to fetch a new segment?

When playing, I don't see any 404's or something, it just seems that streamlink is waiting to long to fetch a new segment?

[cli][debug] Pre-buffering 8192 bytes
[stream.dash][debug] video/mp4 segment 899: downloading (2023-10-13T18:57:16.000000Z / 2023-10-13T18:57:22.111918Z)
[stream.dash][debug] video/mp4 segment initialization: completed
[stream.dash][debug] audio/mp4 segment 899: downloading (2023-10-13T18:57:15.989333Z / 2023-10-13T18:57:22.116368Z)
[stream.dash][debug] audio/mp4 segment initialization: completed
[stream.dash][debug] audio/mp4 segment 900: downloading (2023-10-13T18:57:20.000000Z / 2023-10-13T18:57:22.203791Z)
[stream.dash][debug] audio/mp4 segment 899: completed
[stream.dash][debug] audio/mp4 segment 900: completed
[stream.dash][debug] video/mp4 segment 900: downloading (2023-10-13T18:57:20.000000Z / 2023-10-13T18:57:23.050972Z)
[stream.dash][debug] video/mp4 segment 899: completed
[cli][debug] Writing stream to output
[stream.dash][debug] video/mp4 segment 900: completed
[stream.dash.manifest][debug] Generating segment timeline for dynamic playlist: ('p0', None, 'V0')
[stream.dash][debug] Reloading manifest ('p0', None, 'V0')
[stream.dash.manifest][debug] Generating segment timeline for dynamic playlist: ('p0', None, 'A0')
[stream.dash][debug] Reloading manifest ('p0', None, 'A0')

# stream freezes

[stream.dash.manifest][debug] Generating segment timeline for dynamic playlist: ('p0', None, 'V0')
[stream.dash][debug] Reloading manifest ('p0', None, 'V0')
[stream.dash][debug] video/mp4 segment 899: downloading (2023-10-13T18:57:26.000000Z / 2023-10-13T18:57:31.516089Z)
[stream.dash.manifest][debug] Generating segment timeline for dynamic playlist: ('p0', None, 'A0')
[stream.dash][debug] Reloading manifest ('p0', None, 'A0')
[stream.dash][debug] audio/mp4 segment 899: downloading (2023-10-13T18:57:26.000000Z / 2023-10-13T18:57:31.522496Z)
[stream.dash][debug] audio/mp4 segment 899: completed
[stream.dash][debug] video/mp4 segment 899: completed

# stream continues

^C[stream.ffmpegmux][debug] Closing ffmpeg thread
[stream.segmented][debug] Closing worker thread
[stream.segmented][debug] Closing writer thread
[stream.segmented][debug] Closing worker thread
[stream.ffmpegmux][debug] Pipe copy complete: /tmp/streamlinkpipe-36766-1-5702
[stream.segmented][debug] Closing writer thread
[stream.ffmpegmux][debug] Pipe copy complete: /tmp/streamlinkpipe-36766-2-5931
[stream.ffmpegmux][debug] Closed all the substreams
[cli][info] Stream ended
Interrupted! Exiting...
[cli][info] Closing currently open stream...

I'm really sure that I'm on the right branch. I'm building with python3 setup.py bdist_wheel and installing with pip install --fore-reinstall dist/streamlink-6.2.1+28.g495da28d.dirty-py3-none-any.whl. streamlink --version indicates streamlink 6.2.1+28.g495da28d.dirty. (dirty because I've modified the * 2).

@bastimeyer
Copy link
Member Author

streamlink is waiting too long to fetch a new segment

Yes, the manifest reload logic isn't ideal. It's basically still the same as the initial implementation from 2018 and way worse than the one of the HLS implementation. It requires a rewrite, so that the reload time can be calculated better.

Your manifest doesn't set a minimumUpdatePeriod, so the manifest reload time in the DASHStreamWorker falls back to the period duration and then back to a static value of 5 seconds. On top of that, if no new segments got queued, the reload wait time increases with a back-off factor of 1.3, which doesn't make any sense. It should lower the time of the next reload instead and it should also take the last segment length into consideration. It also reloads the manifest for each substream, meaning for both the video and audio streams, which is pretty bad.

I'm going to merge this PR now, since the changes here are fine. If you want, you can open a new issue for the reload logic of dynamic DASH manifests.

@bastimeyer
Copy link
Member Author

Btw, you don't need to build a wheel every time. Just install in editable mode after cloning the git repo.
https://streamlink.github.io/developing.html

@bastimeyer bastimeyer merged commit a23bb61 into streamlink:master Oct 14, 2023
@bastimeyer bastimeyer deleted the stream/dash/suggestedPresentationDelay-minBufferTime branch October 14, 2023 12:55
epaz added a commit to epaz/streamlink_drm that referenced this pull request Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stream.dash: default suggestedPresentationDelay shouldn't be below minBufferTime
3 participants