gstreamer: bound hardware probes so a wedged codec cannot hang the run - #549
Conversation
| return $? | ||
| fi | ||
|
|
||
| timeout --signal="$bounded_sig" --kill-after="$GST_KILL_GRACE" "$bounded_secs" "$@" |
There was a problem hiding this comment.
The helper uses GNU-only --signal and --kill-after. BusyBox timeout, commonly used by Yocto images, accepts -s and -k. Use portable short options and normalize BusyBox/GNU timeout statuses:
timeout -s "$bounded_sig" -k "$GST_KILL_GRACE" ...
| shift 2 | ||
|
|
||
| if ! command -v timeout >/dev/null 2>&1; then | ||
| log_warn "No timeout command available, running '$1' without a time bound" |
There was a problem hiding this comment.
Missing timeout restores the unbounded hang. Fail or skip cleanly when no compatible timeout implementation exists. Do not execute hardware probes or pipelines unbounded.
The GStreamer video suites can hang indefinitely when the hardware video codec wedges, turning a single failed test case into a dead test run. Observed on hamoa/x1e80100 (LAVA jobs 374345 and 377799), where the VPU throws a system error on first use: qcom-iris aa00000.video-codec: received system error of type 0x5000003 after which every V4L2 access blocks in the kernel. Two unbounded call sites turn that into a full lava-test-shell timeout: - has_element() runs gst-inspect-1.0 with no time bound at all. It is the first thing run_encode_test() calls, and gst-inspect opens every /dev/video* node while validating the plugin registry, so it never returns. In job 374345 the run stopped there, before the pipeline was even built, and burned the entire 10 minute budget. - gstreamer_run_gstlaunch_timeout() uses `timeout --signal=INT` with no --kill-after. A gst-launch-1.0 blocked in a V4L2 ioctl never reacts to SIGINT, so timeout waits for it forever and the nominal duration + 10 bound never takes effect. This is what happened in job 377799. In both cases the remaining test definitions never execute and LAVA reports them as "missing expected test case" failures, which hides the real result: one codec broke, the rest were never tried. Add gstreamer_run_bounded(), which always escalates to SIGKILL after GST_KILL_GRACE seconds, and use it for gst-inspect-1.0, gst-discoverer-1.0 and gst-launch-1.0. A wedged codec now surfaces as an unavailable element or a failed pipeline, and the following test cases still get to run. SIGINT is still the first signal sent to gst-launch-1.0, so muxers keep finalising their output via -e on a healthy timeout; only a process that ignores it is escalated. Behaviour is unchanged when the hardware works. Signed-off-by: Milosz Wasilewski <milosz.wasilewski@oss.qualcomm.com>
2afe9ae to
0bce495
Compare
| # shellcheck disable=SC2086 | ||
| "$GSTBIN" $GSTLAUNCHFLAGS $pipe | ||
| return $? | ||
| return "$gstlaunch_rc" |
There was a problem hiding this comment.
The helper correctly returns 124 or 125, but Video_Encode_Decode/run.sh does not reject the encode return code. It proceeds to log validation and accepts any existing output larger than 1000 bytes. Because the encoded directory can be shared, a stale file or partial output from a killed process can produce PASS.
IMO, Remove the intended output before launching and make the encode case fail immediately when gstRc != 0. Keep timeout acceptance only in callers where reaching PLAYING until the configured duration is intentionally considered success.
| gstreamer_run_bounded "$GST_PROBE_TIMEOUT" TERM "$GSTINSPECT" "$elem" >/dev/null 2>&1 | ||
| has_element_rc=$? | ||
|
|
||
| if gstreamer_bounded_no_timeout "$has_element_rc"; then |
There was a problem hiding this comment.
has_element() converts timeout and missing-timeout results into ordinary return code 1 and caches them. Video callers interpret an unavailable encoder or decoder as SKIP.
Preserve distinct probe outcomes for missing, timed out and unable to execute. Hardware codec probe timeout/no-timeout should become FAIL while a genuinely optional missing element may remain SKIP. Cache the classified result rather than only 0/1.
| has_element_rc="$GST_ELEM_MISSING" | ||
| fi | ||
|
|
||
| eval "HAS_ELEMENT_CACHE_${has_element_key}=\$has_element_rc" |
There was a problem hiding this comment.
has_element() cache does not persist for video probes. Calls such as encoder=$(gstreamer_v4l2_encoder_for_codec ...) run in subshells, so HAS_ELEMENT_CACHE_* assignments disappear. Repeated resolutions can therefore each consume the full 60-second timeout.
|
|
||
| # Check if decoder is available | ||
| decoder=$(gstreamer_v4l2_decoder_for_codec "$codec") | ||
| probe_rc=$? |
There was a problem hiding this comment.
VP9 decoder probing still converts timeout/no-timeout outcomes to return code 1, causing run_decode_test() to report SKIP instead of FAIL. Capture and return has_element’s status as done for H.264/H.265.
has_element() collapsed every failure to 1, so a probe that had to be stopped
was indistinguishable from an element that is simply absent. run_encode_test
treats an empty encoder as SKIP, so a wedged codec was reported as
"not applicable".
Classify the outcome instead: GST_ELEM_OK, GST_ELEM_MISSING, GST_ELEM_TIMEOUT
and GST_ELEM_NO_TIMEOUT. Every non-zero value still means "not usable", so
boolean callers are unaffected, but the reason survives - including in the
cache, which now stores the classification.
gstreamer_v4l2_{encoder,decoder}_for_codec propagate it, and
gstreamer_probe_unhealthy() tells callers which outcomes are hardware or
environment failures. Video_Encode_Decode now reports FAIL for those and keeps
SKIP for a genuinely missing element.
run_encode_test logged the gst-launch status and then ignored it, deciding
purely on log validation plus an output file larger than 1000 bytes. The
encoded directory is shared between the test definitions in a job, so a stale
artifact produced PASS.
Delete the intended output before launching and fail immediately when the
encode status is non-zero.
log_warn and log_error write to stdout, and has_element runs inside command
substitution, so the timeout warning was spliced into the element name and
from there into the pipeline string - leaving $encoder non-empty and defeating
both the old SKIP path and the new FAIL path. Send those two diagnostics to
stderr.
Signed-off-by: Milosz Wasilewski <milosz.wasilewski@oss.qualcomm.com>
c82ef14 to
95130b1
Compare
|
Srikanth Muppandam (@smuppand) I pushed a new version but it doesn't work as expected. Testing it now. I'll make an update once I have something better. |
…e up The probe classification added earlier separates a wedged codec from an absent one, but it does not catch the case that motivated it. When the video firmware fails to load, the driver never registers its V4L2 element, so gst-inspect answers cleanly that the element does not exist. That is indistinguishable from a platform which genuinely has no such codec, and Video_Encode_Decode reports SKIP for both. LAVA jobs 382831 and 382832 show it plainly: two hamoa-iot-evk boards logged qcom-iris aa00000.video-codec: firmware download failed -16 qcom-iris aa00000.video-codec: core init failed and the suite still reported SKIP Only the kernel log separates "this platform has no such codec" from "the codec is there and broken", so consult it - but only for the V4L2 hardware codec lookups, and only when the element is absent. If the probe succeeded, or the element is absent on a machine with a quiet log, the result is unchanged. GST_ELEM_HW_FAULT joins the existing outcomes and gstreamer_probe_unhealthy treats it as a failure, so Video_Encode_Decode reports FAIL without any change to its own logic. The signature is deliberately narrow - hard firmware or init failures on a codec device - so that a transient session error the driver recovers from, or an unrelated GPU firmware message, does not turn a legitimate SKIP into a failure. gstreamer_probe_reason() turns the numeric outcome into a sentence, so the test log says why the codec was unusable rather than printing rc=4. Signed-off-by: Milosz Wasilewski <milosz.wasilewski@oss.qualcomm.com>
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
The previous cache-persistence and VP9 classification issues are fixed.
| # Drops every cached has_element answer. Use after reloading codec modules or | ||
| # otherwise changing which GStreamer elements are registered. | ||
| gstreamer_reset_element_cache() { | ||
| rm -rf "$GST_ELEM_CACHE_DIR" 2>/dev/null || true |
There was a problem hiding this comment.
unsafe recursive cache deletion, GST_ELEM_CACHE_DIR is externally configurable, then passed directly to rm -rf. An incorrect value could delete unrelated data. Keep the directory internal or validate it against the expected gst-elem-cache-$$ path before deletion.
| # True when the kernel log shows a video codec driver failing to initialise. | ||
| gstreamer_codec_hw_faulted() { | ||
| command -v dmesg >/dev/null 2>&1 || return 1 | ||
| dmesg 2>/dev/null | grep -Eqi "$GST_CODEC_FAULT_RE" |
There was a problem hiding this comment.
This bypasses scan_dmesg_errors, does not retain evidence, and repeatedly scans the live log. Capture the suite-specific snapshot once and reuse dmesg_snapshot.log or dmesg_errors.log for codec-fault classification and final checks.
gstreamer_codec_hw_faulted() only recognised a codec that had bound and
then failed to bring up its firmware. A driver that never binds at all
fails earlier than that and never prints any firmware wording:
qcom-iris aa00000.video-codec: probe with driver qcom-iris failed with error -5
That is the only codec line such a board logs. The element is absent for
the same reason as on a genuinely faulty board - the driver is not there
to register it - but the old pattern found nothing, so the encode test
reported SKIP ("Encoder not available for h264") and a dead video codec
looked like a platform that simply has no encoder.
Add the two probe-failure spellings the kernel uses. Verified against the
kernel logs of 21 hamoa-iot-evk runs: the new alternatives match only the
two boots that actually failed to probe, and add no match on any of the
other 19, including healthy boots of the same board.
Signed-off-by: Milosz Wasilewski <milosz.wasilewski@oss.qualcomm.com>
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
Maybe we have to fix even this Runner/suites/Multimedia/GSTreamer/Video/Video_Encode_Decode/run.sh:754 — final scan overwrites the snapshot used for classification
Capture through scan_dmesg_errors once and reuse the retained snapshot for codec classification and final health checks. Remove the direct dmesg fallback.
| esac | ||
| fi | ||
|
|
||
| gstreamer_run_bounded "$GST_PROBE_TIMEOUT" TERM "$GSTINSPECT" "$elem" >/dev/null 2>&1 |
There was a problem hiding this comment.
per-element caching can still exhaust the outer LAVA timeout. Cache a codec-wide unhealthy result after the first V4L2 timeout. Subsequent hardware encoder/decoder lookups should fail immediately rather than starting another gst-inspect.
| # left in there. | ||
| gstreamer_reset_element_cache() { | ||
| reset_dir="$GST_ELEM_CACHE_DIR" | ||
| case "${reset_dir##*/}" in |
There was a problem hiding this comment.
cleanup still accepts an arbitrary externally supplied directory. Keep the cache path internal or require an exact match with the process-owned ${TMPDIR:-/tmp}/gst-elem-cache-$$ path before deleting entries.
yeah, I pushed wrong branch. Will fix later today. |
3498edc to
b41d864
Compare
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
Milosz Wasilewski (@mwasilew) A few comments are still unaddressed. Once these three blockers have been addressed, Blockers: Prevent cumulative probe timeout, constrain cache ownership, and retain one authoritative dmesg snapshot. we are good to go
| # Best effort: if the cache cannot be written the probe still works, it just | ||
| # is not remembered. | ||
| mkdir -p "$GST_ELEM_CACHE_DIR" 2>/dev/null || true | ||
| if [ ! -f "$GST_ELEM_CACHE_DIR/$GST_ELEM_CACHE_MARKER" ]; then |
There was a problem hiding this comment.
marker does not establish directory ownership. Use only the internally derived ${TMPDIR:-/tmp}/gst-elem-cache-$$ path, or track the exact cache filenames created by this process.
| # module reload needs the codec state re-read too, not just the probes. Only | ||
| # the snapshot goes - the filtered error log and its history are evidence and | ||
| # are left alone, and the next capture regenerates the snapshot anyway. | ||
| rm -f "$GST_CODEC_DMESG_DIR/dmesg_snapshot.log" 2>/dev/null || true |
There was a problem hiding this comment.
cleanup deletes the retained dmesg snapshot, Separate element-cache invalidation from dmesg lifecycle. Never remove the suite snapshot during EXIT cleanup, capture it once through scan_dmesg_errors, and reuse it for classification and final checks.
gstreamer_reset_element_cache() handed GST_ELEM_CACHE_DIR to rm -rf, and the variable was read from the environment. The reset runs from the suite's EXIT trap, so a stray value turned routine cleanup into an unbounded recursive delete of whatever that path named. Derive the path internally and stop consulting the environment for it, so nothing outside this library chooses what the reset deletes; a caller that needs a different location sets TMPDIR. The reset additionally re-derives the owned path and refuses anything else, since the variable can still be reassigned after the library is sourced, and removes entries individually by the name shape has_element() writes rather than recursing, so an unrelated file in a shared TMPDIR is never a candidate. rmdir() then drops the directory only when nothing else is in it. Signed-off-by: Milosz Wasilewski <milosz.wasilewski@oss.qualcomm.com>
gstreamer_codec_hw_faulted() read the live ring buffer on every call. It
bypassed scan_dmesg_errors(), left nothing on disk showing why a codec had
been classified as faulted, and re-read the log once per probed element.
Take the snapshot once per run through scan_dmesg_errors() and classify
against that file. The capture is the run's single authoritative one: the
codec verdicts and the suite's end-of-run dmesg check now report from the
same evidence instead of scanning the buffer twice with two patterns, so
what the final check prints is what the verdicts were based on. The suite
widens GST_CODEC_MODULE_RE to the modules it cares about and points
GST_CODEC_DMESG_DIR at its own dmesg directory, so dmesg_snapshot.log and
dmesg_errors.log land with the run's other artifacts.
The snapshot has no cleanup hook on purpose. It is evidence, and the EXIT
trap that clears the element cache must not take it with it; element-cache
invalidation and the dmesg lifecycle are deliberately separate.
The module pattern has to reach up to the colon. The driver name and the
device node share one field ("qcom-iris aa00000.video-codec:"), and
scan_dmesg_errors() anchors on ^[ts] (module):, so a bare alternation never
matches. Checked against the real helper: all three fault shapes are
captured and the benign dummy-regulator line stays excluded.
scan_dmesg_errors() logs on stdout and the codec lookups run inside command
substitution, so its output goes to stderr here. On stdout it would be
spliced into the element name the caller is resolving.
Signed-off-by: Milosz Wasilewski <milosz.wasilewski@oss.qualcomm.com>
Bounding each probe individually is not enough. A wedged codec device does not wedge one element: gst-inspect hangs the same way for every element the driver would have registered, so a run that resolves an encoder and two decoders pays GST_PROBE_TIMEOUT three times over. The per-element cache does not help, because each lookup asks about a different element. That is how the outer LAVA timeout still gets consumed even though no single probe runs unbounded. Record the first outcome that means the probe could not complete - timed out, or no usable timeout(1) - as a codec-wide verdict, and answer later hardware codec lookups from it without starting another gst-inspect. Only those two outcomes are recorded: a missing element is specific to that element and says nothing about whether the next one can be probed. Measured against a signal-ignoring gst-inspect with a 3s bound, three lookups: 12s and three probes before, 4s and one probe after. The five V4L2 lookup arms were the same block of code five times over and now share one helper, which is what makes the verdict apply to all of them rather than to whichever arm was edited. Signed-off-by: Milosz Wasilewski <milosz.wasilewski@oss.qualcomm.com>
b41d864 to
11a2069
Compare
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
LGTM
1844623
into
qualcomm-linux:main
2 participants
The GStreamer video suites can hang indefinitely when the hardware video codec wedges, turning a single failed test case into a dead test run.
Observed on hamoa/x1e80100 (LAVA jobs 374345 and 377799), where the VPU throws a system error on first use:
qcom-iris aa00000.video-codec: received system error of type 0x5000003
after which every V4L2 access blocks in the kernel. Two unbounded call sites turn that into a full lava-test-shell timeout:
has_element() runs gst-inspect-1.0 with no time bound at all. It is the first thing run_encode_test() calls, and gst-inspect opens every /dev/video* node while validating the plugin registry, so it never returns. In job 374345 the run stopped there, before the pipeline was even built, and burned the entire 10 minute budget.
gstreamer_run_gstlaunch_timeout() uses
timeout --signal=INTwith no --kill-after. A gst-launch-1.0 blocked in a V4L2 ioctl never reacts to SIGINT, so timeout waits for it forever and the nominal duration + 10 bound never takes effect. This is what happened in job 377799.In both cases the remaining test definitions never execute and LAVA reports them as "missing expected test case" failures, which hides the real result: one codec broke, the rest were never tried.
Add gstreamer_run_bounded(), which always escalates to SIGKILL after GST_KILL_GRACE seconds, and use it for gst-inspect-1.0, gst-discoverer-1.0 and gst-launch-1.0. A wedged codec now surfaces as an unavailable element or a failed pipeline, and the following test cases still get to run.
SIGINT is still the first signal sent to gst-launch-1.0, so muxers keep finalising their output via -e on a healthy timeout; only a process that ignores it is escalated. Behaviour is unchanged when the hardware works.