Skip to content

Docker: FFmpeg with audio devices support (Pulse, Alsa) #2586

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

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Jan 15, 2025

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Fixes #2584
Fixes #1509

  • Env var SE_RECORD_AUDIO to enable recording audio
  • Env var SE_AUDIO_SOURCE to specify FFmpeg arguments for audio source (support Pulse, Alsa device). Default is -f pulse -ac 2 -i default

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement, documentation, configuration changes


Description

  • Added audio recording support to FFmpeg with PulseAudio integration.

  • Updated Dockerfiles to include necessary dependencies for audio recording.

  • Introduced new environment variables for audio recording configuration.

  • Enhanced documentation to reflect new audio recording capabilities.


Changes walkthrough 📝

Relevant files
Enhancement
2 files
video.sh
Enable audio recording in FFmpeg with conditional logic. 
+16/-4   
extract_env.py
Sort files and dictionaries for consistent variable extraction.
+3/-0     
Configuration changes
5 files
Dockerfile
Add PulseAudio support and dependencies for FFmpeg.           
+3/-2     
Dockerfile
Update session retry interval configuration.                         
+1/-1     
Dockerfile
Add audio dependencies and environment variables for video container.
+4/-2     
value.yaml
Add default values for new audio-related environment variables.
+174/-170
docker-compose-v3-test-standalone-docker.yaml
Enable audio recording in test configuration.                       
+1/-0     
Documentation
2 files
ENV_VARIABLES.md
Document new environment variables for audio recording.   
+4/-2     
description.yaml
Add descriptions for new audio-related environment variables.
+6/-0     

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling

The new audio recording feature lacks proper error handling for cases where audio device initialization fails or audio recording encounters issues during the session.

if [ "${SE_RECORD_AUDIO,,}" = "true" ]; then
  echo "$(date -u +"${ts_format}") [${process_name}] - Audio source arguments: ${SE_AUDIO_SOURCE}"
else
  SE_AUDIO_SOURCE=""
fi
Process Management

The FFMPEG process management could be improved by adding proper cleanup of audio resources when the recording is stopped or interrupted.

ffmpeg -hide_banner -loglevel warning -flags low_delay -threads 2 -fflags nobuffer+genpts -strict experimental -y -f x11grab \
  -video_size ${VIDEO_SIZE} -r ${FRAME_RATE} -i ${DISPLAY} ${SE_AUDIO_SOURCE} -codec:v ${CODEC} ${PRESET} -pix_fmt yuv420p "$video_file" &
FFMPEG_PID=$!
if ps -p $FFMPEG_PID >/dev/null; then
  wait $FFMPEG_PID
fi

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible issue
Implement robust process monitoring to detect and handle FFmpeg failures during recording

Add process monitoring to detect if FFmpeg crashes or exits unexpectedly during
recording.

Video/video.sh [218-223]

 ffmpeg -hide_banner -loglevel warning -flags low_delay -threads 2 -fflags nobuffer+genpts -strict experimental -y -f x11grab \
   -video_size ${VIDEO_SIZE} -r ${FRAME_RATE} -i ${DISPLAY} ${SE_AUDIO_SOURCE} -codec:v ${CODEC} ${PRESET} -pix_fmt yuv420p "$video_file" &
 FFMPEG_PID=$!
-if ps -p $FFMPEG_PID >/dev/null; then
-  wait $FFMPEG_PID
+if ! ps -p $FFMPEG_PID >/dev/null; then
+  echo "$(date -u +"${ts_format}") [${process_name}] - Error: FFmpeg failed to start"
+  exit 1
+fi
+while kill -0 $FFMPEG_PID 2>/dev/null; do
+  sleep 1
+done
+wait $FFMPEG_PID
+exit_code=$?
+if [ $exit_code -ne 0 ]; then
+  echo "$(date -u +"${ts_format}") [${process_name}] - Error: FFmpeg exited with code $exit_code"
+  exit $exit_code
 fi
  • Apply this suggestion
Suggestion importance[1-10]: 9

Why: The suggestion significantly improves reliability by adding comprehensive FFmpeg process monitoring, error detection, and proper exit code handling, which are crucial for maintaining recording stability.

9
Add error handling for audio device initialization to prevent recording failures

Add error handling to verify that the FFmpeg process started successfully and handle
potential failures when audio recording is enabled.

Video/video.sh [207-211]

 if [ "${SE_RECORD_AUDIO,,}" = "true" ]; then
   echo "$(date -u +"${ts_format}") [${process_name}] - Audio source arguments: ${SE_AUDIO_SOURCE}"
+  # Verify audio device availability
+  if ! ffmpeg -f pulse -list_devices true 2>&1 | grep -q "pulse"; then
+    echo "$(date -u +"${ts_format}") [${process_name}] - Warning: PulseAudio not available, disabling audio recording"
+    SE_AUDIO_SOURCE=""
+  fi
 else
   SE_AUDIO_SOURCE=""
 fi
  • Apply this suggestion
Suggestion importance[1-10]: 8

Why: The suggestion adds critical error handling to verify PulseAudio availability before starting recording, preventing silent failures when audio recording is enabled but the audio device is not available.

8

Copy link

qodo-merge-pro bot commented Jan 15, 2025

CI Failure Feedback 🧐

(Checks updated until commit 63ada3a)

Action: Rerun workflow when failure

Failed stage: Authenticate GitHub CLI for PR [❌]

Failure summary:

The action failed because the GitHub authentication token lacks the required 'read:org' permission
scope. Specifically:

  • The workflow attempted to authenticate using gh auth login --with-token with the GH_CLI_TOKEN_PR
    token
  • The provided token does not have the 'read:org' scope which is required for the GitHub CLI
    authentication
  • This resulted in an authentication error and the workflow failed with exit code 1

  • Relevant error logs:
    1:  ##[group]Operating System
    2:  Ubuntu
    ...
    
    28:  SecurityEvents: write
    29:  Statuses: write
    30:  ##[endgroup]
    31:  Secret source: Actions
    32:  Prepare workflow directory
    33:  Prepare all required actions
    34:  Getting action download info
    35:  Download action repository 'actions/checkout@main' (SHA:cbb722410c2e876e24abbe8de2cc27693e501dcb)
    36:  Complete job name: Rerun workflow when failure
    ...
    
    48:  show-progress: true
    49:  lfs: false
    50:  submodules: false
    51:  set-safe-directory: true
    52:  env:
    53:  GH_CLI_TOKEN: ***
    54:  GH_CLI_TOKEN_PR: ***
    55:  RUN_ID: 12798016840
    56:  RERUN_FAILED_ONLY: true
    ...
    
    119:  ##[group]Run sudo apt update
    120:  �[36;1msudo apt update�[0m
    121:  �[36;1msudo apt install gh�[0m
    122:  shell: /usr/bin/bash -e {0}
    123:  env:
    124:  GH_CLI_TOKEN: ***
    125:  GH_CLI_TOKEN_PR: ***
    126:  RUN_ID: 12798016840
    127:  RERUN_FAILED_ONLY: true
    ...
    
    169:  0 upgraded, 0 newly installed, 0 to remove and 63 not upgraded.
    170:  ##[group]Run echo "$GH_CLI_TOKEN_PR" | gh auth login --with-token
    171:  �[36;1mecho "$GH_CLI_TOKEN_PR" | gh auth login --with-token�[0m
    172:  shell: /usr/bin/bash -e {0}
    173:  env:
    174:  GH_CLI_TOKEN: ***
    175:  GH_CLI_TOKEN_PR: ***
    176:  RUN_ID: 12798016840
    177:  RERUN_FAILED_ONLY: true
    178:  RUN_ATTEMPT: 1
    179:  ##[endgroup]
    180:  error validating token: missing required scope 'read:org'
    181:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @VietND96 VietND96 merged commit 575723a into trunk Jan 15, 2025
    33 of 36 checks passed
    @VietND96 VietND96 deleted the ffmpeg branch January 15, 2025 23:49
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    1 participant