`aravissrc` gst element switch test by EmmanuelP · Pull Request #918 · AravisProject/aravis

EmmanuelP · 2024-08-14T07:46:19Z

This test crashes as soon as the new source element is put in PLAYING mode. But that is probably due to a mishandling of the pipeline. Doing this sort of pipeline manipulation seems not trivial: https://gstreamer.freedesktop.org/documentation/application-development/advanced/pipeline-manipulation.html?gi-language=c

Understand and fix the crash happening when new source state is set to PLAYING

This test crashes as soon as the new source element is put in PLAYING mode. But that is probably due to a mishandling of the pipeline. Doing this sort of pipeline manipulation seems not trivial: <https://gstreamer.freedesktop.org/documentation/application-development/advanced/pipeline-manipulation.html?gi-language=c>

Copilot

Pull request overview

Adds a GStreamer-based test program intended to reproduce/debug a crash when switching the aravissrc element in a running pipeline, and updates the Meson build so this test is built when the GStreamer plugin is enabled.

Changes:

Move subdir('tests') after the GStreamer plugin configuration so tests can depend on gst_enabled.
Add a new arv-gst-test executable (built only when gst_enabled is true).
Introduce tests/arvgsttest.c, a small pipeline-manipulation program that removes and re-adds an aravissrc element.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
`meson.build`	Reorders `tests` subdir inclusion to occur after GStreamer plugin setup.
`tests/meson.build`	Builds `arv-gst-test` when `gst_enabled` is true.
`tests/arvgsttest.c`	New GStreamer pipeline manipulation test for switching `aravissrc`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T20:44:37Z

tests/arvgsttest.c

+        printf ("Start the pipeline\n");
+        g_assert_cmpint (gst_element_set_state(pipeline, GST_STATE_PLAYING), ==, GST_STATE_CHANGE_ASYNC);
+


gst_element_set_state(pipeline, GST_STATE_PLAYING) does not reliably return only GST_STATE_CHANGE_ASYNC; it may return SUCCESS or NO_PREROLL depending on the pipeline. Asserting equality to ASYNC makes this test brittle. Consider checking for != GST_STATE_CHANGE_FAILURE and then waiting for the target state via gst_element_get_state() with a timeout.

Copilot · 2026-02-08T20:44:38Z

tests/arvgsttest.c

+        printf ("Stop the source\n");
+        g_assert_cmpint (gst_element_set_state(source, GST_STATE_NULL), ==, GST_STATE_CHANGE_SUCCESS);
+


gst_element_set_state(source, GST_STATE_NULL) can return GST_STATE_CHANGE_ASYNC for live/streaming elements; asserting GST_STATE_CHANGE_SUCCESS can fail even when the transition is correct. Prefer checking for failure and then waiting for the element to actually reach NULL with gst_element_get_state() using a non-zero timeout before proceeding with unlink/remove.

Copilot · 2026-02-08T20:44:38Z

tests/arvgsttest.c

+        gst_element_get_state (source, &current_state, &pending_state, 0);
+        printf ("source current state:%d - pending state:%d\n", current_state, pending_state);
+
+        sleep (1);
+
+        gst_element_unlink(source, sink);
+        g_assert (gst_bin_remove(GST_BIN(pipeline), source));
+


The pipeline is modified (unlink + gst_bin_remove) while it is still in PLAYING and without waiting for the source to fully complete its NULL transition (gst_element_get_state(..., 0) returns immediately). This makes the manipulation timing-dependent and can trigger crashes. Wait for the NULL state transition to complete (non-zero timeout) before unlinking/removing, or pause/block the data flow per GStreamer pipeline manipulation guidance.

Copilot · 2026-02-08T20:44:38Z

tests/meson.build

+        if gst_enabled
+          executable ('arv-gst-test', 'arvgsttest.c',
+          link_with: aravis_library,
+          dependencies: aravis_dependencies + gst_deps,
+          include_directories: [library_inc])
+
+        endif


Indentation in this new if gst_enabled block uses spaces and doesn't match the surrounding Meson file which otherwise uses tab-indented blocks. This makes the file harder to scan and can trip style/whitespace tooling; align the indentation with the existing foreach/if blocks above.

Copilot · 2026-02-08T20:44:38Z

tests/arvgsttest.c

+        g_object_set(G_OBJECT(source),
+                     "camera-name", "Aravis-Fake-GV01",
+                     "num-arv-buffers", 5,
+                     "packet-resend", FALSE,
+                     "packet-size", 9000,
+                     "auto-packet-size", FALSE,
+                     NULL);
+


This file uses g_object_set(G_OBJECT(...), ...) without the spacing convention used elsewhere in tests/ (e.g. g_object_set (stream, ...) in tests/arvdevicetest.c:190). Consider aligning the formatting here (spaces after function name and inside G_OBJECT (...)) to match the surrounding codebase style.

Copilot · 2026-02-08T20:44:38Z

tests/arvgsttest.c

+        gst_element_get_state (source, &current_state, &pending_state, 0);
+        printf ("new source current state:%d - pending state:%d\n", current_state, pending_state);
+
+        printf ("Start the new source\n");
+        g_assert_cmpint (gst_element_set_state(source, GST_STATE_PLAYING), ==, GST_STATE_CHANGE_SUCCESS);
+
+        gst_element_get_state (source, &current_state, &pending_state, 0);


After creating new_source, the code still queries and transitions the old source element (which has been removed from the pipeline) instead of new_source. This makes the printed state misleading and can also cause crashes/undefined behavior when setting the removed element to PLAYING. Use new_source for the state queries and when starting playback here.

Suggested change

gst_element_get_state (source, &current_state, &pending_state, 0);

printf ("new source current state:%d - pending state:%d\n", current_state, pending_state);

printf ("Start the new source\n");

g_assert_cmpint (gst_element_set_state(source, GST_STATE_PLAYING), ==, GST_STATE_CHANGE_SUCCESS);

gst_element_get_state (source, &current_state, &pending_state, 0);

gst_element_get_state (new_source, &current_state, &pending_state, 0);

printf ("new source current state:%d - pending state:%d\n", current_state, pending_state);

printf ("Start the new source\n");

g_assert_cmpint (gst_element_set_state(new_source, GST_STATE_PLAYING), ==, GST_STATE_CHANGE_SUCCESS);

gst_element_get_state (new_source, &current_state, &pending_state, 0);

Copilot · 2026-02-08T20:44:39Z

tests/meson.build

+        if gst_enabled
+          executable ('arv-gst-test', 'arvgsttest.c',
+          link_with: aravis_library,
+          dependencies: aravis_dependencies + gst_deps,


The executable is linked with dependencies: aravis_dependencies + gst_deps, but gst_deps is already defined as aravis_dependencies + [...] in the top-level meson.build. This duplicates the Aravis deps in the dependency list unnecessarily; using just gst_deps here is simpler.

Suggested change

dependencies: aravis_dependencies + gst_deps,

dependencies: gst_deps,

Copilot · 2026-02-08T20:44:39Z

tests/arvgsttest.c

+        gst_element_unlink(source, sink);
+        g_assert (gst_bin_remove(GST_BIN(pipeline), source));
+


After gst_bin_remove(..., source) you still own the original reference returned by gst_element_factory_make(). Add an explicit gst_object_unref(source)/g_clear_object(&source) after removal to avoid leaking the element.

Copilot · 2026-02-08T20:44:39Z

tests/arvgsttest.c

+        printf ("Stop the pipeline\n");
+        g_assert_cmpint (gst_element_set_state (pipeline, GST_STATE_NULL), ==, GST_STATE_CHANGE_SUCCESS);
+
+        printf ("Free the pipeline\n");


new_source is created with gst_element_factory_make() and added to the pipeline, but the local reference is never released. Even though unreffing the pipeline will drop the bin’s reference, your original reference will remain and leak unless you unref new_source (or g_clear_object(&new_source)) before exiting.

Suggested change

printf ("Free the pipeline\n");

printf ("Free the pipeline\n");

gst_object_unref (new_source);

gst_object_unref (sink);

gst_object_unref (source);

EmmanuelP marked this pull request as draft August 14, 2024 07:46

EmmanuelP changed the title ~~Wip/gst aravissrc switch test~~ aravissrc gst element switch test Aug 14, 2024

EmmanuelP mentioned this pull request Aug 14, 2024

Unable to restart camera with aravissrc gstreamer #907

Closed

EmmanuelP force-pushed the wip/gst-aravissrc-switch-test branch from 6bcdd47 to f34367a Compare August 14, 2024 13:37

EmmanuelP force-pushed the wip/gst-aravissrc-switch-test branch from 95d6118 to 89f7342 Compare February 8, 2026 20:40

EmmanuelP requested a review from Copilot February 8, 2026 20:40

Copilot started reviewing on behalf of EmmanuelP February 8, 2026 20:41 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`aravissrc` gst element switch test#918

`aravissrc` gst element switch test#918
EmmanuelP wants to merge 1 commit intoaravis-0-8from
wip/gst-aravissrc-switch-test

EmmanuelP commented Aug 14, 2024

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

		printf ("Start the pipeline\n");
		g_assert_cmpint (gst_element_set_state(pipeline, GST_STATE_PLAYING), ==, GST_STATE_CHANGE_ASYNC);

		printf ("Stop the source\n");
		g_assert_cmpint (gst_element_set_state(source, GST_STATE_NULL), ==, GST_STATE_CHANGE_SUCCESS);

	dependencies: aravis_dependencies + gst_deps,
	dependencies: gst_deps,

		gst_element_unlink(source, sink);
		g_assert (gst_bin_remove(GST_BIN(pipeline), source));

-        printf ("Free the pipeline\n");
+        printf ("Free the pipeline\n");
+        gst_object_unref (new_source);
+        gst_object_unref (sink);
+        gst_object_unref (source);

Uh oh!

Conversation

EmmanuelP commented Aug 14, 2024

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments