-
Notifications
You must be signed in to change notification settings - Fork 0
Add makemkvcon disc ripping and encoding automation with YAML configuration, online metadata lookup, and secure API key management #1
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
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
…aceholders Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
|
@copilot how about disc and track identification using an online database? |
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Implemented online disc and track identification! The solution includes: Disc Identification:
Track/Metadata Identification:
Usage:
metadata:
lookup_enabled: true
tmdb_api_key: "your_key"
./rip_disc.py --config config.yaml --title "inception" --output ~/Movies
# Output: Inception (2010).mkv (with correct capitalization from TMDB)Benefits:
|
|
@copilot Read any secrets like API KEYs from an environment variable instead of a config file |
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Implemented! API keys are now read from environment variables for better security. Changes:
Usage: # Linux/macOS
export TMDB_API_KEY="your_api_key_here"
# Windows CMD
set TMDB_API_KEY=your_api_key_here
# Windows PowerShell
$env:TMDB_API_KEY="your_api_key_here"Benefits:
|
…l Ctrl+C exit\n\n- Removed terminal clear at startup to avoid wiping the screen\n- Suppressed progress bar init log for cleaner output\n- Handle KeyboardInterrupt: print friendly message and exit 130
…e\n\n- Refresh batch scripts for movies and TV\n- Update README usage and notes\n- Adjust requirements.txt for Rich integration\n- Add Makefile targets for rip workflows\n- Exclude run log from commits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive automated workflow for ripping and encoding DVDs, Blu-Ray, and UltraHD discs using MakeMKV. The implementation includes intelligent content detection, online metadata lookup via TMDB, and secure API key management through environment variables.
Key changes:
- Main Python script with 1400+ lines handling disc scanning, track selection, encoding, and metadata-based file naming
- Online disc identification using MusicBrainz and TMDB for accurate titles and metadata
- Secure API key management with environment variable support prioritized over config files
- Shell wrappers and batch processing scripts for streamlined workflows
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 35 comments.
Show a summary per file
| File | Description |
|---|---|
| rip_disc.py | Core implementation with disc scanning, content identification, track selection, encoding, online metadata lookup, and secure API key handling |
| rip_movie.sh | Shell wrapper for single movie disc ripping with argument parsing |
| rip_tv.sh | Shell wrapper for TV series disc ripping with season/episode handling |
| batch_rip_movies.sh | Batch processing script for multiple movie discs |
| batch_rip_tv_season.sh | Batch processing script for TV season discs |
| requirements.txt | Python dependencies including PyYAML, requests, tmdbsimple, discid, and rich |
| config.example.yaml | Example configuration with metadata lookup settings and API key documentation |
| Makefile | Build automation for virtual environment and dependency management |
| README.md | Comprehensive documentation with usage examples and troubleshooting |
| EXAMPLES.md | Detailed usage examples for various scenarios |
| .gitignore | Git ignore rules including config files with secrets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if dur > 0: | ||
| est_title_bytes = int(dur * 2.5 * 1024 * 1024) | ||
| # Use tqdm-aware logging to avoid interfering with the bar line | ||
| write_log(f"📊 Estimated title {title_id} size (approx.): {self.human_bytes(est_title_bytes)}", 'info') |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The write_log function is called on line 780 but defined later on line 784. This will cause a NameError because the function doesn't exist at the point it's called. Move the function definition before its first use (before line 773).
| except Exception: | ||
| pass | ||
| # Helper to print logs using Rich console | ||
| def write_log(msg: str, level: str = 'info'): |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The write_log function references live variable which is only defined inside the if RICH_AVAILABLE: block (line 726). When Rich is not available, calling this function will raise a NameError. Consider adding a check for RICH_AVAILABLE inside the function, or ensure live is initialized to None before the if block and handle the None case properly.
| def write_log(msg: str, level: str = 'info'): | |
| def write_log(msg: str, level: str = 'info'): | |
| # If Rich is not available, always fall back to the standard logger | |
| if not RICH_AVAILABLE: | |
| if level == 'info': | |
| logger.info(msg) | |
| elif level == 'warning': | |
| logger.warning(msg) | |
| elif level == 'error': | |
| logger.error(msg) | |
| else: | |
| logger.debug(msg) | |
| return |
| def _get_rows_cols(): | ||
| try: | ||
| fd = os.open('/dev/tty', os.O_RDONLY) | ||
| try: | ||
| buf = struct.pack('hhhh', 0, 0, 0, 0) | ||
| res = fcntl.ioctl(fd, termios.TIOCGWINSZ, buf) | ||
| rows, cols, _, _ = struct.unpack('hhhh', res) | ||
| if rows and cols: | ||
| return int(rows), int(cols) | ||
| finally: | ||
| os.close(fd) | ||
| except Exception: | ||
| pass | ||
| try: | ||
| env_cols = int(os.environ.get('COLUMNS', '0')) | ||
| env_rows = int(os.environ.get('LINES', '0')) | ||
| if env_cols > 0 and env_rows > 0: | ||
| return env_rows, env_cols | ||
| except Exception: | ||
| pass | ||
| try: | ||
| out = subprocess.run('stty size < /dev/tty', shell=True, capture_output=True, text=True) | ||
| if out.returncode == 0 and out.stdout.strip(): | ||
| r, c = out.stdout.strip().split() | ||
| return int(r), int(c) | ||
| except Exception: | ||
| pass | ||
| return 24, 80 | ||
|
|
||
| # Create Rich console for all output, write to /dev/tty to bypass tee | ||
| try: | ||
| rows, cols = _get_rows_cols() | ||
| except Exception: | ||
| rows, cols = (24, 80) | ||
|
|
||
| try: | ||
| tty_file = open('/dev/tty', 'w') | ||
| console = Console(file=tty_file, force_terminal=True, width=cols) | ||
| except Exception: | ||
| console = Console(force_terminal=True, width=cols) | ||
| tty_file = None |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code attempts to open /dev/tty which is Unix-specific and won't exist on Windows systems. While the exception is caught, this makes the code non-functional on Windows. Consider adding a platform check and using Windows equivalents (like CON) or disabling Rich progress bars gracefully on Windows.
| def _get_rows_cols(): | |
| try: | |
| fd = os.open('/dev/tty', os.O_RDONLY) | |
| try: | |
| buf = struct.pack('hhhh', 0, 0, 0, 0) | |
| res = fcntl.ioctl(fd, termios.TIOCGWINSZ, buf) | |
| rows, cols, _, _ = struct.unpack('hhhh', res) | |
| if rows and cols: | |
| return int(rows), int(cols) | |
| finally: | |
| os.close(fd) | |
| except Exception: | |
| pass | |
| try: | |
| env_cols = int(os.environ.get('COLUMNS', '0')) | |
| env_rows = int(os.environ.get('LINES', '0')) | |
| if env_cols > 0 and env_rows > 0: | |
| return env_rows, env_cols | |
| except Exception: | |
| pass | |
| try: | |
| out = subprocess.run('stty size < /dev/tty', shell=True, capture_output=True, text=True) | |
| if out.returncode == 0 and out.stdout.strip(): | |
| r, c = out.stdout.strip().split() | |
| return int(r), int(c) | |
| except Exception: | |
| pass | |
| return 24, 80 | |
| # Create Rich console for all output, write to /dev/tty to bypass tee | |
| try: | |
| rows, cols = _get_rows_cols() | |
| except Exception: | |
| rows, cols = (24, 80) | |
| try: | |
| tty_file = open('/dev/tty', 'w') | |
| console = Console(file=tty_file, force_terminal=True, width=cols) | |
| except Exception: | |
| console = Console(force_terminal=True, width=cols) | |
| tty_file = None | |
| if os.name != 'nt': | |
| def _get_rows_cols(): | |
| try: | |
| fd = os.open('/dev/tty', os.O_RDONLY) | |
| try: | |
| buf = struct.pack('hhhh', 0, 0, 0, 0) | |
| res = fcntl.ioctl(fd, termios.TIOCGWINSZ, buf) | |
| rows, cols, _, _ = struct.unpack('hhhh', res) | |
| if rows and cols: | |
| return int(rows), int(cols) | |
| finally: | |
| os.close(fd) | |
| except Exception: | |
| pass | |
| try: | |
| env_cols = int(os.environ.get('COLUMNS', '0')) | |
| env_rows = int(os.environ.get('LINES', '0')) | |
| if env_cols > 0 and env_rows > 0: | |
| return env_rows, env_cols | |
| except Exception: | |
| pass | |
| try: | |
| out = subprocess.run('stty size < /dev/tty', shell=True, capture_output=True, text=True) | |
| if out.returncode == 0 and out.stdout.strip(): | |
| r, c = out.stdout.strip().split() | |
| return int(r), int(c) | |
| except Exception: | |
| pass | |
| return 24, 80 | |
| else: | |
| # On Windows, avoid using /dev/tty and rely on standard terminal size detection. | |
| import shutil | |
| def _get_rows_cols(): | |
| try: | |
| size = shutil.get_terminal_size(fallback=(80, 24)) | |
| # shutil returns (cols, rows) | |
| return int(size.lines), int(size.columns) | |
| except Exception: | |
| return 24, 80 | |
| # Create Rich console for all output, write to /dev/tty to bypass tee where supported | |
| try: | |
| rows, cols = _get_rows_cols() | |
| except Exception: | |
| rows, cols = (24, 80) | |
| tty_file = None | |
| if os.name != 'nt': | |
| try: | |
| tty_file = open('/dev/tty', 'w') | |
| console = Console(file=tty_file, force_terminal=True, width=cols) | |
| except Exception: | |
| console = Console(force_terminal=True, width=cols) | |
| tty_file = None | |
| else: | |
| # On Windows, do not attempt to open /dev/tty; let Rich handle the console. | |
| console = Console(force_terminal=True, width=cols) |
| - Typically this is the main feature film | ||
|
|
||
| **For TV Series:** | ||
| - Finds all titles between 20-50 minutes |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states TV episodes are "20-50 minutes" but the code constant MAX_EPISODE_DURATION_SECONDS = 3600 is 60 minutes, and the comment on line 594 also says "20-60 minutes each". Update the documentation to match the actual implementation (20-60 minutes).
| - Finds all titles between 20-50 minutes | |
| - Finds all titles between 20-60 minutes |
| if tty_file: | ||
| tty_file.close() |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable tty_file is referenced on line 947 but may be undefined when RICH_AVAILABLE is False. The variable is only initialized inside the if RICH_AVAILABLE: block (lines 712-717), so accessing it outside that block when Rich is not available will raise a NameError. Initialize tty_file = None before the if RICH_AVAILABLE: block to fix this issue.
| est_title_bytes = int(dur * 2.5 * 1024 * 1024) | ||
| # Use tqdm-aware logging to avoid interfering with the bar line | ||
| write_log(f"📊 Estimated title {title_id} size (approx.): {self.human_bytes(est_title_bytes)}", 'info') | ||
| except Exception: |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | ||
| pass |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as exc: | |
| logging.getLogger(__name__).debug( | |
| "Non-fatal error while updating rip progress UI", | |
| exc_info=exc, | |
| ) |
| except Exception: | ||
| pass |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as e: | |
| logger.warning("Failed to restore previous SIGWINCH handler: %s", e) |
| except Exception: | ||
| pass |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as e: | |
| logger.debug(f"Failed to extract title from filename {file_path}: {e}") |
| last_pct_logged = pct_int | ||
| except Exception: | ||
| logger.debug(f"Could not parse progress line: {line}") | ||
| pass |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary 'pass' statement.
| pass |
…mmand handling\n\n- Use context manager + run_command_with_progress()\n- Keep ffmpeg binary in command; strip -progress pipe:1\n- Show percent updates via Rich bar writing to /dev/tty\n- Add package to requirements.txt
- Change WARNING level display to 'WARN' for cleaner output - Upgrade missing library logs to warning level for expected dependencies (tmdbsimple, ffmpeg-progress-yield) - Remove success log for Rich library (only warn when missing) - Track actual CINFO source (CINFO:0 vs CINFO:1) and report in disc type detection logs - Add TV-specific Blu-ray bitrate fallback (4.3 MB/s) for more accurate TV episode estimates - Add warning when TV mode detects few episodes but finds a long title (suggests movie disc)
- Add --disc-type option to rip_movie.sh and rip_tv.sh for manual override - Add --year option to rip_tv.sh for TV special identification - Update README.md with disc type override documentation and examples - Document size estimation behavior and fallback rates
- Remove 'make shell' target; 'make activate' now launches an activated shell - Add 'make rip-tv' target alongside 'make rip-movie' - Set bash as recipe shell for consistency (SHELL := /bin/bash) - Document TMDB_API_KEY and OMDB_API_KEY environment variables - Clarify OMDB is primary metadata source, TMDB is fallback - Add Mermaid diagrams: workflow, architecture, and activation sequence - Update all examples to use Make targets - Remove Windows-specific documentation (PowerShell, cmd) - Improve diagram readability with explicit dark text colors - Update EXAMPLES.md with Make target usage examples
- Reorder help targets by logical workflow: Setup → Activation → Usage → Cleanup - Clarify that 'make all' and 'make install' are equivalent - Note that 'make venv' creates environment only (without dependencies) - Add practical examples for rip-movie and rip-tv with OUTPUT and EXTRA_ARGS - Improve readability with section headers and spacing
rip_disc.py: Suppress verbose MakeMKV scan output after first title, add concise progress logs (per-title captions shown once, include position in progress bar), and show percentage updates as titles rip. Report found title count upfront. Add informative logs when OMDB/TMDB lookups succeed; warn when falling back to disc title. Support starting episode numbering via new --episode-start CLI flag and propagate through process_disc() and rename flow. rip_tv.sh: Add --episode-start option, propagate to Python script, and update help text for clarity. README.md: Enable Mermaid theme initialization for activation sequence diagram for better visual consistency.
|
I decided to do this in .NET instead. |
Implements automated workflow for ripping DVDs, Blu-Ray, and UltraHD discs using makemkvcon, with intelligent content detection, track selection, online disc identification, metadata-based file naming, and secure API key management via environment variables.
Core Implementation
rip_disc.py(900+ lines): Main Python script handling disc scanning, content identification, track selection, encoding, and file namingShell wrappers:
rip_movie.sh,rip_tv.sh- Simplified interfaces for common use casesBatch scripts:
batch_rip_movies.sh,batch_rip_tv_season.sh- Process multiple discs sequentiallyOnline Identification Features
OnlineDiscIdentifier Class:
Benefits:
Configuration
YAML-based configuration with sensible defaults:
API Key Configuration (Secure Method)
Recommended: Environment Variable
Alternative: Config File (Less Secure)
The environment variable takes precedence over the config file. Using environment variables prevents accidental commits of API keys to version control.
Usage
Security & Quality
shutil.which()Dependencies
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.