From bb1978d976095cb1faf83b8c7074f7ede5ce0c4c Mon Sep 17 00:00:00 2001 From: Brannon King Date: Mon, 16 Mar 2020 16:02:54 -0600 Subject: [PATCH 1/5] ffmpeg now invoked via stream update --- lbry/extras/daemon/daemon.py | 12 +++++++++++- lbry/file_analysis.py | 3 +++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lbry/extras/daemon/daemon.py b/lbry/extras/daemon/daemon.py index 014a69194..1ffec6cf0 100644 --- a/lbry/extras/daemon/daemon.py +++ b/lbry/extras/daemon/daemon.py @@ -3174,12 +3174,13 @@ class Daemon(metaclass=JSONRPCServerType): self, claim_id, bid=None, file_path=None, channel_id=None, channel_name=None, channel_account_id=None, clear_channel=False, account_id=None, wallet_id=None, claim_address=None, funding_account_ids=None, - preview=False, blocking=False, replace=False, **kwargs): + preview=False, blocking=False, replace=False, validate_file=False, optimize_file=False, **kwargs): """ Update an existing stream claim and if a new file is provided announce it to lbrynet. Usage: stream_update ( | --claim_id=) [--bid=] [--file_path=] + [--validate_file] [--optimize_file] [--file_name=] [--file_size=] [--file_hash=] [--fee_currency=] [--fee_amount=] [--fee_address=] [--clear_fee] @@ -3199,6 +3200,11 @@ class Daemon(metaclass=JSONRPCServerType): --claim_id= : (str) id of the stream claim to update --bid= : (decimal) amount to back the claim --file_path= : (str) path to file to be associated with name. + --validate_file : (bool) validate that the video container and encodings match + common web browser support or that optimization succeeds if specified. + FFmpeg is required and file_path must be specified. + --optimize_file : (bool) transcode the video & audio if necessary to ensure common + web browser support. FFmpeg is required and file_path must be specified. --file_name= : (str) override file name, defaults to name from file_path. --file_size= : (str) override file size, otherwise automatically computed. --file_hash= : (str) override file hash, otherwise automatically computed. @@ -3327,6 +3333,10 @@ class Daemon(metaclass=JSONRPCServerType): if fee_address: kwargs['fee_address'] = fee_address + file_path = await self._video_file_analyzer.verify_or_repair( + validate_file, optimize_file, file_path, ignore_non_video=True + ) + if replace: claim = Claim() claim.stream.message.source.CopyFrom( diff --git a/lbry/file_analysis.py b/lbry/file_analysis.py index 550ffc6a1..bce7d7799 100644 --- a/lbry/file_analysis.py +++ b/lbry/file_analysis.py @@ -305,6 +305,9 @@ class VideoFileAnalyzer: if not validate and not repair: return file_path + if ignore_non_video and not file_path: + return file_path + await self._verify_ffmpeg_installed() try: scan_data = await self._get_scan_data(validate, file_path) From 5ab634e375ad64ce390d3a3009e0181ed2ec3c63 Mon Sep 17 00:00:00 2001 From: Brannon King Date: Mon, 16 Mar 2020 17:48:45 -0600 Subject: [PATCH 2/5] support search path for ffmpeg --- docs/api.json | 2 +- lbry/conf.py | 4 +- lbry/extras/daemon/daemon.py | 2 +- lbry/file_analysis.py | 84 +++++++++++++-------- tests/integration/other/test_transcoding.py | 5 +- 5 files changed, 60 insertions(+), 37 deletions(-) diff --git a/docs/api.json b/docs/api.json index 4cd926733..62d31d358 100644 --- a/docs/api.json +++ b/docs/api.json @@ -3077,7 +3077,7 @@ "curl": "curl -d'{\"method\": \"settings_get\", \"params\": {}}' http://localhost:5279/", "lbrynet": "lbrynet settings get", "python": "requests.post(\"http://localhost:5279\", json={\"method\": \"settings_get\", \"params\": {}}).json()", - "output": "{\n \"jsonrpc\": \"2.0\",\n \"result\": {\n \"announce_head_and_sd_only\": true,\n \"api\": \"localhost:5279\",\n \"audio_encoder\": \"aac -b:a 192k\",\n \"blob_download_timeout\": 30.0,\n \"blob_lru_cache_size\": 0,\n \"blockchain_name\": \"lbrycrd_regtest\",\n \"cache_time\": 150,\n \"coin_selection_strategy\": \"standard\",\n \"comment_server\": \"https://comments.lbry.com/api\",\n \"components_to_skip\": [\n \"dht\",\n \"upnp\",\n \"hash_announcer\",\n \"peer_protocol_server\"\n ],\n \"concurrent_blob_announcers\": 10,\n \"concurrent_reflector_uploads\": 10,\n \"config\": \"/home/lex/.local/share/lbry/lbrynet/daemon_settings.yml\",\n \"data_dir\": \"/tmp/tmpf0g7xmd6\",\n \"download_dir\": \"/tmp/tmpf0g7xmd6\",\n \"download_timeout\": 30.0,\n \"ffmpeg_folder\": \"\",\n \"fixed_peer_delay\": 2.0,\n \"known_dht_nodes\": [],\n \"lbryum_servers\": [\n [\n \"127.0.0.1\",\n 50001\n ]\n ],\n \"max_connections_per_download\": 4,\n \"max_key_fee\": {\n \"amount\": 50.0,\n \"currency\": \"USD\"\n },\n \"network_interface\": \"0.0.0.0\",\n \"node_rpc_timeout\": 5.0,\n \"peer_connect_timeout\": 3.0,\n \"prometheus_port\": 0,\n \"reflect_streams\": true,\n \"reflector_servers\": [\n [\n \"127.0.0.1\",\n 5566\n ]\n ],\n \"s3_headers_depth\": 960,\n \"save_blobs\": true,\n \"save_files\": true,\n \"share_usage_data\": false,\n \"split_buckets_under_index\": 1,\n \"streaming_get\": true,\n \"streaming_server\": \"localhost:5280\",\n \"tcp_port\": 3333,\n \"track_bandwidth\": true,\n \"udp_port\": 4444,\n \"use_upnp\": false,\n \"video_encoder\": \"libx264 -crf 18 -vf \\\"format=yuv420p\\\"\",\n \"volume_analysis_time\": \"240\",\n \"volume_filter\": \"-af loudnorm\",\n \"wallet_dir\": \"/tmp/tmpf0g7xmd6\",\n \"wallets\": [\n \"default_wallet\"\n ]\n }\n}" + "output": "{\n \"jsonrpc\": \"2.0\",\n \"result\": {\n \"announce_head_and_sd_only\": true,\n \"api\": \"localhost:5279\",\n \"audio_encoder\": \"aac -b:a 192k\",\n \"blob_download_timeout\": 30.0,\n \"blob_lru_cache_size\": 0,\n \"blockchain_name\": \"lbrycrd_regtest\",\n \"cache_time\": 150,\n \"coin_selection_strategy\": \"standard\",\n \"comment_server\": \"https://comments.lbry.com/api\",\n \"components_to_skip\": [\n \"dht\",\n \"upnp\",\n \"hash_announcer\",\n \"peer_protocol_server\"\n ],\n \"concurrent_blob_announcers\": 10,\n \"concurrent_reflector_uploads\": 10,\n \"config\": \"/home/lex/.local/share/lbry/lbrynet/daemon_settings.yml\",\n \"data_dir\": \"/tmp/tmpf0g7xmd6\",\n \"download_dir\": \"/tmp/tmpf0g7xmd6\",\n \"download_timeout\": 30.0,\n \"ffmpeg_path\": \"\",\n \"fixed_peer_delay\": 2.0,\n \"known_dht_nodes\": [],\n \"lbryum_servers\": [\n [\n \"127.0.0.1\",\n 50001\n ]\n ],\n \"max_connections_per_download\": 4,\n \"max_key_fee\": {\n \"amount\": 50.0,\n \"currency\": \"USD\"\n },\n \"network_interface\": \"0.0.0.0\",\n \"node_rpc_timeout\": 5.0,\n \"peer_connect_timeout\": 3.0,\n \"prometheus_port\": 0,\n \"reflect_streams\": true,\n \"reflector_servers\": [\n [\n \"127.0.0.1\",\n 5566\n ]\n ],\n \"s3_headers_depth\": 960,\n \"save_blobs\": true,\n \"save_files\": true,\n \"share_usage_data\": false,\n \"split_buckets_under_index\": 1,\n \"streaming_get\": true,\n \"streaming_server\": \"localhost:5280\",\n \"tcp_port\": 3333,\n \"track_bandwidth\": true,\n \"udp_port\": 4444,\n \"use_upnp\": false,\n \"video_encoder\": \"libx264 -crf 18 -vf \\\"format=yuv420p\\\"\",\n \"volume_analysis_time\": \"240\",\n \"volume_filter\": \"-af loudnorm\",\n \"wallet_dir\": \"/tmp/tmpf0g7xmd6\",\n \"wallets\": [\n \"default_wallet\"\n ]\n }\n}" } ] }, diff --git a/lbry/conf.py b/lbry/conf.py index 8b39570a1..f715d22b0 100644 --- a/lbry/conf.py +++ b/lbry/conf.py @@ -464,7 +464,9 @@ class BaseConfig: class TranscodeConfig(BaseConfig): - ffmpeg_folder = String('The path to ffmpeg and ffprobe', '') + ffmpeg_path = String('A list of places to check for ffmpeg and ffprobe. ' + f'$data_dir/ffmpeg/bin and $PATH are checked afterward. Separator: {os.pathsep}', + '', previous_names=['ffmpeg_folder']) video_encoder = String('FFmpeg codec and parameters for the video encoding. ' 'Example: libaom-av1 -crf 25 -b:v 0 -strict experimental', 'libx264 -crf 21 -preset faster -pix_fmt yuv420p') diff --git a/lbry/extras/daemon/daemon.py b/lbry/extras/daemon/daemon.py index 1ffec6cf0..9e0e3d658 100644 --- a/lbry/extras/daemon/daemon.py +++ b/lbry/extras/daemon/daemon.py @@ -786,7 +786,7 @@ class Daemon(metaclass=JSONRPCServerType): 'analyze_audio_volume': (bool) should ffmpeg analyze audio } """ - return await self._video_file_analyzer.status(recheck=True) + return await self._video_file_analyzer.status(reset=True) async def jsonrpc_status(self): """ diff --git a/lbry/file_analysis.py b/lbry/file_analysis.py index bce7d7799..73ebc0eca 100644 --- a/lbry/file_analysis.py +++ b/lbry/file_analysis.py @@ -26,9 +26,9 @@ class VideoFileAnalyzer: def __init__(self, conf: TranscodeConfig): self._conf = conf self._available_encoders = "" - self._ffmpeg_installed = False - self._which = None - self._checked_ffmpeg = False + self._ffmpeg_installed = None + self._whichFFmpeg = None + self._whichFFprobe = None self._env_copy = dict(os.environ) if lbry.utils.is_running_from_bundle(): # handle the situation where PyInstaller overrides our runtime environment: @@ -38,51 +38,69 @@ class VideoFileAnalyzer: if DISABLED: return "Disabled on Windows", -1 args = shlex.split(arguments) + process = await asyncio.create_subprocess_exec( - os.path.join(self._conf.ffmpeg_folder, command), *args, - stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, env=self._env_copy + command, *args, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, env=self._env_copy ) stdout, stderr = await process.communicate() # returns when the streams are closed return stdout.decode(errors='replace') + stderr.decode(errors='replace'), process.returncode + async def _execute_ffmpeg(self, arguments): + assert self._ffmpeg_installed + return await self._execute(self._whichFFmpeg, arguments) + + async def _execute_ffprobe(self, arguments): + assert self._ffmpeg_installed + return await self._execute(self._whichFFprobe, arguments) + async def _verify_executable(self, name): try: version, code = await self._execute(name, "-version") except Exception as e: code = -1 version = str(e) - if code != 0 or not version.startswith(name): + if code != 0 or not version.startswith(str(pathlib.Path(name).stem)): log.warning("Unable to run %s, but it was requested. Code: %d; Message: %s", name, code, version) raise FileNotFoundError(f"Unable to locate or run {name}. Please install FFmpeg " - f"and ensure that it is callable via PATH or conf.ffmpeg_folder") + f"and ensure that it is callable via PATH or conf.ffmpeg_path") return version async def _verify_ffmpeg_installed(self): if self._ffmpeg_installed: return - await self._verify_executable("ffprobe") - version = await self._verify_executable("ffmpeg") - self._which = shutil.which(os.path.join(self._conf.ffmpeg_folder, "ffmpeg")) - self._ffmpeg_installed = True - log.debug("Using %s at %s", version.splitlines()[0].split(" Copyright")[0], self._which) + self._ffmpeg_installed = False + path = self._conf.ffmpeg_path + if hasattr(self._conf, "data_dir"): + path += os.path.pathsep + os.path.join(getattr(self._conf, "data_dir"), "ffmpeg", "bin") + path += os.path.pathsep + self._env_copy.get("PATH", "") - async def status(self, reset=False, recheck=False): + self._whichFFmpeg = shutil.which("ffmpeg", path=path) + if not self._whichFFmpeg: + log.warning("Unable to locate ffmpeg executable. Path: %s", path) + raise FileNotFoundError(f"Unable to locate ffmpeg executable. Path: {path}") + self._whichFFprobe = shutil.which("ffprobe", path=path) + if not self._whichFFprobe: + log.warning("Unable to locate ffprobe executable. Path: %s", path) + raise FileNotFoundError(f"Unable to locate ffprobe executable. Path: {path}") + if os.path.dirname(self._whichFFmpeg) != os.path.dirname(self._whichFFprobe): + log.warning("ffmpeg and ffprobe are in different folders!") + await self._verify_executable(self._whichFFprobe) + version = await self._verify_executable(self._whichFFmpeg) + self._ffmpeg_installed = True + log.debug("Using %s at %s", version.splitlines()[0].split(" Copyright")[0], self._whichFFmpeg) + + async def status(self, reset=False): if reset: self._available_encoders = "" - self._ffmpeg_installed = False - self._which = None - if self._checked_ffmpeg and not recheck: - installed = self._ffmpeg_installed - else: - installed = True + self._ffmpeg_installed = None + if self._ffmpeg_installed is None: try: await self._verify_ffmpeg_installed() except FileNotFoundError: - installed = False - self._checked_ffmpeg = True + pass return { - "available": installed, - "which": self._which, + "available": self._ffmpeg_installed, + "which": self._whichFFmpeg, "analyze_audio_volume": int(self._conf.volume_analysis_time) > 0 } @@ -135,7 +153,7 @@ class VideoFileAnalyzer: if {"webm", "ogg"}.intersection(container.split(",")): return "" - result, _ = await self._execute("ffprobe", f'-v debug "{video_file}"') + result, _ = await self._execute_ffprobe(f'-v debug "{video_file}"') match = re.search(r"Before avformat_find_stream_info.+?\s+seeks:(\d+)\s+", result) if match and int(match.group(1)) != 0: return "Video stream descriptors are not at the start of the file (the faststart flag was not used)." @@ -163,8 +181,8 @@ class VideoFileAnalyzer: if not validate_volume: return "" - result, _ = await self._execute("ffmpeg", f'-i "{video_file}" -t {seconds} ' - f'-af volumedetect -vn -sn -dn -f null "{os.devnull}"') + result, _ = await self._execute_ffmpeg(f'-i "{video_file}" -t {seconds} ' + f'-af volumedetect -vn -sn -dn -f null "{os.devnull}"') try: mean_volume = float(re.search(r"mean_volume:\s+([-+]?\d*\.\d+|\d+)", result).group(1)) max_volume = float(re.search(r"max_volume:\s+([-+]?\d*\.\d+|\d+)", result).group(1)) @@ -199,7 +217,7 @@ class VideoFileAnalyzer: # if we don't have h264 use vp9; it's fairly compatible even though it's slow if not self._available_encoders: - self._available_encoders, _ = await self._execute("ffmpeg", "-encoders -v quiet") + self._available_encoders, _ = await self._execute_ffmpeg("-encoders -v quiet") encoder = self._conf.video_encoder.split(" ", 1)[0] if re.search(fr"^\s*V..... {encoder} ", self._available_encoders, re.MULTILINE): @@ -233,7 +251,7 @@ class VideoFileAnalyzer: wants_opus = extension != "mp4" if not self._available_encoders: - self._available_encoders, _ = await self._execute("ffmpeg", "-encoders -v quiet") + self._available_encoders, _ = await self._execute_ffmpeg("-encoders -v quiet") encoder = self._conf.audio_encoder.split(" ", 1)[0] if wants_opus and 'opus' in encoder: @@ -283,8 +301,8 @@ class VideoFileAnalyzer: return "mp4" async def _get_scan_data(self, validate, file_path): - result, _ = await self._execute("ffprobe", - f'-v quiet -print_format json -show_format -show_streams "{file_path}"') + arguments = f'-v quiet -print_format json -show_format -show_streams "{file_path}"' + result, _ = await self._execute_ffprobe(arguments) try: scan_data = json.loads(result) except Exception as e: @@ -293,7 +311,7 @@ class VideoFileAnalyzer: if "format" not in scan_data or "duration" not in scan_data["format"]: log.debug("Format data is missing from ffprobe results for: %s", file_path) - raise ValueError(f'Media file does not appear to contain video content at: {file_path}') + raise ValueError(f'Media file does not appear to contain video content: {file_path}') if float(scan_data["format"]["duration"]) < 0.1: log.debug("Media file appears to be an image: %s", file_path) @@ -352,7 +370,6 @@ class VideoFileAnalyzer: transcode_command.append("copy") transcode_command.append("-movflags +faststart -c:a") - path = pathlib.Path(file_path) extension = self._get_best_container_extension(scan_data, video_encoder) if audio_msg or volume_msg: @@ -365,12 +382,13 @@ class VideoFileAnalyzer: transcode_command.append("copy") # TODO: put it in a temp folder and delete it after we upload? + path = pathlib.Path(file_path) output = path.parent / f"{path.stem}_fixed.{extension}" transcode_command.append(f'"{output}"') ffmpeg_command = " ".join(transcode_command) log.info("Proceeding on transcode via: ffmpeg %s", ffmpeg_command) - result, code = await self._execute("ffmpeg", ffmpeg_command) + result, code = await self._execute_ffmpeg(ffmpeg_command) if code != 0: raise Exception(f"Failure to complete the transcode command. Output: {result}") except Exception as e: diff --git a/tests/integration/other/test_transcoding.py b/tests/integration/other/test_transcoding.py index 08ea9fe00..b08f65c04 100644 --- a/tests/integration/other/test_transcoding.py +++ b/tests/integration/other/test_transcoding.py @@ -129,6 +129,8 @@ class TranscodeValidation(ClaimTestCase): async def test_extension_choice(self): + self.assertTrue((await self.analyzer.status())["available"]) + scan_data = await self.analyzer._get_scan_data(True, self.video_file_name) extension = self.analyzer._get_best_container_extension(scan_data, "") self.assertEqual(extension, pathlib.Path(self.video_file_name).suffix[1:]) @@ -151,6 +153,7 @@ class TranscodeValidation(ClaimTestCase): self.assertEqual("ogv", extension) async def test_no_ffmpeg(self): - self.conf.ffmpeg_folder = "I don't really exist/" + self.conf.ffmpeg_path = "I don't really exist/" + self.analyzer._env_copy.pop("PATH", None) with self.assertRaisesRegex(Exception, "Unable to locate"): await self.analyzer.verify_or_repair(True, False, self.video_file_name) From ac89ba9b8d6e554a33648901c9a21e934e93af4d Mon Sep 17 00:00:00 2001 From: Brannon King Date: Tue, 17 Mar 2020 13:43:44 -0600 Subject: [PATCH 3/5] don't require ProactorEventLoop on Windows fix linter errors --- lbry/file_analysis.py | 63 ++++++++++++++++++++++++++---------------- scripts/check_video.py | 5 +--- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/lbry/file_analysis.py b/lbry/file_analysis.py index 73ebc0eca..fdea21c9d 100644 --- a/lbry/file_analysis.py +++ b/lbry/file_analysis.py @@ -6,13 +6,12 @@ import pathlib import re import shlex import shutil -import platform +import subprocess import lbry.utils from lbry.conf import TranscodeConfig log = logging.getLogger(__name__) -DISABLED = platform.system() == "Windows" class VideoFileAnalyzer: @@ -27,31 +26,47 @@ class VideoFileAnalyzer: self._conf = conf self._available_encoders = "" self._ffmpeg_installed = None - self._whichFFmpeg = None - self._whichFFprobe = None + self._which_ffmpeg = None + self._which_ffprobe = None self._env_copy = dict(os.environ) if lbry.utils.is_running_from_bundle(): # handle the situation where PyInstaller overrides our runtime environment: self._replace_or_pop_env('LD_LIBRARY_PATH') async def _execute(self, command, arguments): - if DISABLED: - return "Disabled on Windows", -1 args = shlex.split(arguments) - process = await asyncio.create_subprocess_exec( - command, *args, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, env=self._env_copy - ) - stdout, stderr = await process.communicate() # returns when the streams are closed - return stdout.decode(errors='replace') + stderr.decode(errors='replace'), process.returncode + # This create_subprocess_exec call is broken in Windows Python 3.7, but it's prettier than what's below. + # The recommended fix is switching to ProactorEventLoop, but that breaks UDP in Linux Python 3.7. + # Test it again in Python 3.8, both Linux and Windows. + # process = await asyncio.create_subprocess_exec( + # command, *args, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, env=self._env_copy + # ) + # stdout, stderr = await process.communicate() # returns when the streams are closed + # return stdout.decode(errors='replace') + stderr.decode(errors='replace'), process.returncode + + def execute_internal(): + args.insert(0, command) + try: + # if log.isEnabledFor(logging.DEBUG): + # log.debug("Executing: %s", " ".join(args)) + with subprocess.Popen( + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=self._env_copy + ) as process: + (stdout, stderr) = process.communicate() # blocks until the process exits + return stdout.decode(errors='replace') + stderr.decode(errors='replace'), process.returncode + except subprocess.SubprocessError as e: + return str(e), -1 + + return await asyncio.get_event_loop().run_in_executor(None, execute_internal) async def _execute_ffmpeg(self, arguments): assert self._ffmpeg_installed - return await self._execute(self._whichFFmpeg, arguments) + return await self._execute(self._which_ffmpeg, arguments) async def _execute_ffprobe(self, arguments): assert self._ffmpeg_installed - return await self._execute(self._whichFFprobe, arguments) + return await self._execute(self._which_ffprobe, arguments) async def _verify_executable(self, name): try: @@ -74,20 +89,20 @@ class VideoFileAnalyzer: path += os.path.pathsep + os.path.join(getattr(self._conf, "data_dir"), "ffmpeg", "bin") path += os.path.pathsep + self._env_copy.get("PATH", "") - self._whichFFmpeg = shutil.which("ffmpeg", path=path) - if not self._whichFFmpeg: + self._which_ffmpeg = shutil.which("ffmpeg", path=path) + if not self._which_ffmpeg: log.warning("Unable to locate ffmpeg executable. Path: %s", path) raise FileNotFoundError(f"Unable to locate ffmpeg executable. Path: {path}") - self._whichFFprobe = shutil.which("ffprobe", path=path) - if not self._whichFFprobe: + self._which_ffprobe = shutil.which("ffprobe", path=path) + if not self._which_ffprobe: log.warning("Unable to locate ffprobe executable. Path: %s", path) raise FileNotFoundError(f"Unable to locate ffprobe executable. Path: {path}") - if os.path.dirname(self._whichFFmpeg) != os.path.dirname(self._whichFFprobe): + if os.path.dirname(self._which_ffmpeg) != os.path.dirname(self._which_ffprobe): log.warning("ffmpeg and ffprobe are in different folders!") - await self._verify_executable(self._whichFFprobe) - version = await self._verify_executable(self._whichFFmpeg) + await self._verify_executable(self._which_ffprobe) + version = await self._verify_executable(self._which_ffmpeg) self._ffmpeg_installed = True - log.debug("Using %s at %s", version.splitlines()[0].split(" Copyright")[0], self._whichFFmpeg) + log.debug("Using %s at %s", version.splitlines()[0].split(" Copyright")[0], self._which_ffmpeg) async def status(self, reset=False): if reset: @@ -100,7 +115,7 @@ class VideoFileAnalyzer: pass return { "available": self._ffmpeg_installed, - "which": self._whichFFmpeg, + "which": self._which_ffmpeg, "analyze_audio_volume": int(self._conf.volume_analysis_time) > 0 } @@ -139,12 +154,12 @@ class VideoFileAnalyzer: bit_rate = float(scan_data["format"]["bit_rate"]) else: bit_rate = os.stat(file_path).st_size / float(scan_data["format"]["duration"]) - log.debug(" Detected bitrate is %s Mbps. Allowed is %s Mbps", + log.debug(" Detected bitrate is %s Mbps. Allowed max: %s Mbps", str(bit_rate / 1000000.0), str(bit_rate_max / 1000000.0)) if bit_rate > bit_rate_max: return "The bit rate is above the configured maximum. Actual: " \ - f"{bit_rate / 1000000.0} Mbps; Allowed: {bit_rate_max / 1000000.0} Mbps" + f"{bit_rate / 1000000.0} Mbps; Allowed max: {bit_rate_max / 1000000.0} Mbps" return "" diff --git a/scripts/check_video.py b/scripts/check_video.py index 61331142b..c95fbf776 100755 --- a/scripts/check_video.py +++ b/scripts/check_video.py @@ -17,7 +17,7 @@ def enable_logging(): handler = logging.StreamHandler(sys.stdout) handler.setLevel(logging.DEBUG) - formatter = logging.Formatter('%(message)s') # %(asctime)s - %(levelname)s - + formatter = logging.Formatter('%(message)s') handler.setFormatter(formatter) root.addHandler(handler) @@ -49,9 +49,6 @@ def main(): video_file = sys.argv[1] conf = TranscodeConfig() analyzer = VideoFileAnalyzer(conf) - if sys.version_info < (3, 8) and platform.system() == "Windows": - # TODO: remove after we move to requiring Python 3.8 - asyncio.set_event_loop(asyncio.ProactorEventLoop()) try: asyncio.run(process_video(analyzer, video_file)) except KeyboardInterrupt: From bd291109dfa290479f52009c4be31a2d22033b72 Mon Sep 17 00:00:00 2001 From: Brannon King Date: Wed, 18 Mar 2020 12:00:41 -0600 Subject: [PATCH 4/5] addressing code review comments --- lbry/file_analysis.py | 65 +++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 37 deletions(-) diff --git a/lbry/file_analysis.py b/lbry/file_analysis.py index fdea21c9d..a74b46766 100644 --- a/lbry/file_analysis.py +++ b/lbry/file_analysis.py @@ -33,51 +33,44 @@ class VideoFileAnalyzer: # handle the situation where PyInstaller overrides our runtime environment: self._replace_or_pop_env('LD_LIBRARY_PATH') - async def _execute(self, command, arguments): - args = shlex.split(arguments) + @staticmethod + def _execute(command, environment): + args = shlex.split(command) - # This create_subprocess_exec call is broken in Windows Python 3.7, but it's prettier than what's below. - # The recommended fix is switching to ProactorEventLoop, but that breaks UDP in Linux Python 3.7. - # Test it again in Python 3.8, both Linux and Windows. - # process = await asyncio.create_subprocess_exec( - # command, *args, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, env=self._env_copy - # ) - # stdout, stderr = await process.communicate() # returns when the streams are closed - # return stdout.decode(errors='replace') + stderr.decode(errors='replace'), process.returncode - - def execute_internal(): - args.insert(0, command) - try: - # if log.isEnabledFor(logging.DEBUG): - # log.debug("Executing: %s", " ".join(args)) - with subprocess.Popen( - args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=self._env_copy - ) as process: - (stdout, stderr) = process.communicate() # blocks until the process exits - return stdout.decode(errors='replace') + stderr.decode(errors='replace'), process.returncode - except subprocess.SubprocessError as e: - return str(e), -1 - - return await asyncio.get_event_loop().run_in_executor(None, execute_internal) + # if log.isEnabledFor(logging.DEBUG): + # log.debug("Executing: %s", " ".join(args)) + try: + with subprocess.Popen( + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=environment + ) as process: + (stdout, stderr) = process.communicate() # blocks until the process exits + return stdout.decode(errors='replace') + stderr.decode(errors='replace'), process.returncode + except subprocess.SubprocessError as e: + return str(e), -1 + # This create_subprocess_exec call is broken in Windows Python 3.7, but it's prettier than what's here. + # The recommended fix is switching to ProactorEventLoop, but that breaks UDP in Linux Python 3.7. + # We work around that issue here by using run_in_executor. Check it again in Python 3.8. async def _execute_ffmpeg(self, arguments): - assert self._ffmpeg_installed - return await self._execute(self._which_ffmpeg, arguments) + arguments = self._which_ffmpeg + " " + arguments + return await asyncio.get_event_loop().run_in_executor(None, self._execute, arguments, self._env_copy) async def _execute_ffprobe(self, arguments): - assert self._ffmpeg_installed - return await self._execute(self._which_ffprobe, arguments) + arguments = self._which_ffprobe + " " + arguments + return await asyncio.get_event_loop().run_in_executor(None, self._execute, arguments, self._env_copy) - async def _verify_executable(self, name): + async def _verify_executables(self): try: - version, code = await self._execute(name, "-version") + await self._execute_ffprobe("-version") + version, code = await self._execute_ffmpeg("-version") except Exception as e: code = -1 version = str(e) - if code != 0 or not version.startswith(str(pathlib.Path(name).stem)): - log.warning("Unable to run %s, but it was requested. Code: %d; Message: %s", name, code, version) - raise FileNotFoundError(f"Unable to locate or run {name}. Please install FFmpeg " + if code != 0 or not version.startswith("ffmpeg"): + log.warning("Unable to run ffmpeg, but it was requested. Code: %d; Message: %s", code, version) + raise FileNotFoundError(f"Unable to locate or run ffmpeg or ffprobe. Please install FFmpeg " f"and ensure that it is callable via PATH or conf.ffmpeg_path") + log.debug("Using %s at %s", version.splitlines()[0].split(" Copyright")[0], self._which_ffmpeg) return version async def _verify_ffmpeg_installed(self): @@ -99,10 +92,8 @@ class VideoFileAnalyzer: raise FileNotFoundError(f"Unable to locate ffprobe executable. Path: {path}") if os.path.dirname(self._which_ffmpeg) != os.path.dirname(self._which_ffprobe): log.warning("ffmpeg and ffprobe are in different folders!") - await self._verify_executable(self._which_ffprobe) - version = await self._verify_executable(self._which_ffmpeg) + await self._verify_executables() self._ffmpeg_installed = True - log.debug("Using %s at %s", version.splitlines()[0].split(" Copyright")[0], self._which_ffmpeg) async def status(self, reset=False): if reset: From bf11bcc0846a8db57949b745fd7ad1eae49ef93c Mon Sep 17 00:00:00 2001 From: Brannon King Date: Wed, 18 Mar 2020 12:43:27 -0600 Subject: [PATCH 5/5] fixed transcoding tests for non-async execute status ordering broke path check --- tests/integration/other/test_transcoding.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integration/other/test_transcoding.py b/tests/integration/other/test_transcoding.py index b08f65c04..9a80fc8da 100644 --- a/tests/integration/other/test_transcoding.py +++ b/tests/integration/other/test_transcoding.py @@ -32,13 +32,14 @@ class TranscodeValidation(ClaimTestCase): self.conf = TranscodeConfig() self.conf.volume_analysis_time = 0 # disable it as the test file isn't very good here self.analyzer = VideoFileAnalyzer(self.conf) + self.assertTrue((await self.analyzer.status())["available"]) # ensure ffmpeg path detected file_ogg = self.make_name("ogg", ".ogg") self.video_file_ogg = str(file_ogg) if not file_ogg.exists(): command = f'-i "{self.video_file_name}" -c:v libtheora -q:v 4 -c:a libvorbis -q:a 4 ' \ f'-c:s copy -c:d copy "{file_ogg}"' with MeasureTime(f"Creating {file_ogg.name}"): - output, code = await self.analyzer._execute("ffmpeg", command) + output, code = await self.analyzer._execute_ffmpeg(command) self.assertEqual(code, 0, output) file_webm = self.make_name("webm", ".webm") @@ -47,7 +48,7 @@ class TranscodeValidation(ClaimTestCase): command = f'-i "{self.video_file_name}" -c:v libvpx-vp9 -crf 36 -b:v 0 -cpu-used 2 ' \ f'-c:a libopus -b:a 128k -c:s copy -c:d copy "{file_webm}"' with MeasureTime(f"Creating {file_webm.name}"): - output, code = await self.analyzer._execute("ffmpeg", command) + output, code = await self.analyzer._execute_ffmpeg(command) self.assertEqual(code, 0, output) async def test_should_work(self): @@ -68,7 +69,7 @@ class TranscodeValidation(ClaimTestCase): if not file_name.exists(): command = f'-i "{self.video_file_name}" -c copy -map 0 "{file_name}"' with MeasureTime(f"Creating {file_name.name}"): - output, code = await self.analyzer._execute("ffmpeg", command) + output, code = await self.analyzer._execute_ffmpeg(command) self.assertEqual(code, 0, output) with self.assertRaisesRegex(Exception, "Container format is not in the approved list"): @@ -82,7 +83,7 @@ class TranscodeValidation(ClaimTestCase): if not file_name.exists(): command = f'-i "{self.video_file_name}" -c copy -map 0 -c:v libx265 -preset superfast "{file_name}"' with MeasureTime(f"Creating {file_name.name}"): - output, code = await self.analyzer._execute("ffmpeg", command) + output, code = await self.analyzer._execute_ffmpeg(command) self.assertEqual(code, 0, output) with self.assertRaisesRegex(Exception, "Video codec is not in the approved list"): @@ -104,7 +105,7 @@ class TranscodeValidation(ClaimTestCase): command = f'-i "{self.video_file_name}" -c copy -map 0 -c:v libx264 ' \ f'-vf format=yuv444p "{file_name}"' with MeasureTime(f"Creating {file_name.name}"): - output, code = await self.analyzer._execute("ffmpeg", command) + output, code = await self.analyzer._execute_ffmpeg(command) self.assertEqual(code, 0, output) with self.assertRaisesRegex(Exception, "pixel format does not match the approved"): @@ -118,7 +119,7 @@ class TranscodeValidation(ClaimTestCase): if not file_name.exists(): command = f'-i "{self.video_file_name}" -c copy -map 0 -c:a pcm_s16le "{file_name}"' with MeasureTime(f"Creating {file_name.name}"): - output, code = await self.analyzer._execute("ffmpeg", command) + output, code = await self.analyzer._execute_ffmpeg(command) self.assertEqual(code, 0, output) with self.assertRaisesRegex(Exception, "Audio codec is not in the approved list"): @@ -129,8 +130,6 @@ class TranscodeValidation(ClaimTestCase): async def test_extension_choice(self): - self.assertTrue((await self.analyzer.status())["available"]) - scan_data = await self.analyzer._get_scan_data(True, self.video_file_name) extension = self.analyzer._get_best_container_extension(scan_data, "") self.assertEqual(extension, pathlib.Path(self.video_file_name).suffix[1:]) @@ -155,5 +154,6 @@ class TranscodeValidation(ClaimTestCase): async def test_no_ffmpeg(self): self.conf.ffmpeg_path = "I don't really exist/" self.analyzer._env_copy.pop("PATH", None) + await self.analyzer.status(reset=True) with self.assertRaisesRegex(Exception, "Unable to locate"): await self.analyzer.verify_or_repair(True, False, self.video_file_name)