Skip to content

stream.ffmpegmux: cache executable lookups #4660

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

  • Remove redundant commands with .exe suffixes.
    shutil.which will pick those up via PATHEXT on Windows.
  • Move executable lookup to FFMPEGMuxer.resolve_command, so its inputs
    and return values can be cached using functools.lru_cache without
    relying on a Streamlink session instance as input
  • Keep interface of FFMPEGMuxer.is_usable and FFMPEGMuxer.command
  • Add proper tests

FFMPEGMuxer.is_usable currently doesn't get cached, which means that when generating a list of MuxedHLSStreams for example, ffmpeg will get looked up every single time, and additionally when a the stream gets opened.

I didn't remove avconv yet, even if it's totally useless nowadays (the libav fork of ffmpeg was abandoned in 2018). There might still be someone though using an old Debian or CentOS release which defaults to libav.

- Remove redundant commands with `.exe` suffixes.
  `shutil.which` will pick those up via `PATHEXT` on Windows.
- Move executable lookup to `FFMPEGMuxer.resolve_command`, so its inputs
  and return values can be cached using `functools.lru_cache` without
  relying on a Streamlink session instance as input
- Keep interface of `FFMPEGMuxer.is_usable` and `FFMPEGMuxer.command`
- Add proper tests
@bastimeyer bastimeyer force-pushed the stream/ffmpegmux/cache-executable-lookups branch from bc1d6e2 to 3193fb6 Compare July 17, 2022 19:49
@bastimeyer
Copy link
Member Author

I'll submit another PR which will update the other ffmpegmuxer tests once this one got merged, as it's unrelated to the changes here.

@bastimeyer
Copy link
Member Author

Any reviews? The changes are fairly simple.

@gravyboat
Copy link
Member

Sorry @bastimeyer. I actually looked at this 3 days ago but guess I closed the tab instead of merging.

@gravyboat gravyboat merged commit 199429f into streamlink:master Jul 20, 2022
@bastimeyer bastimeyer deleted the stream/ffmpegmux/cache-executable-lookups branch July 20, 2022 22:23
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Jul 23, 2022
- Remove redundant commands with `.exe` suffixes.
  `shutil.which` will pick those up via `PATHEXT` on Windows.
- Move executable lookup to `FFMPEGMuxer.resolve_command`, so its inputs
  and return values can be cached using `functools.lru_cache` without
  relying on a Streamlink session instance as input
- Keep interface of `FFMPEGMuxer.is_usable` and `FFMPEGMuxer.command`
- Add proper tests
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.

2 participants