Skip to content

aravissrc gst element switch test #918

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.

Sign up for GitHub

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

Draft
EmmanuelP wants to merge 1 commit into
base: aravis-0-8
Choose a base branch
from
+110 −1
Draft

aravissrc gst element switch test #918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ else # not Linux
endif

subdir ('src')
subdir ('tests')

viewer_enabled = false
viewer_option = get_option ('viewer')
Expand All @@ -126,6 +125,8 @@ gst_deps = aravis_dependencies + [dependency ('gstreamer-base-1.0', required: gs
dependency ('gstreamer-app-1.0', required: gst_option)]
subdir('gst', if_found: gst_deps)

subdir ('tests')

doc_deps = dependency ('gi-docgen', version:'>= 2021.1', fallback: ['gi-docgen', 'dummy_dep'], required:get_option('documentation'))
subdir('docs', if_found: doc_deps)

Expand Down
100 changes: 100 additions & 0 deletions tests/arvgsttest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/* Simple script which will start an aravis pipeline, then restart ONLY the camera. */
#include <gst/gst.h>
#include <stdio.h>
#include <unistd.h>

int
main (int argc, char *argv[])
{
GstElement *pipeline;
GstElement *source;
GstElement *sink;
GstElement *new_source;
GstState current_state, pending_state;

gst_init (&argc, &argv);

pipeline = gst_pipeline_new ("aravis_pipeline");
source = gst_element_factory_make ("aravissrc", "source");
if (!source) {
printf ("Failed to create the source element\n");
return EXIT_FAILURE;
}

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);

Comment on lines +24 to +31
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
sink = gst_element_factory_make ("fakesink", "fake_sink");

if (!pipeline || !source || !sink) {
printf ("Not all elements could be created.\n");
return EXIT_FAILURE;
}

gst_bin_add_many (GST_BIN (pipeline), source, sink, NULL);

if (!gst_element_link (source, sink)) {
printf ("Elements could not be linked.\n");
gst_object_unref (pipeline);
return EXIT_FAILURE;
}

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

Comment on lines +47 to +49
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
printf ("Wait for a few frames\n");
sleep (1);

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

Comment on lines +53 to +55
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
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));

Comment on lines +56 to +63
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
Comment on lines +61 to +63
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
printf ("Create a new source\n");
new_source = gst_element_factory_make ("aravissrc", "new_source");
if (!new_source) {
printf ("Failed to create the source element\n");
return EXIT_FAILURE;
}
g_object_set(G_OBJECT(new_source),
"camera-name", "Aravis-Fake-GV01",
"num-arv-buffers", 5,
"packet-resend", FALSE,
"packet-size", 9000,
"auto-packet-size", FALSE,
NULL);

g_assert (gst_bin_add(GST_BIN(pipeline), new_source));
g_assert (gst_element_link(new_source, sink));
g_assert (gst_element_sync_state_with_parent(new_source));

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);
Comment on lines +82 to +88
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
printf ("new source current state:%d - pending state:%d\n", current_state, pending_state);

printf ("Wait for a few frames\n");

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");
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copilot uses AI. Check for mistakes.
gst_object_unref (pipeline);

return EXIT_SUCCESS;
}
8 changes: 8 additions & 0 deletions tests/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,12 @@ if get_option('tests')
include_directories: [library_inc])
endforeach

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

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
include_directories: [library_inc])

endif
Comment on lines +106 to +112
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

endif
Loading