make Audio_Record_Playback more deterministic for offline CI#411
make Audio_Record_Playback more deterministic for offline CI#411smuppand wants to merge 1 commit intoqualcomm-linux:mainfrom
Conversation
… overrides Update audio clip provisioning to prefer locally generated GStreamer sample clips for OGG and MP3 when no external clip source is explicitly requested, making CI runs less dependent on network availability. This keeps backward compatibility by preserving existing --clip-path, --clip-url, AUDIO_CLIP_PATH, and AUDIO_CLIP_URL behavior. The run.sh changes now track whether an external source was explicitly requested and pass that state into provision_test_files(), while the provisioning logic can still fall back to user-provided clips or downloads when requested. Signed-off-by: Srikanth Muppandam <smuppand@qti.qualcomm.com>
|
@nitinn22 to review |
| have_ogg=0 | ||
| have_mp3=0 | ||
|
|
||
| mark_existing_samples() { |
There was a problem hiding this comment.
These helper functions are defined inside provision_test_files(). This works in many shells, but it’s less common/less readable and can confuse tooling (and redefines functions on each call). Consider moving them to top-level (or at least prefix their names) to keep the script maintainable.
|
|
||
| mark_existing_samples() { | ||
| if [ -f "$sample_ogg" ]; then | ||
| size="$(gstreamer_file_size_bytes "$sample_ogg")" |
There was a problem hiding this comment.
size is reused for both OGG and MP3 checks. It’s fine, but might be clearer to use ogg_size / mp3_size so logs and debugging are easier to follow.
|
|
||
| if gstreamer_validate_log "$test_log" "provision_sample_ogg"; then | ||
| if [ -f "$sample_ogg" ] && [ -s "$sample_ogg" ]; then | ||
| size="$(gstreamer_file_size_bytes "$sample_ogg")" |
There was a problem hiding this comment.
size is reused for both OGG and MP3 checks. It’s fine, but might be clearer to use ogg_size / mp3_size so logs and debugging are easier to follow.
| test_log="$outdir/provision_sample_ogg.log" | ||
| : >"$test_log" | ||
|
|
||
| pipeline="audiotestsrc wave=sine freq=440 volume=1.0 num-buffers=$num_buffers ! audioconvert ! vorbisenc ! oggmux ! filesink location=$sample_ogg" |
There was a problem hiding this comment.
Consider adding audioresample (in addition to audioconvert) before encoding for robustness across platforms:
... ! audioconvert ! audioresample ! vorbisenc ...
Some setups may require resampling/negotiation to avoid caps issues
| test_log="$outdir/provision_sample_mp3.log" | ||
| : >"$test_log" | ||
|
|
||
| pipeline="audiotestsrc wave=sine freq=440 volume=1.0 num-buffers=$num_buffers ! audioconvert ! lamemp3enc target=bitrate cbr=true bitrate=128 ! filesink location=$sample_mp3" |
There was a problem hiding this comment.
lamemp3enc property support can vary across plugin versions. If this is expected to run across multiple distro/plugin versions, consider either:
keeping the pipeline minimal (lamemp3enc ! filesink ...) and letting defaults apply, or
checking/guarding the properties (if your helper lib supports querying element properties), or at least logging a clear warning if gst-launch fails due to invalid property
| log_info "Using user-provided clip path: $clip_path" | ||
|
|
||
| if [ "$have_ogg" -eq 0 ] && [ -f "$clip_path/sample_audio.ogg" ]; then | ||
| cp "$clip_path/sample_audio.ogg" "$sample_ogg" 2>/dev/null || true |
There was a problem hiding this comment.
Copy failures are fully suppressed (2>/dev/null || true), which can make debugging hard (permissions, missing files, etc.)
| fi | ||
|
|
||
| if [ "$have_mp3" -eq 0 ] && [ -f "$clip_path/sample_audio.mp3" ]; then | ||
| cp "$clip_path/sample_audio.mp3" "$sample_mp3" 2>/dev/null || true |
There was a problem hiding this comment.
Copy failures are fully suppressed (2>/dev/null || true), which can make debugging hard (permissions, missing files, etc.)
| log_info "Generating local OGG sample from audiotestsrc..." | ||
| log_info "Pipeline: $pipeline" | ||
|
|
||
| if gstreamer_run_gstlaunch_timeout "$((duration_secs + 10))" "$pipeline" >>"$test_log" 2>&1; then |
There was a problem hiding this comment.
Assumes duration_secs is numeric. If parsing guarantees numeric earlier, all good. If not, consider defensive validation or defaulting:
: "${duration_secs:=10}" and numeric check before arithmetic.
| log_info "Generating local MP3 sample from audiotestsrc..." | ||
| log_info "Pipeline: $pipeline" | ||
|
|
||
| if gstreamer_run_gstlaunch_timeout "$((duration_secs + 10))" "$pipeline" >>"$test_log" 2>&1; then |
There was a problem hiding this comment.
Assumes duration_secs is numeric. If parsing guarantees numeric earlier, all good. If not, consider defensive validation or defaulting:
: "${duration_secs:=10}" and numeric check before arithmetic.
|
Main suggestions are around avoiding silent copy failures and improving portability/robustness of the generation pipelines (audioresample, encoder property compatibility). Also, if a LAVA job can be posted with these encode followed by decode and full suite working fine would be perfect. |
2 participants
Improve Audio_Record_Playback stability and result reporting for CI runs. Enhancements to address as part of #408
This update keeps backward compatibility for user-provided clip sources, but makes the default path more reliable by preferring locally generated OGG/MP3 sample clips when no clip path or clip URL is explicitly requested.
It also tightens result handling for dependent pulsesrc playback cases. When running in all mode, playback_pulsesrc_wav and playback_pulsesrc_flac now fail if the corresponding pulsesrc recording
in the same run did not produce a valid file, instead of being marked as skip. This makes the final summary reflect the actual end-to-end failure more accurately.
Changes included:
prerequisite recording failed in the same run
Why: