fix(jetson): remove GPL video codecs, fix ffmpeg/examples/healthcheck#1190
Open
toolboc wants to merge 3 commits into
Open
fix(jetson): remove GPL video codecs, fix ffmpeg/examples/healthcheck#1190toolboc wants to merge 3 commits into
toolboc wants to merge 3 commits into
Conversation
- Remove unused x264/x265 GPL video codecs from FFmpeg build (audio-only) - Remove --disable-programs so ffmpeg CLI is available for MP3 export - Pin bitsandbytes to >=0.49.0,<0.51 for aarch64 stability - Fix healthcheck port variable to match docker-compose (API_PORT) - Add examples symlink workaround for _get_project_root() path bug
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughJetson Dockerfile: replaced video codec dev packages with audio/assembly tools, reconfigured FFmpeg to drop GPL/x264/x265 and enable audio codecs, tightened bitsandbytes pin, added nano-vllm post-install sanity check, created additional runtime symlink, and expanded healthcheck and entrypoint API-port resolution. ChangesJetson Build Configuration and Dependencies
Sequence Diagram(s)sequenceDiagram
autonumber
participant HC as Container Healthcheck
participant GR as Gradio Server
participant API as API Server
HC->>GR: HTTP probe on ${GRADIO_PORT}
alt Gradio OK
HC-->>HC: pass
else Gradio not ready
HC->>API: HTTP probe on ${API_PORT}/${ACESTEP_API_PORT}/8001
alt API OK
HC-->>HC: pass
else
HC-->>HC: fail
end
end
Note right of API: Entrypoint uses API_PORT → ACESTEP_API_PORT fallback for startup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile.jetson`:
- Around line 282-285: The HEALTHCHECK is probing ${API_PORT:-8001} while the
entrypoint starts the API server using ${ACESTEP_API_PORT:-8001}, causing a
mismatch if only API_PORT is overridden; update the HEALTHCHECK command to probe
the same effective port expression used by the entrypoint (use the nested
fallback ${API_PORT:-${ACESTEP_API_PORT:-8001}}) and change the entrypoint echo
and the python --port argument in the API start command to also use
${API_PORT:-${ACESTEP_API_PORT:-8001}} so both the probe and the server use the
identical port resolution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Use nested fallback ${API_PORT:-${ACESTEP_API_PORT:-8001}} in both the
HEALTHCHECK and entrypoint so the probe and server always resolve the
same port regardless of which env var is set.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Dockerfile.jetson (1)
242-245:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix shell parsing break in runtime-dir RUN command.
At Line 242–Line 245, the inline comments after the continuation cause
&& ln -sf ...to be parsed as a new command starting with&&, which breaks the build.Suggested minimal fix
- RUN mkdir -p /app/checkpoints /app/gradio_outputs \ - # Symlink examples into the path where api_routes.py's _get_project_root() - # resolves (acestep/ui/) until the upstream bug is fixed. - && ln -sf /app/examples /app/acestep/ui/examples + # Symlink examples into the path where api_routes.py's _get_project_root() + # resolves (acestep/ui/) until the upstream bug is fixed. + RUN mkdir -p /app/checkpoints /app/gradio_outputs \ + && ln -sf /app/examples /app/acestep/ui/examples🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile.jetson` around lines 242 - 245, The RUN shell line that creates directories and then symlinks examples is broken by an inline comment placed after a line-continuation, causing the following "&& ln -sf /app/examples /app/acestep/ui/examples" to be parsed incorrectly; fix by moving the comment out of the continued command (either put the explanatory comment on its own line above the RUN or place it after the entire RUN block), and ensure the RUN uses proper backslash-newline continuations so the commands stay on the same shell line (the block referencing ln -sf /app/examples /app/acestep/ui/examples and the note about _get_project_root() should be kept but not inserted mid-continuation).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@Dockerfile.jetson`:
- Around line 242-245: The RUN shell line that creates directories and then
symlinks examples is broken by an inline comment placed after a
line-continuation, causing the following "&& ln -sf /app/examples
/app/acestep/ui/examples" to be parsed incorrectly; fix by moving the comment
out of the continued command (either put the explanatory comment on its own line
above the RUN or place it after the entire RUN block), and ensure the RUN uses
proper backslash-newline continuations so the commands stay on the same shell
line (the block referencing ln -sf /app/examples /app/acestep/ui/examples and
the note about _get_project_root() should be kept but not inserted
mid-continuation).
Shell comments between a backslash continuation and && break parsing. Move the explanatory comment above the RUN command.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses review feedback from #793 and #735:
--disable-programssoffmpegCLI binary is available for MP3 export inaudio_utils.pybitsandbytes>=0.49.0,<0.51for aarch64 stabilityAPI_PORTmatching docker-compose.jetson.yml_get_project_root()path resolution bug inapi_routes.pySummary by CodeRabbit
Changes
New Features
Improvements