feat: Add Apple Silicon support with VideoToolbox hardware encoding (#15)#18
feat: Add Apple Silicon support with VideoToolbox hardware encoding (#15)#18ebbbang wants to merge 1 commit intoJMS1717:mainfrom
Conversation
Add hardware-accelerated video encoding for macOS using VideoToolbox. This enables H.264 and HEVC encoding on M1/M2/M3/M4 Macs via a hybrid setup where Docker runs services and a native worker uses the GPU. - Add _check_videotoolbox() detection in hw_detect.py - Add h264_videotoolbox and hevc_videotoolbox encoder mappings - Add VideoToolbox startup tests - Create docker-compose.macos.yml (exposes Redis on port 6380) - Create scripts/macos-setup.sh for easy setup - Update models.py and settings_manager.py with new codecs - Update documentation (README, GPU_SUPPORT.md, copilot-instructions)
WalkthroughThis PR introduces Apple Silicon and macOS support with VideoToolbox hardware acceleration. Changes include backend model expansion for VideoToolbox codecs, new hardware detection logic, a macOS-specific Docker Compose file, a native worker setup script, and comprehensive documentation updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/macos-setup.sh (1)
112-112: Consider adding concurrency flag for consistency.The Celery worker command omits the concurrency flag. Based on coding guidelines, the supervisord pattern includes
--concurrency=4. Consider adding it for consistency:- celery -A worker.celery_app worker --loglevel=info + celery -A worker.celery_app worker --loglevel=info --concurrency=4This ensures consistent behavior with the Docker-based worker.
Based on coding guidelines, worker startup should follow the supervisord pattern with concurrency specified.
docs/GPU_SUPPORT.md (1)
128-146: Optional: Add language specifier to code fence.The ASCII diagram would be more semantically correct with a language identifier:
-``` +```text ┌─────────────────────────────────────────────────────┐ │ Docker (Linux containers) │This addresses the static analysis hint while clarifying the content type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/copilot-instructions.md(2 hunks).gitignore(1 hunks)README.md(1 hunks)backend-api/app/models.py(4 hunks)backend-api/app/settings_manager.py(2 hunks)docker-compose.macos.yml(1 hunks)docs/GPU_SUPPORT.md(4 hunks)scripts/macos-setup.sh(1 hunks)worker/app/hw_detect.py(7 hunks)worker/app/startup_tests.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/{backend-api,worker}/app/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/{backend-api,worker}/app/**/*.py: Job/task IDs: backend generatesjob_idand worker uses Celerytask_id. Redis keys must follow the pattern:job:{task.id},progress:{task_id}andcancel:{task_id}
File naming convention: uploads must be saved to/app/uploadswithjobid_filenameformat; outputs to/app/outputswith_8mblocal_{taskid}suffix to avoid collisions
When editing worker or backend code, preserve Redis key names (job:{task.id},progress:{task_id},cancel:{task_id}) and published event formats (type, task_id, progress fields)
Files:
backend-api/app/settings_manager.pyworker/app/startup_tests.pybackend-api/app/models.pyworker/app/hw_detect.py
backend-api/app/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend API: use
celery_app.send_task('worker.worker.compress_video', ...)pattern to enqueue Celery tasks with proper task kwargs; output naming must follow file naming conventions
Files:
backend-api/app/settings_manager.pybackend-api/app/models.py
worker/app/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
worker/app/**/*.py: Progress messages published to Redis must be JSON events withtypefield (log,progress,done, orerror) and includetask_idandprogressfields for frontend consumption
Hardware encoder detection and testing: do not assume a listed hardware encoder will initialize successfully; prefer reading the startup test cache or respectingENCODER_TEST_CACHElogic inworker/app/worker.py
Files:
worker/app/startup_tests.pyworker/app/hw_detect.py
worker/app/{hw_detect,worker}.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Encoder mapping: requested codec is mapped to hardware encoder in
worker/app/hw_detect.pyviamap_codec_to_hwfunction; when startup test marks encoder unavailable, fall back to CPU encoders
Files:
worker/app/hw_detect.py
🧠 Learnings (9)
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to backend-api/app/**/*.py : Backend API: use `celery_app.send_task('worker.worker.compress_video', ...)` pattern to enqueue Celery tasks with proper task kwargs; output naming must follow file naming conventions
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to **/{backend-api,worker}/app/**/*.py : File naming convention: uploads must be saved to `/app/uploads` with `jobid_filename` format; outputs to `/app/outputs` with `_8mblocal_{taskid}` suffix to avoid collisions
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to **/{backend-api,worker}/app/**/*.py : Job/task IDs: backend generates `job_id` and worker uses Celery `task_id`. Redis keys must follow the pattern: `job:{task.id}`, `progress:{task_id}` and `cancel:{task_id}`
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to worker/app/**/*.py : Hardware encoder detection and testing: do not assume a listed hardware encoder will initialize successfully; prefer reading the startup test cache or respecting `ENCODER_TEST_CACHE` logic in `worker/app/worker.py`
Applied to files:
.github/copilot-instructions.mddocs/GPU_SUPPORT.mdworker/app/startup_tests.pyworker/app/hw_detect.py
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to worker/app/{hw_detect,worker}.py : Encoder mapping: requested codec is mapped to hardware encoder in `worker/app/hw_detect.py` via `map_codec_to_hw` function; when startup test marks encoder unavailable, fall back to CPU encoders
Applied to files:
.github/copilot-instructions.mddocs/GPU_SUPPORT.mdworker/app/startup_tests.pyworker/app/hw_detect.py
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to **/{backend-api,worker}/app/**/*.py : When editing worker or backend code, preserve Redis key names (`job:{task.id}`, `progress:{task_id}`, `cancel:{task_id}`) and published event formats (type, task_id, progress fields)
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to supervisord.conf : Worker startup command should follow supervisord pattern: `celery -A worker.celery_app worker --loglevel=info -n 8mblocal@%h --concurrency=4` with `REDIS_URL` environment variable set
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Applies to worker/app/**/*.py : Progress messages published to Redis must be JSON events with `type` field (`log`, `progress`, `done`, or `error`) and include `task_id` and `progress` fields for frontend consumption
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:09:55.051Z
Learnt from: CR
Repo: JMS1717/8mb.local PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:09:55.051Z
Learning: Frontend development: run `npm run dev` from frontend directory using Vite; `npm run build` for production bundles; use SSE for live progress updates and call backend APIs
Applied to files:
.github/copilot-instructions.md
🪛 LanguageTool
.github/copilot-instructions.md
[uncategorized] ~29-~29: The operating system from Apple is written “macOS”.
Context: ...undles. - macOS with Apple Silicon: Use docker-compose.macos.yml for services, then run `./scripts/...
(MAC_OS)
🪛 markdownlint-cli2 (0.18.1)
docs/GPU_SUPPORT.md
128-128: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
547-547: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
552-552: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.8)
worker/app/hw_detect.py
349-349: Starting a process with a partial executable path
(S607)
359-375: Starting a process with a partial executable path
(S607)
379-379: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (18)
.gitignore (1)
9-9: LGTM!The
venv/ignore pattern aligns with the macOS setup workflow that creates a Python virtual environment for the native worker..github/copilot-instructions.md (1)
15-15: LGTM! Documentation accurately reflects macOS VideoToolbox workflow.The updates correctly document the hybrid macOS setup (Docker services + native worker) and VideoToolbox as a macOS-only hardware path, consistent with the new artifacts introduced in this PR.
Also applies to: 29-29, 34-36
scripts/macos-setup.sh (1)
1-114: LGTM! Script follows best practices for macOS setup.The script correctly:
- Verifies prerequisites (Homebrew, Apple Silicon, Python 3.10+, FFmpeg with VideoToolbox)
- Installs missing dependencies when Homebrew is available
- Sets up a Python virtual environment and installs requirements
- Provides clear instructions for running the native worker with proper environment variables
The Redis URL (
redis://localhost:6380/0) correctly aligns with the port mapping indocker-compose.macos.yml.docker-compose.macos.yml (1)
1-26: LGTM! Port mapping correctly enables hybrid setup.The configuration appropriately:
- Exposes Redis on port 6380 externally (
6380:6379) to avoid conflicts with native Redis- Sets internal
REDIS_URLtoredis://127.0.0.1:6379/0for container services- Allows the native macOS worker to connect via
redis://localhost:6380/0This correctly implements the hybrid Docker + native worker architecture.
worker/app/startup_tests.py (1)
418-424: LGTM! VideoToolbox test configuration follows established patterns.The VideoToolbox hardware type is correctly integrated:
- Tests H.264 and HEVC encoders (VideoToolbox's supported codecs)
- Uses appropriate hardware decoder flags (
-hwaccel videotoolbox)- Consistent with existing NVIDIA/Intel/VAAPI test structure
- Correctly omits AV1 (VideoToolbox doesn't support AV1 encoding)
Based on learnings, encoder testing follows the startup test pattern and will validate actual initialization.
backend-api/app/settings_manager.py (1)
401-403: LGTM! VideoToolbox codec visibility settings integrated correctly.The additions:
- Follow the established pattern for hardware codec visibility (NVENC/QSV/VAAPI/AMF)
- Use consistent environment variable naming (
CODEC_H264_VIDEOTOOLBOX,CODEC_HEVC_VIDEOTOOLBOX)- Default to
truelike other codecs- Align with the VideoToolbox codec literals added to
models.pyBased on coding guidelines, codec visibility settings control which codecs appear in the UI.
Also applies to: 428-429
README.md (1)
546-575: LGTM! macOS section clearly documents both installation paths.The documentation correctly:
- Distinguishes CPU-only Docker (Option 1) from hybrid VideoToolbox setup (Option 2)
- Provides step-by-step commands for the hybrid setup
- Uses correct Redis URL (
redis://localhost:6380/0) matching the port mapping indocker-compose.macos.yml- Sets appropriate environment variables (UPLOAD_DIR, OUTPUT_DIR, PYTHONPATH)
- References GPU_SUPPORT.md for additional details
docs/GPU_SUPPORT.md (3)
10-11: LGTM! VideoToolbox correctly integrated into multi-vendor GPU support.The documentation accurately:
- Positions VideoToolbox as the fourth hardware option (macOS/Apple Silicon)
- Notes the requirement for a native worker (Docker on macOS lacks GPU passthrough)
- Shows correct encoder mappings (h264_videotoolbox, hevc_videotoolbox, N/A for AV1)
- Explains AV1 CPU fallback due to VideoToolbox limitations
Based on learnings, encoder mapping follows the established pattern in worker/app/hw_detect.py.
Also applies to: 20-21, 29-35
124-181: LGTM! macOS hybrid setup clearly documented.The section provides comprehensive guidance:
- Architecture diagram illustrating Docker services + native worker
- Step-by-step setup commands matching README.md and scripts/macos-setup.sh
- Correct Redis URL (
redis://localhost:6380/0) for native worker connection- Prerequisites and limitations clearly stated
268-268: LGTM! Future enhancements updated to reflect completion.The checkbox correctly marks Apple VideoToolbox support as completed.
worker/app/hw_detect.py (5)
4-4: LGTM!The
platformimport is correctly used to detect macOS/Darwin for VideoToolbox availability.
76-89: LGTM!The VideoToolbox detection correctly integrates into the detection priority order and appropriately omits AV1 support (as noted in the comment). The structure is consistent with other hardware acceleration backends.
413-414: LGTM!The VideoToolbox encoder handling correctly omits
init_hw_device(unlike QSV/VAAPI), which is consistent with how VideoToolbox encoders operate. The pixel format (nv12) and profile settings are appropriate for VideoToolbox hardware encoding.Note: The pattern of setting
decode_method="videotoolbox"in detection but not adding hwaccel init flags here follows the same pattern as NVENC (which setsdecode_method="cuda"but handles hwaccel in the worker). This assumes the worker will determine whether to use hardware decoding based on input codec support.Also applies to: 453-459
510-515: LGTM!The legacy fallback path for VideoToolbox encoders correctly mirrors the explicit encoder handling, ensuring consistent flag application regardless of which code path is taken.
341-382: Align VideoToolbox encoding test timeout with other hardware detection tests.The 10-second timeout for the VideoToolbox encoding test (line 377) is inconsistent with the 5-second timeout used for Intel QSV (line 209). Both functions test identical minimal encoding scenarios (64×64 video, 0.1s duration, 1 frame), yet VideoToolbox uses 2× the timeout without code comments or documentation justifying the difference. Reduce the VideoToolbox encoding test timeout to 5 seconds to match the established pattern across hardware detection functions.
⛔ Skipped due to learnings
Learnt from: CR Repo: JMS1717/8mb.local PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-15T16:09:55.051Z Learning: Applies to worker/app/**/*.py : Hardware encoder detection and testing: do not assume a listed hardware encoder will initialize successfully; prefer reading the startup test cache or respecting `ENCODER_TEST_CACHE` logic in `worker/app/worker.py`backend-api/app/models.py (3)
20-20: LGTM!The VideoToolbox codec additions (
h264_videotoolbox,hevc_videotoolbox) are consistently applied across all relevant model definitions and correctly omitav1_videotoolboxsince VideoToolbox does not support AV1 encoding.Also applies to: 68-68, 108-108
96-98: LGTM!The VideoToolbox visibility settings follow the established pattern for other hardware encoder types and correctly include only H.264 and HEVC (omitting AV1, which VideoToolbox doesn't support).
143-143: Clarify: Is this field related to VideoToolbox support?The
phasefield addition toJobMetadatais not mentioned in the PR objectives or description, which focus on Apple Silicon/VideoToolbox hardware encoding support. This field appears to be for tracking encoding phases (queued → encoding → finalizing → done).While the field is properly typed and backward compatible, can you confirm whether this belongs in this PR or should be part of a separate feature? If it's related to better progress reporting for VideoToolbox encoding jobs, please clarify the connection.
|
@bradlington Can you give this a try? CC: @JMS1717 |
Closes #15
Summary
Architecture
Since Docker Desktop on macOS runs in a Linux VM without GPU passthrough, this uses a hybrid approach:
Changes
worker/app/hw_detect.py: Added_check_videotoolbox()and encoder mappingsworker/app/startup_tests.py: Added VideoToolbox test configurationbackend-api/app/models.py: Added VideoToolbox codec literalsbackend-api/app/settings_manager.py: Added VideoToolbox visibility settingsdocker-compose.macos.yml: New compose file for macOS hybrid setupscripts/macos-setup.sh: Setup script for native worker dependenciesdocs/GPU_SUPPORT.md: Added VideoToolbox documentationREADME.md: Updated macOS section with hybrid setup instructions.github/copilot-instructions.md: Added VideoToolbox references.gitignore: Addedvenv/Usage