-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
cli: rewrite progress output #4656
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
3a3e5da to
d8beab6
Compare
- Split `streamlink_cli.utils.progress` into `progress` + `terminal`
- `terminal`:
Responsible for calculating character output widths and for writing
data to the output text stream
- `progress`:
Responsible for calculating the download speed and for formatting
the progress output
- Use a separate thread for the progress output, with the output being
written in constant time intervals, independent of the data that gets
fed by the stream iterator of the main thread
- Output format changes:
- Remove "prefix" from output format (full path already gets logged)
- Avoid showing download speeds until a time threshold is reached
- Use binary values for file size and download speed values
- Add leading zeros to time units (only ever grow output text size)
- Don't import from alias module in `streamlink_cli.main`
- Split, move and rewrite tests
d8beab6 to
ed65152
Compare
|
Added the missing tests. I decided to not test the threading logic of the |
| interval: float = 0.25, | ||
| history: int = 20, | ||
| threshold: int = 2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These values could be tweaked.
Four updates per seconds seems reasonable, and so is keeping a download speed history (the result is the average in this time frame) of twenty seconds which ensures that segmented streams with large segment durations (but still less than 20 seconds) don't make the download speed go up and down in between segment downloads and segment queue wait times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see if anyone has feedback after the next release. If so they can be modified but these seem reasonable to me.
|
Looks great @bastimeyer! |
Original PR #4656 by bastimeyer Original: streamlink/streamlink#4656
Merged from original PR #4656 Original: streamlink/streamlink#4656
streamlink_cli.utils.progressintoprogress+terminalterminal:Responsible for calculating character output widths and for writing
data to the output text stream
progress:Responsible for calculating the download speed and for formatting
the progress output
written in constant time intervals, independent of the data that gets
fed by the stream iterator of the main thread
streamlink_cli.mainThe download progress on the master branch is currently bound to the main thread and data of the stream iterator in
read_stream(). As explained in #4642, this has several issues. Rewriting the progress output is one of the requirements for being able to resolve the linked issue.The interface of the
Progressclass is written with the currentstream_iteratorimplementation inread_stream()in mind, which will change according to my plans described in #4642, but that shouldn't be a big issue with how theput()method is implemented.