-
Notifications
You must be signed in to change notification settings - Fork 33
Audio Support (rev2) #90
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
base: main
Are you sure you want to change the base?
Conversation
Mesa DescriptionTL;DREnabled PulseAudio-based audio support in the headful Chromium image and updated the client UI with a new theme color and improved video unmute experience. Why we made these changesTo provide audio drivers and socket support for a live view with audio, adapting previous audio support to the new image setup, and to update the client loader theme. What changed?Container/Runtime (chromium-headful)
Client/UI
ValidationNo explicit validation steps were provided in the PR description. Description generated by Mesa. Update settings |
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.
Performed full review of b51da91...947a021
Analysis
-
Security Permission Issues: Multiple files have overly permissive settings (777/666) including xorg.conf, start-pulseaudio.sh, and Dockerfile configurations for /etc/pulse. These should be replaced with more restrictive 0660/0750 permissions with appropriate group ownership.
-
Contradictory Configuration: PulseAudio has conflicting settings with daemon.conf setting
allow-module-loading = yeswhile start-pulseaudio.sh uses the--disallow-module-loadingflag, creating potential maintenance confusion. -
Environment Variable Redundancy: Environment variables are set in multiple places (Dockerfile, wrapper.sh), which could lead to maintenance challenges if values need to change.
-
Code Redundancy: The video.vue component uses both
{ once: true }option AND manual cleanup in beforeDestroy, creating unnecessary code.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
15 files reviewed | 0 comments | Edit Agent Settings
Note
Introduces PulseAudio-based audio support and a new
/computer/cursorAPI, updates Docker/runtime (FFmpeg caching, BuildKit cache keys, Playwright engine switch), and refreshes the web client theme/loader with mute handling.POST /computer/cursorwithSetCursorto hide/show cursor viaunclutter; OpenAPI and generated client/server updated.--syncfromxdotoolcalls; minor input handling refinements.PLAYWRIGHT_ENGINE(playwright-core or patchright)./etc/pulse/{default.pa,daemon.conf}, D-Bus policies, runtime env (PULSE_SERVER,XDG_RUNTIME_DIR), user groups; supervisord-managed PulseAudio with readiness checks.0666.patchright.run-*-shacceptPLAYWRIGHT_ENGINEand map recordings.ensure-common-build-run-vars.sh: UKC vars required only whenrequire-ukc-varsis passed.#7B42F6; loader replaced with spinning logo.Written by Cursor Bugbot for commit 2050f1f. This will update automatically on new commits. Configure here.