diff --git a/videoarchiver/ffmpeg/exceptions.py b/videoarchiver/ffmpeg/exceptions.py index a77fe06..551dac6 100644 --- a/videoarchiver/ffmpeg/exceptions.py +++ b/videoarchiver/ffmpeg/exceptions.py @@ -1,70 +1,100 @@ """FFmpeg-related exceptions""" + class FFmpegError(Exception): """Base exception for FFmpeg-related errors""" + pass + class DownloadError(FFmpegError): """Exception raised when FFmpeg download fails""" + pass + class VerificationError(FFmpegError): """Exception raised when FFmpeg verification fails""" + pass + class EncodingError(FFmpegError): """Exception raised when video encoding fails""" + pass + class AnalysisError(FFmpegError): """Exception raised when video analysis fails""" + pass + class GPUError(FFmpegError): """Exception raised when GPU operations fail""" + pass + class HardwareAccelerationError(FFmpegError): """Exception raised when hardware acceleration fails""" + def __init__(self, message: str, fallback_used: bool = False): self.fallback_used = fallback_used super().__init__(message) + class FFmpegNotFoundError(FFmpegError): """Exception raised when FFmpeg binary is not found""" + pass + class FFprobeError(FFmpegError): """Exception raised when FFprobe operations fail""" + pass + class CompressionError(FFmpegError): """Exception raised when video compression fails""" + def __init__(self, message: str, input_size: int, target_size: int): self.input_size = input_size self.target_size = target_size super().__init__(f"{message} (Input: {input_size}B, Target: {target_size}B)") + class FormatError(FFmpegError): """Exception raised when video format is invalid or unsupported""" + pass + class PermissionError(FFmpegError): """Exception raised when file permissions prevent operations""" + pass + class TimeoutError(FFmpegError): """Exception raised when FFmpeg operations timeout""" + pass + class ResourceError(FFmpegError): """Exception raised when system resources are insufficient""" + def __init__(self, message: str, resource_type: str): self.resource_type = resource_type super().__init__(f"{message} (Resource: {resource_type})") + class QualityError(FFmpegError): """Exception raised when video quality requirements cannot be met""" + def __init__(self, message: str, target_quality: int, achieved_quality: int): self.target_quality = target_quality self.achieved_quality = achieved_quality @@ -72,12 +102,16 @@ class QualityError(FFmpegError): f"{message} (Target: {target_quality}p, Achieved: {achieved_quality}p)" ) + class AudioError(FFmpegError): """Exception raised when audio processing fails""" + pass + class BitrateError(FFmpegError): """Exception raised when bitrate requirements cannot be met""" + def __init__(self, message: str, target_bitrate: int, actual_bitrate: int): self.target_bitrate = target_bitrate self.actual_bitrate = actual_bitrate @@ -85,16 +119,19 @@ class BitrateError(FFmpegError): f"{message} (Target: {target_bitrate}bps, Actual: {actual_bitrate}bps)" ) + def handle_ffmpeg_error(error_output: str) -> FFmpegError: """Convert FFmpeg error output to appropriate exception""" error_output = error_output.lower() - + if "no such file" in error_output: return FFmpegNotFoundError("FFmpeg binary not found") elif "permission denied" in error_output: return PermissionError("Insufficient permissions") elif "hardware acceleration" in error_output: - return HardwareAccelerationError("Hardware acceleration failed", fallback_used=True) + return HardwareAccelerationError( + "Hardware acceleration failed", fallback_used=True + ) elif "invalid data" in error_output: return FormatError("Invalid or corrupted video format") elif "insufficient memory" in error_output: diff --git a/videoarchiver/ffmpeg/ffmpeg_downloader.py b/videoarchiver/ffmpeg/ffmpeg_downloader.py index 82d9d04..4fceff8 100644 --- a/videoarchiver/ffmpeg/ffmpeg_downloader.py +++ b/videoarchiver/ffmpeg/ffmpeg_downloader.py @@ -19,6 +19,7 @@ from .exceptions import DownloadError logger = logging.getLogger("VideoArchiver") + @contextmanager def temp_path_context(): """Context manager for temporary path creation and cleanup""" @@ -32,6 +33,7 @@ def temp_path_context(): except Exception as e: logger.error(f"Error cleaning up temp directory {temp_dir}: {e}") + class FFmpegDownloader: FFMPEG_URLS = { "Windows": { @@ -42,15 +44,15 @@ class FFmpegDownloader: }, "Linux": { "x86_64": { - "url": "https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-amd64-static.tar.xz", + "url": "https://github.com/BtbN/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-linux64-gpl.tar.xz", "bin_names": ["ffmpeg", "ffprobe"], }, "aarch64": { - "url": "https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-arm64-static.tar.xz", + "url": "https://github.com/BtbN/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-linuxarm64-gpl.tar.xz", "bin_names": ["ffmpeg", "ffprobe"], }, "armv7l": { - "url": "https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-armhf-static.tar.xz", + "url": "https://github.com/BtbN/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-linuxarm32-gpl.tar.xz", "bin_names": ["ffmpeg", "ffprobe"], }, }, @@ -75,7 +77,7 @@ class FFmpegDownloader: self.base_dir = base_dir self.ffmpeg_path = self.base_dir / self._get_binary_names()[0] self.ffprobe_path = self.base_dir / self._get_binary_names()[1] - + logger.info(f"Initialized FFmpeg downloader for {system}/{machine}") logger.info(f"FFmpeg binary path: {self.ffmpeg_path}") logger.info(f"FFprobe binary path: {self.ffprobe_path}") @@ -85,14 +87,18 @@ class FFmpegDownloader: try: return self.FFMPEG_URLS[self.system][self.machine]["bin_names"] except KeyError: - raise DownloadError(f"Unsupported system/architecture: {self.system}/{self.machine}") + raise DownloadError( + f"Unsupported system/architecture: {self.system}/{self.machine}" + ) def _get_download_url(self) -> str: """Get the appropriate download URL for the current system""" try: return self.FFMPEG_URLS[self.system][self.machine]["url"] except KeyError: - raise DownloadError(f"Unsupported system/architecture: {self.system}/{self.machine}") + raise DownloadError( + f"Unsupported system/architecture: {self.system}/{self.machine}" + ) def download(self) -> Dict[str, Path]: """Download and set up FFmpeg and FFprobe binaries with retries""" @@ -103,7 +109,7 @@ class FFmpegDownloader: for attempt in range(max_retries): try: logger.info(f"Download attempt {attempt + 1}/{max_retries}") - + # Ensure base directory exists with proper permissions self.base_dir.mkdir(parents=True, exist_ok=True) os.chmod(str(self.base_dir), 0o777) @@ -119,28 +125,27 @@ class FFmpegDownloader: with temp_path_context() as temp_dir: # Download archive archive_path = self._download_archive(temp_dir) - + # Verify download if not self._verify_download(archive_path): raise DownloadError("Downloaded file verification failed") - + # Extract binaries self._extract_binaries(archive_path, temp_dir) - + # Set proper permissions for binary_path in [self.ffmpeg_path, self.ffprobe_path]: os.chmod(str(binary_path), 0o755) - + # Verify binaries if not self.verify(): raise DownloadError("Binary verification failed") - + logger.info(f"Successfully downloaded FFmpeg to {self.ffmpeg_path}") - logger.info(f"Successfully downloaded FFprobe to {self.ffprobe_path}") - return { - "ffmpeg": self.ffmpeg_path, - "ffprobe": self.ffprobe_path - } + logger.info( + f"Successfully downloaded FFprobe to {self.ffprobe_path}" + ) + return {"ffmpeg": self.ffmpeg_path, "ffprobe": self.ffprobe_path} except Exception as e: last_error = str(e) @@ -154,17 +159,20 @@ class FFmpegDownloader: def _download_archive(self, temp_dir: str) -> Path: """Download FFmpeg archive with progress tracking""" url = self._get_download_url() - archive_path = Path(temp_dir) / f"ffmpeg_archive{'.zip' if self.system == 'Windows' else '.tar.xz'}" - + archive_path = ( + Path(temp_dir) + / f"ffmpeg_archive{'.zip' if self.system == 'Windows' else '.tar.xz'}" + ) + logger.info(f"Downloading FFmpeg from {url}") try: response = requests.get(url, stream=True, timeout=30) response.raise_for_status() - - total_size = int(response.headers.get('content-length', 0)) + + total_size = int(response.headers.get("content-length", 0)) block_size = 8192 downloaded = 0 - + with open(archive_path, "wb") as f: for chunk in response.iter_content(chunk_size=block_size): f.write(chunk) @@ -172,9 +180,9 @@ class FFmpegDownloader: if total_size > 0: percent = (downloaded / total_size) * 100 logger.debug(f"Download progress: {percent:.1f}%") - + return archive_path - + except Exception as e: raise DownloadError(f"Failed to download FFmpeg: {str(e)}") @@ -183,20 +191,20 @@ class FFmpegDownloader: try: if not archive_path.exists(): return False - + # Check file size size = archive_path.stat().st_size if size < 1000000: # Less than 1MB is suspicious logger.error(f"Downloaded file too small: {size} bytes") return False - + # Check file hash - with open(archive_path, 'rb') as f: + with open(archive_path, "rb") as f: file_hash = hashlib.sha256(f.read()).hexdigest() logger.debug(f"Archive hash: {file_hash}") - + return True - + except Exception as e: logger.error(f"Download verification failed: {str(e)}") return False @@ -205,38 +213,60 @@ class FFmpegDownloader: """Extract FFmpeg and FFprobe binaries from archive""" logger.info("Extracting FFmpeg and FFprobe binaries") - if self.system == "Windows": - self._extract_zip(archive_path, temp_dir) - else: - self._extract_tar(archive_path, temp_dir) + try: + if self.system == "Windows": + self._extract_zip(archive_path, temp_dir) + else: + self._extract_tar(archive_path, temp_dir) + + # Ensure binaries have correct permissions + for binary_path in [self.ffmpeg_path, self.ffprobe_path]: + if binary_path.exists(): + os.chmod(str(binary_path), 0o755) + logger.info(f"Set permissions for {binary_path}") + else: + raise DownloadError( + f"Binary not found after extraction: {binary_path}" + ) + + except Exception as e: + raise DownloadError(f"Failed to extract binaries: {e}") def _extract_zip(self, archive_path: Path, temp_dir: str): """Extract from zip archive (Windows)""" with zipfile.ZipFile(archive_path, "r") as zip_ref: binary_names = self._get_binary_names() for binary_name in binary_names: - binary_files = [f for f in zip_ref.namelist() if binary_name in f] + binary_files = [ + f + for f in zip_ref.namelist() + if f.endswith(f"/{binary_name}") or f.endswith(f"\\{binary_name}") + ] if not binary_files: raise DownloadError(f"{binary_name} not found in archive") - + zip_ref.extract(binary_files[0], temp_dir) extracted_path = Path(temp_dir) / binary_files[0] target_path = self.base_dir / binary_name shutil.copy2(extracted_path, target_path) + logger.info(f"Extracted {binary_name} to {target_path}") def _extract_tar(self, archive_path: Path, temp_dir: str): """Extract from tar archive (Linux/macOS)""" with tarfile.open(archive_path, "r:xz") as tar_ref: binary_names = self._get_binary_names() for binary_name in binary_names: - binary_files = [f for f in tar_ref.getnames() if f.endswith(f"/{binary_name}")] + binary_files = [ + f for f in tar_ref.getnames() if f.endswith(f"/{binary_name}") + ] if not binary_files: raise DownloadError(f"{binary_name} not found in archive") - + tar_ref.extract(binary_files[0], temp_dir) extracted_path = Path(temp_dir) / binary_files[0] target_path = self.base_dir / binary_name shutil.copy2(extracted_path, target_path) + logger.info(f"Extracted {binary_name} to {target_path}") def verify(self) -> bool: """Verify FFmpeg and FFprobe binaries work""" @@ -253,28 +283,32 @@ class FFmpegDownloader: [str(self.ffmpeg_path), "-version"], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - timeout=5 + timeout=5, ) - + # Test FFprobe functionality ffprobe_result = subprocess.run( [str(self.ffprobe_path), "-version"], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - timeout=5 + timeout=5, ) - + if ffmpeg_result.returncode == 0 and ffprobe_result.returncode == 0: - ffmpeg_version = ffmpeg_result.stdout.decode().split('\n')[0] - ffprobe_version = ffprobe_result.stdout.decode().split('\n')[0] + ffmpeg_version = ffmpeg_result.stdout.decode().split("\n")[0] + ffprobe_version = ffprobe_result.stdout.decode().split("\n")[0] logger.info(f"FFmpeg verification successful: {ffmpeg_version}") logger.info(f"FFprobe verification successful: {ffprobe_version}") return True else: if ffmpeg_result.returncode != 0: - logger.error(f"FFmpeg verification failed: {ffmpeg_result.stderr.decode()}") + logger.error( + f"FFmpeg verification failed: {ffmpeg_result.stderr.decode()}" + ) if ffprobe_result.returncode != 0: - logger.error(f"FFprobe verification failed: {ffprobe_result.stderr.decode()}") + logger.error( + f"FFprobe verification failed: {ffprobe_result.stderr.decode()}" + ) return False except Exception as e: diff --git a/videoarchiver/ffmpeg/ffmpeg_manager.py b/videoarchiver/ffmpeg/ffmpeg_manager.py index e719cb4..24de64f 100644 --- a/videoarchiver/ffmpeg/ffmpeg_manager.py +++ b/videoarchiver/ffmpeg/ffmpeg_manager.py @@ -5,10 +5,30 @@ import platform import multiprocessing import logging import subprocess +import traceback from pathlib import Path from typing import Dict, Any, Optional -from videoarchiver.ffmpeg.exceptions import FFmpegError +from videoarchiver.ffmpeg.exceptions import ( + FFmpegError, + DownloadError, + VerificationError, + EncodingError, + AnalysisError, + GPUError, + HardwareAccelerationError, + FFmpegNotFoundError, + FFprobeError, + CompressionError, + FormatError, + PermissionError, + TimeoutError, + ResourceError, + QualityError, + AudioError, + BitrateError, + handle_ffmpeg_error +) from videoarchiver.ffmpeg.gpu_detector import GPUDetector from videoarchiver.ffmpeg.video_analyzer import VideoAnalyzer from videoarchiver.ffmpeg.encoder_params import EncoderParams @@ -59,6 +79,13 @@ class FFmpegManager: logger.info(f"Found existing FFmpeg: {self.downloader.ffmpeg_path}") logger.info(f"Found existing FFprobe: {self.downloader.ffprobe_path}") if self.downloader.verify(): + # Set executable permissions + if platform.system() != "Windows": + try: + os.chmod(str(self.downloader.ffmpeg_path), 0o755) + os.chmod(str(self.downloader.ffprobe_path), 0o755) + except Exception as e: + raise PermissionError(f"Failed to set binary permissions: {e}") return { "ffmpeg": self.downloader.ffmpeg_path, "ffprobe": self.downloader.ffprobe_path @@ -68,9 +95,13 @@ class FFmpegManager: # Download and verify binaries logger.info("Downloading FFmpeg and FFprobe...") - binaries = self.downloader.download() + try: + binaries = self.downloader.download() + except Exception as e: + raise DownloadError(f"Failed to download FFmpeg: {e}") + if not self.downloader.verify(): - raise FFmpegError("Downloaded binaries are not functional") + raise VerificationError("Downloaded binaries are not functional") # Set executable permissions try: @@ -78,12 +109,14 @@ class FFmpegManager: os.chmod(str(binaries["ffmpeg"]), 0o755) os.chmod(str(binaries["ffprobe"]), 0o755) except Exception as e: - logger.error(f"Failed to set binary permissions: {e}") + raise PermissionError(f"Failed to set binary permissions: {e}") return binaries except Exception as e: logger.error(f"Failed to initialize binaries: {e}") + if isinstance(e, (DownloadError, VerificationError, PermissionError)): + raise raise FFmpegError(f"Failed to initialize binaries: {e}") def _verify_ffmpeg(self) -> None: @@ -91,41 +124,61 @@ class FFmpegManager: try: # Check FFmpeg version version_cmd = [str(self.ffmpeg_path), "-version"] - result = subprocess.run( - version_cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - timeout=10 - ) + try: + result = subprocess.run( + version_cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + timeout=10 + ) + except subprocess.TimeoutExpired: + raise TimeoutError("FFmpeg version check timed out") + if result.returncode != 0: - raise FFmpegError("FFmpeg version check failed") + error = handle_ffmpeg_error(result.stderr) + logger.error(f"FFmpeg version check failed: {result.stderr}") + raise error + logger.info(f"FFmpeg version: {result.stdout.split()[2]}") # Check FFprobe version probe_cmd = [str(self.ffprobe_path), "-version"] - result = subprocess.run( - probe_cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - timeout=10 - ) + try: + result = subprocess.run( + probe_cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + timeout=10 + ) + except subprocess.TimeoutExpired: + raise TimeoutError("FFprobe version check timed out") + if result.returncode != 0: - raise FFmpegError("FFprobe version check failed") + error = handle_ffmpeg_error(result.stderr) + logger.error(f"FFprobe version check failed: {result.stderr}") + raise error + logger.info(f"FFprobe version: {result.stdout.split()[2]}") # Check FFmpeg capabilities caps_cmd = [str(self.ffmpeg_path), "-hide_banner", "-encoders"] - result = subprocess.run( - caps_cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - timeout=10 - ) + try: + result = subprocess.run( + caps_cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + timeout=10 + ) + except subprocess.TimeoutExpired: + raise TimeoutError("FFmpeg capabilities check timed out") + if result.returncode != 0: - raise FFmpegError("FFmpeg capabilities check failed") + error = handle_ffmpeg_error(result.stderr) + logger.error(f"FFmpeg capabilities check failed: {result.stderr}") + raise error # Verify encoders required_encoders = ["libx264"] @@ -144,13 +197,16 @@ class FFmpegManager: if missing_encoders: logger.warning(f"Missing encoders: {', '.join(missing_encoders)}") + if "libx264" in missing_encoders: + raise EncodingError("Required encoder libx264 not available") logger.info("FFmpeg verification completed successfully") - except subprocess.TimeoutExpired: - raise FFmpegError("FFmpeg verification timed out") except Exception as e: - raise FFmpegError(f"FFmpeg verification failed: {e}") + logger.error(f"FFmpeg verification failed: {traceback.format_exc()}") + if isinstance(e, (TimeoutError, EncodingError)): + raise + raise VerificationError(f"FFmpeg verification failed: {e}") def analyze_video(self, input_path: str) -> Dict[str, Any]: """Analyze video content for optimal encoding settings""" @@ -160,7 +216,9 @@ class FFmpegManager: return self.video_analyzer.analyze_video(input_path) except Exception as e: logger.error(f"Video analysis failed: {e}") - return {} + if isinstance(e, FileNotFoundError): + raise + raise AnalysisError(f"Failed to analyze video: {e}") def get_compression_params(self, input_path: str, target_size_mb: int) -> Dict[str, str]: """Get optimal compression parameters for the given input file""" @@ -168,7 +226,7 @@ class FFmpegManager: # Analyze video first video_info = self.analyze_video(input_path) if not video_info: - raise FFmpegError("Failed to analyze video") + raise AnalysisError("Failed to analyze video") # Convert target size to bytes target_size_bytes = target_size_mb * 1024 * 1024 @@ -180,6 +238,8 @@ class FFmpegManager: except Exception as e: logger.error(f"Failed to get compression parameters: {e}") + if isinstance(e, AnalysisError): + raise # Return safe default parameters return { "c:v": "libx264", @@ -192,13 +252,13 @@ class FFmpegManager: def get_ffmpeg_path(self) -> str: """Get path to FFmpeg binary""" if not self.ffmpeg_path.exists(): - raise FFmpegError("FFmpeg is not available") + raise FFmpegNotFoundError("FFmpeg is not available") return str(self.ffmpeg_path) def get_ffprobe_path(self) -> str: """Get path to FFprobe binary""" if not self.ffprobe_path.exists(): - raise FFmpegError("FFprobe is not available") + raise FFmpegNotFoundError("FFprobe is not available") return str(self.ffprobe_path) def force_download(self) -> bool: diff --git a/videoarchiver/processor.py b/videoarchiver/processor.py index b707b2b..42872c5 100644 --- a/videoarchiver/processor.py +++ b/videoarchiver/processor.py @@ -14,7 +14,25 @@ from pathlib import Path from videoarchiver.utils.video_downloader import VideoDownloader from videoarchiver.utils.file_ops import secure_delete_file, cleanup_downloads -from videoarchiver.exceptions import ProcessingError, DiscordAPIError +from videoarchiver.utils.exceptions import ( + VideoArchiverError, + VideoDownloadError, + VideoProcessingError, + VideoVerificationError, + VideoUploadError, + VideoCleanupError, + ConfigurationError, + PermissionError, + NetworkError, + ResourceError, + QueueError, + ComponentError +) +from videoarchiver.ffmpeg.exceptions import ( + FFmpegError, + CompressionError, + VideoVerificationError as FFmpegVerificationError +) from videoarchiver.enhanced_queue import EnhancedVideoQueueManager logger = logging.getLogger("VideoArchiver") @@ -56,6 +74,15 @@ class VideoProcessor: self._failed_downloads = set() self._failed_downloads_lock = asyncio.Lock() + # Force re-download FFmpeg binaries to ensure we have working copies + for guild_id in self.components: + if "ffmpeg_mgr" in self.components[guild_id]: + try: + logger.info(f"Force re-downloading FFmpeg binaries for guild {guild_id}") + self.components[guild_id]["ffmpeg_mgr"].force_download() + except Exception as e: + logger.error(f"Failed to force re-download FFmpeg: {e}") + # Start queue processing logger.info("Starting video processing queue...") self._queue_task = asyncio.create_task(self.queue_manager.process_queue(self._process_video)) @@ -68,18 +95,18 @@ class VideoProcessor: # Get the message channel = self.bot.get_channel(item.channel_id) if not channel: - return False, "Channel not found" + raise ConfigurationError("Channel not found") try: message = await channel.fetch_message(item.message_id) if not message: - return False, "Message not found" + raise ConfigurationError("Message not found") except discord.NotFound: - return False, "Message not found" + raise ConfigurationError("Message not found") except discord.Forbidden: - return False, "Bot lacks permissions to fetch message" + raise PermissionError("Bot lacks permissions to fetch message") except Exception as e: - return False, f"Error fetching message: {str(e)}" + raise NetworkError(f"Error fetching message: {str(e)}") guild_id = message.guild.id file_path = None @@ -92,30 +119,25 @@ class VideoProcessor: # Download video with enhanced error handling try: if guild_id not in self.components: - return False, f"Components not initialized for guild {guild_id}" + raise ComponentError(f"Components not initialized for guild {guild_id}") downloader = self.components[guild_id]["downloader"] if not downloader: - return False, "Downloader not initialized" + raise ComponentError("Downloader not initialized") logger.info(f"Starting download for URL: {item.url}") success, file_path, error = await downloader.download_video(item.url) logger.info(f"Download result: success={success}, file_path={file_path}, error={error}") + + if not success: + raise VideoDownloadError(error) + + except (FFmpegError, CompressionError, FFmpegVerificationError) as e: + raise VideoProcessingError(f"FFmpeg error: {str(e)}") except Exception as e: - logger.error(f"Download error: {traceback.format_exc()}") - success, file_path, error = False, None, str(e) - - if not success: - await message.remove_reaction("⏳", self.bot.user) - await message.add_reaction("❌") - await self._log_message( - message.guild, f"Failed to download video: {error}", "error" - ) - # Track failed download for cleanup - if file_path: - async with self._failed_downloads_lock: - self._failed_downloads.add(file_path) - return False, error + if isinstance(e, (VideoDownloadError, VideoProcessingError)): + raise + raise VideoDownloadError(str(e)) # Get channels with enhanced error handling try: @@ -129,14 +151,11 @@ class VideoProcessor: notification_channel = archive_channel if not archive_channel or not notification_channel: - raise DiscordAPIError("Required channels not found") + raise ConfigurationError("Required channels not found") except Exception as e: - await self._log_message( - message.guild, - f"Channel configuration error: {str(e)}", - "error", - ) - return False, str(e) + if isinstance(e, ConfigurationError): + raise + raise ConfigurationError(f"Channel configuration error: {str(e)}") try: # Upload to archive channel with original message link @@ -191,12 +210,7 @@ class VideoProcessor: return True, None except discord.HTTPException as e: - await self._log_message( - message.guild, f"Discord API error: {str(e)}", "error" - ) - await message.remove_reaction("⏳", self.bot.user) - await message.add_reaction("❌") - return False, str(e) + raise NetworkError(f"Discord API error: {str(e)}") finally: # Always attempt to delete the file if configured @@ -208,35 +222,44 @@ class VideoProcessor: f"Successfully deleted file: {file_path}", ) else: - await self._log_message( - message.guild, - f"Failed to delete file: {file_path}", - "error", - ) - # Emergency cleanup - cleanup_downloads( - str( - self.components[guild_id][ - "downloader" - ].download_path - ) - ) + raise VideoCleanupError(f"Failed to delete file: {file_path}") except Exception as e: - logger.error(f"File deletion error: {str(e)}") + if not isinstance(e, VideoCleanupError): + e = VideoCleanupError(f"File deletion error: {str(e)}") + logger.error(str(e)) # Track for later cleanup async with self._failed_downloads_lock: self._failed_downloads.add(file_path) + raise e except Exception as e: logger.error(f"Process error: {traceback.format_exc()}") - await self._log_message( - message.guild, f"Error in process: {str(e)}", "error" - ) - return False, str(e) + if not isinstance(e, VideoArchiverError): + e = VideoProcessingError(f"Error in process: {str(e)}") + raise e except Exception as e: logger.error(f"Error processing video: {traceback.format_exc()}") - return False, str(e) + error_msg = str(e) + + # Update message reactions based on error type + await message.remove_reaction("⏳", self.bot.user) + if isinstance(e, PermissionError): + await message.add_reaction("🚫") + elif isinstance(e, (NetworkError, ResourceError)): + await message.add_reaction("📡") + else: + await message.add_reaction("❌") + + # Log error with appropriate level + if isinstance(e, (ConfigurationError, ComponentError)): + await self._log_message(message.guild, error_msg, "error") + elif isinstance(e, (VideoDownloadError, VideoProcessingError)): + await self._log_message(message.guild, error_msg, "warning") + else: + await self._log_message(message.guild, error_msg, "error") + + return False, error_msg async def process_video_url(self, url: str, message: discord.Message, priority: int = 0) -> bool: """Process a video URL: download, reupload, and cleanup""" @@ -273,7 +296,7 @@ class VideoProcessor: callback=None, # No callback needed since _process_video handles everything priority=priority, ) - except Exception as e: + except QueueError as e: logger.error(f"Queue error: {str(e)}") await message.remove_reaction("⏳", self.bot.user) await message.add_reaction("❌") @@ -296,9 +319,10 @@ class VideoProcessor: except Exception as e: logger.error(f"Error processing video: {traceback.format_exc()}") - await self._log_message( - message.guild, f"Error processing video: {str(e)}", "error" - ) + error_msg = str(e) + if not isinstance(e, VideoArchiverError): + error_msg = f"Unexpected error processing video: {error_msg}" + await self._log_message(message.guild, error_msg, "error") await message.remove_reaction("⏳", self.bot.user) await message.add_reaction("❌") return False @@ -318,9 +342,12 @@ class VideoProcessor: # Find all video URLs in message using yt-dlp simulation urls = [] - if message.guild.id in self.components: - downloader = self.components[message.guild.id]["downloader"] - if downloader: + try: + if message.guild.id in self.components: + downloader = self.components[message.guild.id]["downloader"] + if not downloader: + raise ComponentError("Downloader not initialized") + # Check each word in the message for word in message.content.split(): # Use yt-dlp simulation to check if URL is supported @@ -332,6 +359,12 @@ class VideoProcessor: if any(site in word for site in ["http://", "https://", "www."]): logger.error(f"Error checking URL {word}: {str(e)}") continue + except ComponentError as e: + logger.error(f"Component error: {str(e)}") + await self._log_message( + message.guild, f"Component error: {str(e)}", "error" + ) + return if urls: logger.info(f"Found {len(urls)} video URLs in message {message.id}") @@ -340,13 +373,21 @@ class VideoProcessor: # First URL gets highest priority priority = len(urls) - i logger.info(f"Processing URL {url} with priority {priority}") - await self.process_video_url(url, message, priority) + try: + await self.process_video_url(url, message, priority) + except Exception as e: + logger.error(f"Error processing URL {url}: {str(e)}") + await self._log_message( + message.guild, f"Error processing URL {url}: {str(e)}", "error" + ) + continue except Exception as e: + error_msg = str(e) + if not isinstance(e, VideoArchiverError): + error_msg = f"Unexpected error processing message: {error_msg}" logger.error(f"Error processing message: {traceback.format_exc()}") - await self._log_message( - message.guild, f"Error processing message: {str(e)}", "error" - ) + await self._log_message(message.guild, error_msg, "error") async def _log_message( self, guild: discord.Guild, message: str, level: str = "info" diff --git a/videoarchiver/utils/exceptions.py b/videoarchiver/utils/exceptions.py index a05c3eb..2d39733 100644 --- a/videoarchiver/utils/exceptions.py +++ b/videoarchiver/utils/exceptions.py @@ -1,9 +1,49 @@ -"""Exceptions for the utils package""" +"""Custom exceptions for VideoArchiver""" -class FileCleanupError(Exception): - """Raised when file cleanup fails""" +class VideoArchiverError(Exception): + """Base exception for VideoArchiver errors""" pass -class VideoVerificationError(Exception): - """Raised when video verification fails""" +class VideoDownloadError(VideoArchiverError): + """Error downloading video""" + pass + +class VideoProcessingError(VideoArchiverError): + """Error processing video""" + pass + +class VideoVerificationError(VideoArchiverError): + """Error verifying video""" + pass + +class VideoUploadError(VideoArchiverError): + """Error uploading video""" + pass + +class VideoCleanupError(VideoArchiverError): + """Error cleaning up video files""" + pass + +class ConfigurationError(VideoArchiverError): + """Error in configuration""" + pass + +class PermissionError(VideoArchiverError): + """Error with file permissions""" + pass + +class NetworkError(VideoArchiverError): + """Error with network operations""" + pass + +class ResourceError(VideoArchiverError): + """Error with system resources""" + pass + +class QueueError(VideoArchiverError): + """Error with queue operations""" + pass + +class ComponentError(VideoArchiverError): + """Error with component initialization or cleanup""" pass diff --git a/videoarchiver/utils/video_downloader.py b/videoarchiver/utils/video_downloader.py index a4ba7f4..5ee0ebb 100644 --- a/videoarchiver/utils/video_downloader.py +++ b/videoarchiver/utils/video_downloader.py @@ -14,7 +14,14 @@ from typing import Dict, List, Optional, Tuple from pathlib import Path from videoarchiver.ffmpeg.ffmpeg_manager import FFmpegManager -from videoarchiver.utils.exceptions import VideoVerificationError +from videoarchiver.ffmpeg.exceptions import ( + FFmpegError, + CompressionError, + VideoVerificationError, + FFprobeError, + TimeoutError, + handle_ffmpeg_error +) from videoarchiver.utils.file_ops import secure_delete_file from videoarchiver.utils.path_manager import temp_path_context @@ -34,6 +41,7 @@ class VideoDownloader: max_file_size: int, enabled_sites: Optional[List[str]] = None, concurrent_downloads: int = 3, + ffmpeg_mgr: Optional[FFmpegManager] = None, ): # Ensure download path exists with proper permissions self.download_path = Path(download_path) @@ -47,8 +55,8 @@ class VideoDownloader: self.enabled_sites = enabled_sites # Initialize FFmpeg manager - self.ffmpeg_mgr = FFmpegManager() - logger.info(f"FFmpeg path: {self.ffmpeg_mgr.get_ffmpeg_path()}") + self.ffmpeg_mgr = ffmpeg_mgr or FFmpegManager() + logger.info(f"Using FFmpeg from: {self.ffmpeg_mgr.get_ffmpeg_path()}") # Create thread pool for this instance self.download_pool = ThreadPoolExecutor( @@ -76,7 +84,11 @@ class VideoDownloader: "extractor_retries": self.MAX_RETRIES, "postprocessor_hooks": [self._check_file_size], "progress_hooks": [self._progress_hook], - "ffmpeg_location": self.ffmpeg_mgr.get_ffmpeg_path(), + "ffmpeg_location": str(self.ffmpeg_mgr.get_ffmpeg_path()), # Convert Path to string + "ffprobe_location": str(self.ffmpeg_mgr.get_ffprobe_path()), # Add ffprobe path + "paths": { + "home": str(self.download_path) # Set home directory for yt-dlp + }, "logger": logger, # Use our logger "ignoreerrors": True, # Don't stop on download errors "no_color": True, # Disable ANSI colors in output @@ -269,29 +281,52 @@ class VideoDownloader: f"compressed_{os.path.basename(original_file)}", ) - # Run FFmpeg directly with subprocess instead of ffmpeg-python - cmd = [ - self.ffmpeg_mgr.get_ffmpeg_path(), - "-i", original_file - ] + # Build FFmpeg command with full path + ffmpeg_path = str(self.ffmpeg_mgr.get_ffmpeg_path()) + logger.debug(f"Using FFmpeg from: {ffmpeg_path}") - # Add all parameters + # Build command with all parameters + cmd = [ffmpeg_path, "-y"] # Overwrite output file if it exists + + # Add input file + cmd.extend(["-i", original_file]) + + # Add all compression parameters for key, value in params.items(): - cmd.extend([f"-{key}", str(value)]) + if key == "c:v" and value == "libx264": + # Use hardware acceleration if available + gpu_info = self.ffmpeg_mgr.gpu_info + if gpu_info["nvidia"]: + cmd.extend(["-c:v", "h264_nvenc"]) + elif gpu_info["amd"]: + cmd.extend(["-c:v", "h264_amf"]) + elif gpu_info["intel"]: + cmd.extend(["-c:v", "h264_qsv"]) + else: + cmd.extend(["-c:v", "libx264"]) + else: + cmd.extend([f"-{key}", str(value)]) # Add output file cmd.append(compressed_file) # Run compression in executor - await asyncio.get_event_loop().run_in_executor( - self.download_pool, - lambda: subprocess.run( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - check=True + logger.debug(f"Running FFmpeg command: {' '.join(cmd)}") + try: + result = await asyncio.get_event_loop().run_in_executor( + self.download_pool, + lambda: subprocess.run( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=True + ) ) - ) + logger.debug(f"FFmpeg output: {result.stderr.decode()}") + except subprocess.CalledProcessError as e: + error = handle_ffmpeg_error(e.stderr.decode()) + logger.error(f"FFmpeg error: {e.stderr.decode()}") + raise error if not os.path.exists(compressed_file): raise FileNotFoundError( @@ -310,7 +345,15 @@ class VideoDownloader: return True, compressed_file, "" else: await self._safe_delete_file(compressed_file) - return False, "", "Failed to compress to target size" + raise CompressionError( + "Failed to compress to target size", + input_size=file_size, + target_size=self.max_file_size * 1024 * 1024 + ) + except (FFmpegError, VideoVerificationError, FileNotFoundError, CompressionError) as e: + if compressed_file and os.path.exists(compressed_file): + await self._safe_delete_file(compressed_file) + return False, "", str(e) except Exception as e: if compressed_file and os.path.exists(compressed_file): await self._safe_delete_file(compressed_file) diff --git a/videoarchiver/video_archiver.py b/videoarchiver/video_archiver.py index 0d32589..d346322 100644 --- a/videoarchiver/video_archiver.py +++ b/videoarchiver/video_archiver.py @@ -18,6 +18,7 @@ from videoarchiver.utils.video_downloader import VideoDownloader from videoarchiver.utils.message_manager import MessageManager from videoarchiver.utils.file_ops import cleanup_downloads from videoarchiver.enhanced_queue import EnhancedVideoQueueManager +from videoarchiver.ffmpeg.ffmpeg_manager import FFmpegManager # Add FFmpeg manager import from videoarchiver.exceptions import ( ProcessingError, ConfigError, @@ -164,6 +165,8 @@ class VideoArchiver(commands.Cog): await components['message_manager'].cancel_all_deletions() if 'downloader' in components: components['downloader'] = None + if 'ffmpeg_mgr' in components: + components['ffmpeg_mgr'] = None except Exception as e: logger.error(f"Error cleaning up guild {guild_id}: {str(e)}") @@ -199,16 +202,23 @@ class VideoArchiver(commands.Cog): await old_components['message_manager'].cancel_all_deletions() if 'downloader' in old_components: old_components['downloader'] = None + if 'ffmpeg_mgr' in old_components: + old_components['ffmpeg_mgr'] = None + # Initialize FFmpeg manager first + ffmpeg_mgr = FFmpegManager() + # Initialize new components with validated settings self.components[guild_id] = { + 'ffmpeg_mgr': ffmpeg_mgr, # Add FFmpeg manager to components 'downloader': VideoDownloader( str(self.download_path), settings['video_format'], settings['video_quality'], settings['max_file_size'], settings['enabled_sites'] if settings['enabled_sites'] else None, - settings['concurrent_downloads'] + settings['concurrent_downloads'], + ffmpeg_mgr=ffmpeg_mgr # Pass FFmpeg manager to VideoDownloader ), 'message_manager': MessageManager( settings['message_duration'], @@ -245,6 +255,8 @@ class VideoArchiver(commands.Cog): await components['message_manager'].cancel_all_deletions() if 'downloader' in components: components['downloader'] = None + if 'ffmpeg_mgr' in components: + components['ffmpeg_mgr'] = None # Remove guild components self.components.pop(guild.id)