Skip to content

utils.processoutput: reorder asyncio tasks #5480

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 6, 2023
Merged

utils.processoutput: reorder asyncio tasks #5480

merged 1 commit into from
Aug 6, 2023

Conversation

bastimeyer
Copy link
Member

Try to drain the stdout/stderr streams first before handling the onexit task, so the overall result can be processed correctly

Fixes #5461

The issue is caused by process.wait() resolving first in the onexit task, so the result of the done future gets set without the async interator(s) of the stdout/stderr stream(s) being drained in the onoutput task(s) first.

In my eyes, this looks like an issue in asyncio's subprocess implementation... No idea if this actually fixes #5461, but reordering the tasks at least made the added test pass while not reordering them made it fail.

@bastimeyer
utils.processoutput: reorder asyncio tasks
01bcca8 
Try to drain the stdout/stderr streams first before handling
the onexit task, so the overall result can be processed correctly
@bastimeyer bastimeyer mentioned this pull request Aug 4, 2023
Closed
4 tasks
gravyboat
gravyboat approved these changes Aug 5, 2023
View reviewed changes
Copy link
Member

@gravyboat gravyboat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I will mark this as approved since the solution seems to make sense to me based on the tests but let's see if the issue reporter can confirm this resolves their problem.

@bastimeyer bastimeyer merged commit a1e2886 into streamlink:master Aug 6, 2023
@bastimeyer bastimeyer deleted the utils/processoutput/exit-before-output branch August 6, 2023 18:00
@WuvorWenda
Copy link

WuvorWenda commented Sep 2, 2023 •
edited
Loading

I will mark this as approved since the solution seems to make sense to me based on the tests but let's see if the issue reporter can confirm this resolves their problem.

I can confirm it works now:
streamlink -l debug -o test "https://XXXX"
[cli][info] streamlink is running as root! Be careful!
[cli][debug] OS: Linux-3.10.0-1062.4.1.el7.x86_64-x86_64-with-glibc2.17
[cli][debug] Python: 3.11.5
[cli][debug] Streamlink: 6.1.0
[cli][debug] Dependencies:
[cli][debug] certifi: 2023.7.22
[cli][debug] isodate: 0.6.1
[cli][debug] lxml: 4.9.2
[cli][debug] pycountry: 22.3.5
[cli][debug] pycryptodome: 3.18.0
[cli][debug] PySocks: 1.7.1
[cli][debug] requests: 2.31.0
[cli][debug] trio: 0.22.2
[cli][debug] trio-websocket: 0.10.3
[cli][debug] typing-extensions: 4.7.1
[cli][debug] urllib3: 2.0.4
[cli][debug] websocket-client: 1.6.1
[cli][debug] Arguments:
[cli][debug] url=https://XXXX
[cli][debug] --loglevel=debug
[cli][debug] --player=mpv
[cli][debug] --player-no-close=True
[cli][debug] --output=test
[cli][debug] --default-stream=['best']
[cli][debug] --ringbuffer-size=33554432
[cli][debug] --stream-segment-attempts=4
[cli][debug] --stream-segment-timeout=30.0
[cli][debug] --stream-timeout=60.0
[cli][debug] --hls-playlist-reload-attempts=4
[cli][debug] --hls-playlist-reload-time=segment
[cli][debug] --hls-live-restart=True
[cli][debug] --ffmpeg-ffmpeg=ffmpeg
[cli][debug] --http-header=[('User-Agent', '"Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.100 Safari/537.36 OPR/62.0.3331.66"'), ('User-Agent', 'Mozilla/5.0 (Windows NT 10.0; rv:114.0) Gecko/20100101 Firefox/114.0')]
[cli][debug] --twitch-disable-ads=True
[cli][debug] --twitch-disable-hosting=True
[cli][info] Found matching plugin hls for URL XXXX
[plugins.hls][debug] URL=https://XXXXX; params={}
[utils.l10n][debug] Language code: en_US
[stream.ffmpegmux][debug] ffmpeg version 5.1.1-static https://johnvansickle.com/ffmpeg/ Copyright (c) 2000-2022 the FFmpeg developers
[stream.ffmpegmux][debug] built with gcc 8 (Debian 8.3.0-6)
[stream.ffmpegmux][debug] configuration: --enable-gpl --enable-version3 --enable-static --disable-debug --disable-ffplay --disable-indev=sndio --disable-outdev=sndio --cc=gcc --enable-fontconfig --enable-frei0r --enable-gnutls --enable-gmp --enable-libgme --enable-gray --enable-libaom --enable-libfribidi --enable-libass --enable-libvmaf --enable-libfreetype --enable-libmp3lame --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-librubberband --enable-libsoxr --enable-libspeex --enable-libsrt --enable-libvorbis --enable-libopus --enable-libtheora --enable-libvidstab --enable-libvo-amrwbenc --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxml2 --enable-libdav1d --enable-libxvid --enable-libzvbi --enable-libzimg
[stream.ffmpegmux][debug] libavutil 57. 28.100 / 57. 28.100
[stream.ffmpegmux][debug] libavcodec 59. 37.100 / 59. 37.100
[stream.ffmpegmux][debug] libavformat 59. 27.100 / 59. 27.100
[stream.ffmpegmux][debug] libavdevice 59. 7.100 / 59. 7.100
[stream.ffmpegmux][debug] libavfilter 8. 44.100 / 8. 44.100
[stream.ffmpegmux][debug] libswscale 6. 7.100 / 6. 7.100
[stream.ffmpegmux][debug] libswresample 4. 7.100 / 4. 7.100
[stream.ffmpegmux][debug] libpostproc 56. 6.100 / 56. 6.100
[stream.hls][debug] Using external audio tracks for stream 720p (language=None, name=audio)
[cli][info] Available streams: 720p (worst, best)
[cli][info] Opening stream: 720p (hls-multi)
[cli][info] Writing output to
/root/test
[cli][debug] Checking file output
File test already exists! Overwrite it? [y/N] y
[stream.ffmpegmux][debug] Opening hls substream
[stream.hls][debug] Reloading playlist
[stream.ffmpegmux][debug] Opening hls substream
[stream.hls][debug] Reloading playlist
[utils.named_pipe][info] Creating pipe streamlinkpipe-30279-1-419
[utils.named_pipe][info] Creating pipe streamlinkpipe-30279-2-2648
[stream.ffmpegmux][debug] ffmpeg command: /usr/bin/ffmpeg -nostats -y -i /tmp/streamlinkpipe-30279-1-419 -i /tmp/streamlinkpipe-30279-2-2648 -c:v copy -c:a copy -map 0:v? -map 0:a? -map 1:a -f mpegts pipe:1
[stream.ffmpegmux][debug] Starting copy to pipe: /tmp/streamlinkpipe-30279-1-419
[stream.ffmpegmux][debug] Starting copy to pipe: /tmp/streamlinkpipe-30279-2-2648
[cli][debug] Pre-buffering 8192 bytes
[stream.hls][debug] First Sequence: 6361; Last Sequence: 6364

--ffmpeg-no-validation is not needed anymore.
Thanks for solving this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gravyboat gravyboat gravyboat approved these changes

Assignees
No one assigned
Labels
bug stream: ffmpegmux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unexpected FFmpeg version output while running ['/usr/bin/ffmpeg', '-version']
3 participants
@bastimeyer @WuvorWenda @gravyboat