Conversation
8d9869d to
56f3538
Compare
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
Move the functions out of run.sh to lib_gstreamer.sh. I see duplicates and still optimizations can be done.
The current commit carries ~1600 lines of code, split it into logical commits inside the same PR:
-
"gstreamer: add advanced video pipeline helpers"
- UVC discovery;
- stack-aware decoder arguments;
- advanced pipeline builders;
- common execution and validation helpers.
-
"video: add UVC and DRC GStreamer tests"
- UVC preview;
- DRC caps-transition validation;
- display prerequisites.
-
"video: add concurrent decode tests"
- H.264/H.265/MJPEG;
- dynamic session layout;
- concurrency-specific validation.
-
"video: add downstream advanced encode tests"
- smart encode;
- cyclic IR;
- slice MB;
- Rotate90;
- current-run artifact validation.
-
"docs: document advanced GStreamer video coverage"
- synchronized defaults;
- prerequisites;
- exact PASS/SKIP criteria;
- verified execution examples.
3534470 to
4ff8619
Compare
|
Srikanth Muppandam (@smuppand) , followed similar pattern like earlier all runner script related changes in one commit and helper function changes in another. addressed the review comments and some comments updated explicitly. |
| : >"$test_log" | ||
|
|
||
| # Build pipeline using library function with 2 sessions | ||
| pipeline=$(gstreamer_build_concurrency_decode_pipeline "mjpeg" "$mjpeg_clip" "2") |
There was a problem hiding this comment.
The helper’s third argument is video_stack, not session count, and the helper hardcodes sessions=8
Add an explicit session-count argument to gstreamer_build_concurrency_decode_pipeline, generate an appropriate two-session layout, and pass "$detected_stack" 2.
There was a problem hiding this comment.
session count want to limit to 8 sessions for h264, h265 and 2 sessions to mjpeg. made changes acc.
There was a problem hiding this comment.
session count want to limit to 8 sessions for h264, h265 and 2 sessions to mjpeg. made changes acc.
Add an explicit session-count argument to gstreamer_build_concurrency_decode_pipeline, validate it, generate the matching layout, and call MJPEG with "$detected_stack" "2".
| : >"$test_log" | ||
|
|
||
| # Build pipeline using library function with 2 sessions | ||
| pipeline=$(gstreamer_build_concurrency_decode_pipeline "mjpeg" "$mjpeg_clip" "2") |
There was a problem hiding this comment.
session count want to limit to 8 sessions for h264, h265 and 2 sessions to mjpeg. made changes acc.
Add an explicit session-count argument to gstreamer_build_concurrency_decode_pipeline, validate it, generate the matching layout, and call MJPEG with "$detected_stack" "2".
|
nitinn22 Please fix the shellcheck failed workflow. |
4ff8619 to
cc4fa57
Compare
|
addressed comments and shell check failures. |
cc4fa57 to
50d33bb
Compare
|
50d33bb to
15d3331
Compare
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
Follow-up on UVC thread
This remains unresolved. Missing frame/FPS evidence still only produces a warning at line 1437, after which timeout can be accepted as PASS. Require positive frame or buffer activity and fail when it is absent.
Follow-up on DRC thread
This remains unresolved. The implementation only finds 1080p and 720p anywhere in combined console and dmesg output. It still does not prove an ordered 1080p→720p caps transition or require frame activity after the second caps event.
Follow-up on concurrency thread
This is only partially resolved. The decoder count is now strict, but it counts element names from verbose output. That proves construction, not that every branch negotiated caps and produced buffers. Require per-branch negotiated and active-buffer evidence.
Follow-up on Weston cleanup thread
This remains unresolved. run.sh cleanup still does not call camera_cleanup_wayland_environment. Also, weston_was_running is reset during every setup call, which can overwrite the suite’s original compositor state.
Follow-up on artifact-validation thread
This remains unresolved. gstreamer_validate_encode_output still returns success when gst-discoverer-1.0 is unavailable. Advanced encode tests must not PASS without validating codec, container, resolution, and duration.
Will test code and update the PR. |
|
Srikanth Muppandam (@smuppand) , after testing the caps and buffer validation changes. the scripts seem fine from implementation standpoint but the logs are failing in validation. seeing sample stdout below. But the expectation on screen is fine wrt all 8 videos are playing fine, the logs arent coming properly for these validation, can we instead make these warning and pass them based on strict check of 8 session initiation as they are L1 tests? |
Please do not downgrade the per-branch caps/buffer failures to warnings. Even for L1 coverage, a PASS needs machine-verifiable evidence that every requested decode branch became active, screen observation and decoder-name counts can produce false passes. Please push the tested implementation first. Then make the evidence collection reliable, for example by enabling deterministic GStreamer verbosity/debug for the explicitly named branches and validating caps plus buffer/frame activity for each branch. If that evidence cannot be collected on a supported target, the testcase should be skipped with a clear capability reason, not passed. |
15d3331 to
1d5c8e5
Compare
Sure, pushed the changes. for now although the test is running it would fail due to this strict check, working with dev counterparts meanwhile to see if with log levels or any other verbosity we can validate these exactly and make them pass. |
1d5c8e5 to
2f94a10
Compare
|
Shell check failures are fixed. |
a903b0b to
a0ea4d7
Compare
Signed-off-by: nitinn <nitinn@qti.qualcomm.com>
Signed-off-by: nitinn <nitinn@qti.qualcomm.com>
a0ea4d7 to
55c7e57
Compare
All changes maintain POSIX compliance and BusyBox compatibility. |
| # Send SIGINT instead of SIGTERM to trigger EOS via -e flag | ||
| gstreamer_run_bounded "$secs" INT "$GSTBIN" $GSTLAUNCHFLAGS $pipe | ||
| gstlaunch_rc=$? | ||
| gst_rc=$? |
There was a problem hiding this comment.
This refactor removes the only read of GST_BOUNDED_RAW_RC. ShellCheck consequently reports SC2034 on the shared helper assignment, and the required PR ShellCheck job is failing. Preserve the raw-status diagnostic, especially the SIGKILL-escalation distinction, or remove/rework the variable consistently across
the helper and callers.
|
|
||
| # Check 2b: Negotiated video caps observed (extended check) | ||
| # Search gst_debug_log since GST_DEBUG_FILE redirects debug output there | ||
| if ! grep -q "caps.*video/x-raw" "$gst_debug_log"; then |
There was a problem hiding this comment.
The extended path requires caps in gst_debug_log, but the default GST_DEBUG=2 generally only records warnings/errors there. Negotiated caps produced by
gst-launch -v are written to console_log. Healthy UVC, DRC, and concurrency pipelines can therefore fail before their mode-specific validation. Validate both
retained logs or enable a targeted caps debug category explicitly.
|
|
||
| # REQUIRE frame activity after the 720p caps event (second resolution) | ||
| # Extract all lines after the 720p caps event | ||
| lines_after_720p=$(tail -n +$((first_720p_line + 1)) "$console_log" || true) |
There was a problem hiding this comment.
first_720p_line is a line number from gst_debug_log, but it is applied to console_log here. These logs have unrelated line numbering, so the claimed post-
transition frame check can falsely pass or fail. Locate the transition and subsequent activity in the same retained evidence stream.
|
|
||
| # Check if this decoder instance has buffer activity evidence | ||
| # Look for buffer/frame activity mentioning this specific decoder instance | ||
| if ! grep -q "${decoder_instance}.*buffer\|${decoder_instance}.*frame\|chain.*${decoder_instance}" "$console_log"; then |
There was a problem hiding this comment.
gst-launch -v does not emit per-buffer activity for each decoder into console_log. At the default debug level this grep cannot prove active buffers and can fail every healthy concurrency run. Instrument each branch with measurable activity or capture a targeted debug stream containing branch-specific buffer evidence.
| gstreamer_reset_element_cache | ||
|
|
||
| # Clean up Wayland/Weston if we started it | ||
| if command -v camera_cleanup_wayland_environment >/dev/null 2>&1; then |
There was a problem hiding this comment.
Before extending this EXIT cleanup, remove the name-wide pkill fallback immediately above. When no direct child is found, pkill -x gst-launch-1.0
terminates unrelated pipelines on the target. Track the PIDs or process groups started by this suite and stop only those.
| # Get discoverer output and save to log file for debugging | ||
| discover_log="${output_file}.rotation_discover.log" | ||
| # Run gst-discoverer with timeout and SIGKILL escalation (30s timeout, 10s grace) | ||
| if ! timeout --signal=INT --kill-after=10 30 gst-discoverer-1.0 "$output_file" >"$discover_log" 2>&1; then |
There was a problem hiding this comment.
Inside if ! timeout ...; then, $? is the status of !, so discover_rc is always zero. Timeout and command-failure diagnostics are incorrect. Run the bounded command, save its status, and branch on that value. Reuse gstreamer_run_bounded rather than invoking timeout directly.
| fi | ||
| ;; | ||
|
|
||
| uvc) |
There was a problem hiding this comment.
The global dependency check requires curl and tar before dispatch, although UVC and advanced-encode use no downloaded clips. Minimal Yocto images can skip
valid camera coverage unnecessarily. Gate clip-fetch dependencies only for modes that require downloaded assets.
This branch has not been deployed
2 participants
Summary
Enhance the GStreamer video encode/decode test framework.
Changes
Validation