stream.dash: remove time.sleep() calls #4630
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Move SegmentedStreamWorker.wait to AwaitableMixin
- Make both SegmentedStream{Worker,Writer} inherit from AwaitableMixin
- Refactor DASHStreamWorker.iter_segments() - Replace dash_manifest.sleeper() with SegmentedStreamWorker.wait(), and make the worker thread shut down immediately on close(). - Prevent unnecessary wait times for static manifest types by calling close() after all segments were put into the writer's queue.
- Replace dash_manifest.sleep_until() with SegmentedStreamWriter.wait(), and make the thread-pool threads shut down immediately on close(). - Fix segment name logging
Billy2011
added a commit
to Billy2011/streamlink-27
that referenced
this pull request
Jul 10, 2022
- Move SegmentedStreamWorker.wait to AwaitableMixin
- Make both SegmentedStream{Worker,Writer} inherit from AwaitableMixin
Billy2011
added a commit
to Billy2011/streamlink-27
that referenced
this pull request
Jul 10, 2022
…#4630) - Refactor DASHStreamWorker.iter_segments() - Replace dash_manifest.sleeper() with SegmentedStreamWorker.wait(), and make the worker thread shut down immediately on close(). - Prevent unnecessary wait times for static manifest types by calling close() after all segments were put into the writer's queue.
Billy2011
added a commit
to Billy2011/streamlink-27
that referenced
this pull request
Jul 10, 2022
- Replace dash_manifest.sleep_until() with SegmentedStreamWriter.wait(), and make the thread-pool threads shut down immediately on close(). - Fix segment name logging
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #4311
This replaces the
time.sleep()calls made by theDASHStreamWorkerthread and threads of theDASHStreamWriterthread pool withthreading.Event.wait()calls, so that the execution of those threads can be stopped immediately when the stream gets closed.SegmentedStream{Worker,Writer}and add a commonwait()method via theAwaitableMixin.Not sure if this is a fitting name. Any better suggestions?
DASHStreamWorkertests. Not strictly necessary, but one test needed to be parametrized, because it was testing two things without resetting its state. Parametrizing while also patching via unittest.mock doesn't work via decorators, so the whole test needed to be rewritten either way.time.sleep()fromDASHStreamWorker.iter_segments()and make thread stop immediately when callingclose()or after putting segments into the writer's queue if the manifest type is "static".time.sleep()fromDASHStreamWriter.fetch()and make threads of the thread pool stop immediately when callingclose().Notes
Any additional closing delay needs to be fixed in the
FFMPEGMuxer(see Delay in closing of Muxed Streams #3137).For example,
FFMPEGMuxer.close()callsclose()on the various substreams sequentially, which means that if the streams wait until all of their threads have terminated (see stream.segmented: join worker+writer on close #4517), this introduces an additional delay.DASH live streams for testing can be found on https://steamcommunity.com/?subsection=broadcasts