Skip to content

Commit

Permalink
[external/FFmpeg] Fix and improve --ffmpeg-location handling
Browse files Browse the repository at this point in the history 
* 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 #32735
  • Loading branch information
@dirkf
dirkf committed Mar 27, 2024
1 parent d8f134a commit 21792b8
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 32 deletions.
16 changes: 15 additions & 1 deletion test/test_downloader_external.py
View file
Expand Up @@ -18,6 +18,7 @@
)
from youtube_dl import YoutubeDL
from youtube_dl.compat import (
compat_contextlib_suppress,
compat_http_cookiejar_Cookie,
compat_http_server,
compat_kwargs,
Expand All @@ -35,6 +36,9 @@
HttpieFD,
WgetFD,
)
from youtube_dl.postprocessor import (
FFmpegPostProcessor,
)
import threading

TEST_SIZE = 10 * 1024
Expand Down Expand Up @@ -227,7 +231,17 @@ def test_make_cmd(self):
self.assertIn('--load-cookies=%s' % downloader._cookies_tempfile, cmd)


@ifExternalFDAvailable(FFmpegFD)
# Handle delegated availability
def ifFFmpegFDAvailable(externalFD):
# raise SkipTest, or set False!
avail = ifExternalFDAvailable(externalFD) and False
with compat_contextlib_suppress(Exception):
avail = FFmpegPostProcessor(downloader=None).available
return unittest.skipUnless(
avail, externalFD.get_basename() + ' not found')


@ifFFmpegFDAvailable(FFmpegFD)
class TestFFmpegFD(unittest.TestCase):
_args = []

Expand Down
17 changes: 12 additions & 5 deletions youtube_dl/downloader/external.py
View file
Expand Up @@ -13,7 +13,12 @@
compat_str,
compat_subprocess_Popen,
)
from ..postprocessor.ffmpeg import FFmpegPostProcessor, EXT_TO_OUT_FORMATS

try:
from ..postprocessor.ffmpeg import FFmpegPostProcessor, EXT_TO_OUT_FORMATS
except ImportError:
FFmpegPostProcessor = None

from ..utils import (
cli_option,
cli_valueless_option,
Expand Down Expand Up @@ -362,13 +367,14 @@ def supports(cls, info_dict):

@classmethod
def available(cls):
return FFmpegPostProcessor().available
# actual availability can only be confirmed for an instance
return bool(FFmpegPostProcessor)

def _call_downloader(self, tmpfilename, info_dict):
url = info_dict['url']
ffpp = FFmpegPostProcessor(downloader=self)
# `downloader` means the parent `YoutubeDL`
ffpp = FFmpegPostProcessor(downloader=self.ydl)
if not ffpp.available:
self.report_error('m3u8 download detected but ffmpeg or avconv could not be found. Please install one.')
self.report_error('ffmpeg required for download but no ffmpeg (nor avconv) executable could be found. Please install one.')
return False
ffpp.check_version()

Expand Down Expand Up @@ -397,6 +403,7 @@ def _call_downloader(self, tmpfilename, info_dict):
# if end_time:
# args += ['-t', compat_str(end_time - start_time)]

url = info_dict['url']
cookies = self.ydl.cookiejar.get_cookies_for_url(url)
if cookies:
args.extend(['-cookies', ''.join(
Expand Down
38 changes: 12 additions & 26 deletions youtube_dl/postprocessor/ffmpeg.py
View file
Expand Up @@ -96,6 +96,7 @@ def get_ffmpeg_version(path):

self._paths = None
self._versions = None
location = None
if self._downloader:
prefer_ffmpeg = self._downloader.params.get('prefer_ffmpeg', True)
location = self._downloader.params.get('ffmpeg_location')
Expand All @@ -118,32 +119,17 @@ def get_ffmpeg_version(path):
location = os.path.dirname(os.path.abspath(location))
if basename in ('ffmpeg', 'ffprobe'):
prefer_ffmpeg = True

self._paths = dict(
(p, os.path.join(location, p)) for p in programs)
self._versions = dict(
(p, get_ffmpeg_version(self._paths[p])) for p in programs)
if self._versions is None:
self._versions = dict(
(p, get_ffmpeg_version(p)) for p in programs)
self._paths = dict((p, p) for p in programs)

if prefer_ffmpeg is False:
prefs = ('avconv', 'ffmpeg')
else:
prefs = ('ffmpeg', 'avconv')
for p in prefs:
if self._versions[p]:
self.basename = p
break

if prefer_ffmpeg is False:
prefs = ('avprobe', 'ffprobe')
else:
prefs = ('ffprobe', 'avprobe')
for p in prefs:
if self._versions[p]:
self.probe_basename = p
self._paths = dict(
(p, p if location is None else os.path.join(location, p))
for p in programs)
self._versions = dict(
x for x in (
(p, get_ffmpeg_version(self._paths[p])) for p in programs)
if x[1] is not None)

for p in ('ffmpeg', 'avconv')[::-1 if prefer_ffmpeg is False else 1]:
if self._versions.get(p):
self.basename = self.probe_basename = p
break

@property
Expand Down

3 comments on commit 21792b8

@vonProteus
Copy link

@vonProteus vonProteus commented on 21792b8 Apr 7, 2024 •
edited

Choose a reason for hiding this comment

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

i have problem when doing this

youtube-dl https://www.youtube.com/c/majorkill18  --download-archive /dev/null --yes-playlist --cookies /data/youtube-cookie-file.txt --write-info-json -x --audio-format mp3 --id  --verbose --max-downloads 1
[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['https://www.youtube.com/c/majorkill18', '--download-archive', '/dev/null', '--yes-playlist', '--cookies', '/data/youtube-cookie-file.txt', '--write-info-json', '-x', '--audio-format', 'mp3', '--id', '--verbose', '--max-downloads', '1']
[debug] Encodings: locale UTF-8, fs utf-8, out utf-8, pref UTF-8
[debug] youtube-dl version 2021.12.17
[debug] Single file build
[debug] Python 3.11.8 (CPython aarch64 64bit) - Linux-6.6.16-linuxkit-aarch64-with - OpenSSL 3.1.4 24 Oct 2023
[debug] exe versions: ffmpeg 6.1.1, ffprobe 6.1.1, rtmpdump 2.4
[debug] Proxy map: {}
[youtube:tab] majorkill18: Downloading webpage
[download] Downloading playlist: Majorkill - Home
[youtube:tab] playlist Majorkill - Home: Downloading 4 videos
[download] Downloading video 1 of 4
[youtube:tab] @majorkill: Downloading webpage
[download] Downloading playlist: Majorkill - Videos
[youtube:tab] Downloading page 1
[youtube:tab] Downloading page 2
…
[youtube:tab] Downloading page 22
[youtube:tab] Downloading page 23
[youtube:tab] playlist Majorkill - Videos: Downloading 709 videos
[download] Downloading video 1 of 709
[youtube] Y8ucV-1AHYw: Downloading webpage
[debug] [youtube] Decrypted nsig wKJCARKt3gWH9d5FAF => oNPa0zBxJrYo8A
[debug] [youtube] Decrypted nsig mSz-1HPTTNK5EloLzN => XsKBQr4cPqgjrw
[info] Writing video description metadata as JSON to: Y8ucV-1AHYw.info.json
[debug] Invoking downloader on '###'
[download] Y8ucV-1AHYw.webm has already been downloaded
[download] 100% of 10.08MiB
[debug] ffmpeg command line: ffmpeg -show_streams file:Y8ucV-1AHYw.webm
ERROR: WARNING: unable to obtain file audio codec with ffprobe
Traceback (most recent call last):
  File "/usr/local/bin/youtube-dl/youtube_dl/YoutubeDL.py", line 2288, in post_process
    files_to_delete, info = pp.run(info)
                            ^^^^^^^^^^^^
  File "/usr/local/bin/youtube-dl/youtube_dl/postprocessor/ffmpeg.py", line 264, in run
    raise PostProcessingError('WARNING: unable to obtain file audio codec with ffprobe')
youtube_dl.utils.PostProcessingError: WARNING: unable to obtain file audio codec with ffprobe

on previous commit its works
can you please fix it? @dirkf

@dirkf
Copy link
Contributor Author

@dirkf dirkf commented on 21792b8 Apr 7, 2024

Choose a reason for hiding this comment

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

The problem, which I can reproduce, is that ffmpeg is being run instead of ffprobe. As a work-around, you could wrap ffmpeg in a script that detects the -show_streams option and calls ffprobe instead. But I expect the problem will be fixed today.

@dirkf
Copy link
Contributor Author

@dirkf dirkf commented on 21792b8 Apr 7, 2024

Choose a reason for hiding this comment

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

By pushing a patch like this:

--- old/youtube-dl/youtube_dl/postprocessor/ffmpeg.py
+++ new/youtube-dl/youtube_dl/postprocessor/ffmpeg.py
@@ -74,8 +74,11 @@
         return FFmpegPostProcessor(downloader)._versions
 
     def _determine_executables(self):
-        programs = ['avprobe', 'avconv', 'ffmpeg', 'ffprobe']
+        # ordered to match prefer_ffmpeg!
+        convs = ['ffmpeg', 'avconv']
+        probes = ['ffprobe', 'avprobe']
         prefer_ffmpeg = True
+        programs = convs + probes
 
         def get_ffmpeg_version(path):
             ver = get_exe_version(path, args=['-version'])
@@ -127,10 +130,13 @@
                 (p, get_ffmpeg_version(self._paths[p])) for p in programs)
             if x[1] is not None)
 
-        for p in ('ffmpeg', 'avconv')[::-1 if prefer_ffmpeg is False else 1]:
-            if self._versions.get(p):
-                self.basename = self.probe_basename = p
-                break
+        basenames = [None, None]
+        for i, progs in enumerate((convs, probes)):
+            for p in progs[::-1 if prefer_ffmpeg is False else 1]:
+                if self._versions.get(p):
+                    basenames[i] = p
+                    break
+        self.basename, self.probe_basename = basenames
 
     @property
     def available(self):

Please sign in to comment.