Skip to content
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

[external/FFmpegFD] Fix ffmpeg detection with --ffmpeg-location ... (etc) #32741

Merged
merged 5 commits into from Mar 27, 2024

Conversation

dirkf
Copy link
Contributor

@dirkf dirkf commented Mar 11, 2024

Boilerplate: own code/bug fix

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

The ffmpeg downloader FFmpegFD depends on the corresponding post-processor FFmpegPostProcessor. The availability of a downloader is a class property, while the availability of a post-processor is an instance property. Due to a combination of code issues, a viable ffmpeg installation might not be found for downloading when a custom location was specified with --ffmpeg-location ... or its API equivalent, as in #32735.

This PR fixes the issue by splitting the availability computation:

  • the availability of the downloader as a class property is made to depend on the existence of the FFmpegPostProcessor class (normally guaranteed)
  • the actual availability at download time depends on the availability computed by the post-processor object created for the download.

Previously the first step would always fail with a custom ffmpeg location not on the PATH; even if it had succeeded, the instance-based availability check would fail because the associated post-processor was constructed using the object itself rather than its parent YoutubeDL.

Summary of changes:

  • a new compat feature compat_contextlib_suppress is added to allow the with suppress(exceptions) ... syntax to be used across Py2/3,. and applied in the utils module
  • also, compat_subprocess_Popen is added so that Popen() is a context manager across Py2/3
  • with these, a long-standing mystery "Resource Warning" diagnostic in the external downloader test is removed
  • ffmpeg detection by FFmpegPostProcessor is simplified
  • the availability logic in FFmpegFD is reworked as above with a more relevant error message.

with compat_contextlib_suppress(*Exceptions):
    # code that fails silently for any of Exceptions
* add compat_subprocess_Popen context manager
* apply context manager in FFmpegFD._call_downloader()
* pass YoutubeDL (FileDownloader) to FFmpegPostProcessor constructor
* consolidate path search in FFmpegPostProcessor
* make availability of FFmpegFD depend on existence of FFmpegPostProcessor
* detect ffmpeg executable on instantiation of FFmpegFD
* resolves ytdl-org#32735
@dirkf dirkf merged commit 21792b8 into ytdl-org:master Mar 27, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

External-downloader "ffmpeg" does not understand ffmpeg-location parameter
1 participant