| Message ID | 20230704155329.28734-1-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi, Le mardi 04 juillet 2023 à 17:53 +0200, Umang Jain a écrit : > Test the enumeration of the cameras through GstDeviceProvider against > the libcamera camera manager. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > test/gstreamer/gstreamer_test.cpp | 52 +++++++++++++++++++++++++++++++ > test/gstreamer/gstreamer_test.h | 1 + > 2 files changed, 53 insertions(+) > > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp > index 6ad0c15c..f94f3fd9 100644 > --- a/test/gstreamer/gstreamer_test.cpp > +++ b/test/gstreamer/gstreamer_test.cpp > @@ -27,6 +27,18 @@ const char *__asan_default_options() > } > } > > +static GstDeviceMonitor * > +libcamerasrc_get_device_provider() > +{ > + GstDeviceMonitor *monitor = gst_device_monitor_new(); > + > + gst_device_monitor_add_filter(monitor, "Video/Source", NULL); > + > + gst_device_monitor_start(monitor); > + > + return monitor; > +} > + > GstreamerTest::GstreamerTest(unsigned int numStreams) > : pipeline_(nullptr), libcameraSrc_(nullptr) > { > @@ -78,6 +90,12 @@ GstreamerTest::GstreamerTest(unsigned int numStreams) > return; > } > > + if (!checkCameraEnumeration()) { > + g_printerr("Failed to enumerate cameras on GstDeviceProvider\n"); > + status_ = TestFail; > + return; > + } > + > status_ = TestPass; > } > > @@ -102,6 +120,40 @@ bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStream > return cameraFound; > } > > +bool GstreamerTest::checkCameraEnumeration() > +{ > + GstDeviceMonitor *monitor; > + GList *devices, *l; > + std::vector<const char *> cameraNames; > + std::unique_ptr<libcamera::CameraManager> cm > + = std::make_unique<libcamera::CameraManager>(); > + > + cm->start(); > + for (auto &camera : cm->cameras()) > + cameraNames.push_back(strdup(camera->id().c_str())); > + cm->stop(); > + cm.reset(); > + > + monitor = libcamerasrc_get_device_provider(); > + devices = gst_device_monitor_get_devices(monitor); > + > + for (l = devices; l != NULL; l = g_list_next(l)) { > + bool matched = false; > + GstDevice *device = GST_DEVICE(l->data); > + for (const gchar *name : cameraNames) { > + if (strcmp(name, gst_device_get_display_name(device)) == 0) { > + matched = true; > + break; > + } Just to note that its a bug that gst_device_get_display_name() returns the camera->id() (which was initially set to the human friendly name). In order to avoid testing a bug, I'd probably go with: g_autofree gchar *gst_name; g_autoptr(GstElement) element = gst_device_create_element (device); g_object_get(element, "camera-name", &gst_name, NULL); if (strcmp(name, gst_name) == 0) { matched = true; break; } Further in the future, I'd be happy if camera-name, becomes read-only friendly name, and camera-id becomes the configurable property. The naming have not been ported wiht the changes in libcamera. With this change: Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > + } > + > + if (!matched) > + return false; > + } > + > + return true; > +} > + > GstreamerTest::~GstreamerTest() > { > g_clear_object(&pipeline_); > diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h > index aa2261e2..3479c7c4 100644 > --- a/test/gstreamer/gstreamer_test.h > +++ b/test/gstreamer/gstreamer_test.h > @@ -31,4 +31,5 @@ protected: > > private: > bool checkMinCameraStreamsAndSetCameraName(unsigned int numStreams); > + bool checkCameraEnumeration(); > };
Hi Nicolas On 7/4/23 8:05 PM, Nicolas Dufresne wrote: > Hi, > > Le mardi 04 juillet 2023 à 17:53 +0200, Umang Jain a écrit : >> Test the enumeration of the cameras through GstDeviceProvider against >> the libcamera camera manager. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> test/gstreamer/gstreamer_test.cpp | 52 +++++++++++++++++++++++++++++++ >> test/gstreamer/gstreamer_test.h | 1 + >> 2 files changed, 53 insertions(+) >> >> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp >> index 6ad0c15c..f94f3fd9 100644 >> --- a/test/gstreamer/gstreamer_test.cpp >> +++ b/test/gstreamer/gstreamer_test.cpp >> @@ -27,6 +27,18 @@ const char *__asan_default_options() >> } >> } >> >> +static GstDeviceMonitor * >> +libcamerasrc_get_device_provider() >> +{ >> + GstDeviceMonitor *monitor = gst_device_monitor_new(); >> + >> + gst_device_monitor_add_filter(monitor, "Video/Source", NULL); >> + >> + gst_device_monitor_start(monitor); >> + >> + return monitor; >> +} >> + >> GstreamerTest::GstreamerTest(unsigned int numStreams) >> : pipeline_(nullptr), libcameraSrc_(nullptr) >> { >> @@ -78,6 +90,12 @@ GstreamerTest::GstreamerTest(unsigned int numStreams) >> return; >> } >> >> + if (!checkCameraEnumeration()) { >> + g_printerr("Failed to enumerate cameras on GstDeviceProvider\n"); >> + status_ = TestFail; >> + return; >> + } >> + >> status_ = TestPass; >> } >> >> @@ -102,6 +120,40 @@ bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStream >> return cameraFound; >> } >> >> +bool GstreamerTest::checkCameraEnumeration() >> +{ >> + GstDeviceMonitor *monitor; >> + GList *devices, *l; >> + std::vector<const char *> cameraNames; >> + std::unique_ptr<libcamera::CameraManager> cm >> + = std::make_unique<libcamera::CameraManager>(); >> + >> + cm->start(); >> + for (auto &camera : cm->cameras()) >> + cameraNames.push_back(strdup(camera->id().c_str())); >> + cm->stop(); >> + cm.reset(); >> + >> + monitor = libcamerasrc_get_device_provider(); >> + devices = gst_device_monitor_get_devices(monitor); >> + >> + for (l = devices; l != NULL; l = g_list_next(l)) { >> + bool matched = false; >> + GstDevice *device = GST_DEVICE(l->data); >> + for (const gchar *name : cameraNames) { >> + if (strcmp(name, gst_device_get_display_name(device)) == 0) { >> + matched = true; >> + break; >> + } > Just to note that its a bug that gst_device_get_display_name() returns the > camera->id() (which was initially set to the human friendly name). In order to We have discussed human friendly names in the past and we didn't find a conclusion since they are not static across reboots. So it's a problem in itself that we need to look into. For cam utility, we parse it differently based on location and/or model [1] [1] https://git.libcamera.org/libcamera/libcamera.git/tree/src/apps/cam/main.cpp#n298 > avoid testing a bug, I'd probably go with: > > g_autofree gchar *gst_name; > g_autoptr(GstElement) element = gst_device_create_element (device); > g_object_get(element, "camera-name", &gst_name, NULL); > if (strcmp(name, gst_name) == 0) { matched = true; > break; > } > > Further in the future, I'd be happy if camera-name, becomes read-only friendly > name, and camera-id becomes the configurable property. The naming have not been > ported wiht the changes in libcamera. > > With this change: > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > >> + } >> + >> + if (!matched) >> + return false; >> + } >> + >> + return true; >> +} >> + >> GstreamerTest::~GstreamerTest() >> { >> g_clear_object(&pipeline_); >> diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h >> index aa2261e2..3479c7c4 100644 >> --- a/test/gstreamer/gstreamer_test.h >> +++ b/test/gstreamer/gstreamer_test.h >> @@ -31,4 +31,5 @@ protected: >> >> private: >> bool checkMinCameraStreamsAndSetCameraName(unsigned int numStreams); >> + bool checkCameraEnumeration(); >> };
Hi 2023. július 4., kedd 17:53 keltezéssel, Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org> írta: > Test the enumeration of the cameras through GstDeviceProvider against > the libcamera camera manager. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > test/gstreamer/gstreamer_test.cpp | 52 +++++++++++++++++++++++++++++++ > test/gstreamer/gstreamer_test.h | 1 + > 2 files changed, 53 insertions(+) > > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp > index 6ad0c15c..f94f3fd9 100644 > --- a/test/gstreamer/gstreamer_test.cpp > +++ b/test/gstreamer/gstreamer_test.cpp > @@ -27,6 +27,18 @@ const char *__asan_default_options() > } > } > > +static GstDeviceMonitor * > +libcamerasrc_get_device_provider() > +{ > + GstDeviceMonitor *monitor = gst_device_monitor_new(); > + > + gst_device_monitor_add_filter(monitor, "Video/Source", NULL); > + > + gst_device_monitor_start(monitor); > + > + return monitor; > +} > + > GstreamerTest::GstreamerTest(unsigned int numStreams) > : pipeline_(nullptr), libcameraSrc_(nullptr) > { > @@ -78,6 +90,12 @@ GstreamerTest::GstreamerTest(unsigned int numStreams) > return; > } > > + if (!checkCameraEnumeration()) { > + g_printerr("Failed to enumerate cameras on GstDeviceProvider\n"); > + status_ = TestFail; > + return; > + } > + > status_ = TestPass; > } > > @@ -102,6 +120,40 @@ bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStream > return cameraFound; > } > > +bool GstreamerTest::checkCameraEnumeration() > +{ > + GstDeviceMonitor *monitor; > + GList *devices, *l; > + std::vector<const char *> cameraNames; > + std::unique_ptr<libcamera::CameraManager> cm > + = std::make_unique<libcamera::CameraManager>(); > + > + cm->start(); > + for (auto &camera : cm->cameras()) > + cameraNames.push_back(strdup(camera->id().c_str())); > [...] As far as I can tell these strings will be leaked. Why not use `std::string`? Regards, Barnabás Pőcze
On Tue, 2023-07-04 at 17:53 +0200, Umang Jain via libcamera-devel wrote: > Test the enumeration of the cameras through GstDeviceProvider against > the libcamera camera manager. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > test/gstreamer/gstreamer_test.cpp | 52 +++++++++++++++++++++++++++++++ > test/gstreamer/gstreamer_test.h | 1 + > 2 files changed, 53 insertions(+) > > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp > index 6ad0c15c..f94f3fd9 100644 > --- a/test/gstreamer/gstreamer_test.cpp > +++ b/test/gstreamer/gstreamer_test.cpp > @@ -27,6 +27,18 @@ const char *__asan_default_options() > } > } > > +static GstDeviceMonitor * > +libcamerasrc_get_device_provider() > +{ > + GstDeviceMonitor *monitor = gst_device_monitor_new(); > + > + gst_device_monitor_add_filter(monitor, "Video/Source", NULL); > + > + gst_device_monitor_start(monitor); > + > + return monitor; > +} You may instead want to do something like to make sure your testing the provider you actually want to test: GstDeviceProviderFactory *factory = gst_device_provider_factory_find ("libcameraprovider"); GstDeviceProvider *provider = gst_device_provider_factory_get (fact); gst_object_unref(fact); GList *devices = gst_device_provider_get_devices(provider); // validate devices g_list_free_full (devices, gst_object_unref); gst_object_unref(provider); Olivier > + > GstreamerTest::GstreamerTest(unsigned int numStreams) > : pipeline_(nullptr), libcameraSrc_(nullptr) > { > @@ -78,6 +90,12 @@ GstreamerTest::GstreamerTest(unsigned int numStreams) > return; > } > > + if (!checkCameraEnumeration()) { > + g_printerr("Failed to enumerate cameras on GstDeviceProvider\n"); > + status_ = TestFail; > + return; > + } > + > status_ = TestPass; > } > > @@ -102,6 +120,40 @@ bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStream > return cameraFound; > } > > +bool GstreamerTest::checkCameraEnumeration() > +{ > + GstDeviceMonitor *monitor; > + GList *devices, *l; > + std::vector<const char *> cameraNames; > + std::unique_ptr<libcamera::CameraManager> cm > + = std::make_unique<libcamera::CameraManager>(); > + > + cm->start(); > + for (auto &camera : cm->cameras()) > + cameraNames.push_back(strdup(camera->id().c_str())); > + cm->stop(); > + cm.reset(); > + > + monitor = libcamerasrc_get_device_provider(); > + devices = gst_device_monitor_get_devices(monitor); > + > + for (l = devices; l != NULL; l = g_list_next(l)) { > + bool matched = false; > + GstDevice *device = GST_DEVICE(l->data); > + for (const gchar *name : cameraNames) { > + if (strcmp(name, gst_device_get_display_name(device)) == 0) { > + matched = true; > + break; > + } > + } > + > + if (!matched) > + return false; > + } > + > + return true; > +} > + > GstreamerTest::~GstreamerTest() > { > g_clear_object(&pipeline_); > diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h > index aa2261e2..3479c7c4 100644 > --- a/test/gstreamer/gstreamer_test.h > +++ b/test/gstreamer/gstreamer_test.h > @@ -31,4 +31,5 @@ protected: > > private: > bool checkMinCameraStreamsAndSetCameraName(unsigned int numStreams); > + bool checkCameraEnumeration(); > };
Hi Barnabás, On 7/4/23 8:50 PM, Barnabás Pőcze wrote: > Hi > > > 2023. július 4., kedd 17:53 keltezéssel, Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org> írta: > >> Test the enumeration of the cameras through GstDeviceProvider against >> the libcamera camera manager. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> test/gstreamer/gstreamer_test.cpp | 52 +++++++++++++++++++++++++++++++ >> test/gstreamer/gstreamer_test.h | 1 + >> 2 files changed, 53 insertions(+) >> >> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp >> index 6ad0c15c..f94f3fd9 100644 >> --- a/test/gstreamer/gstreamer_test.cpp >> +++ b/test/gstreamer/gstreamer_test.cpp >> @@ -27,6 +27,18 @@ const char *__asan_default_options() >> } >> } >> >> +static GstDeviceMonitor * >> +libcamerasrc_get_device_provider() >> +{ >> + GstDeviceMonitor *monitor = gst_device_monitor_new(); >> + >> + gst_device_monitor_add_filter(monitor, "Video/Source", NULL); >> + >> + gst_device_monitor_start(monitor); >> + >> + return monitor; >> +} >> + >> GstreamerTest::GstreamerTest(unsigned int numStreams) >> : pipeline_(nullptr), libcameraSrc_(nullptr) >> { >> @@ -78,6 +90,12 @@ GstreamerTest::GstreamerTest(unsigned int numStreams) >> return; >> } >> >> + if (!checkCameraEnumeration()) { >> + g_printerr("Failed to enumerate cameras on GstDeviceProvider\n"); >> + status_ = TestFail; >> + return; >> + } >> + >> status_ = TestPass; >> } >> >> @@ -102,6 +120,40 @@ bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStream >> return cameraFound; >> } >> >> +bool GstreamerTest::checkCameraEnumeration() >> +{ >> + GstDeviceMonitor *monitor; >> + GList *devices, *l; >> + std::vector<const char *> cameraNames; >> + std::unique_ptr<libcamera::CameraManager> cm >> + = std::make_unique<libcamera::CameraManager>(); >> + >> + cm->start(); >> + for (auto &camera : cm->cameras()) >> + cameraNames.push_back(strdup(camera->id().c_str())); >> [...] > As far as I can tell these strings will be leaked. Why not use `std::string`? ik, yes these are leaked, thanks for noticing. > > > Regards, > Barnabás Pőcze
diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp index 6ad0c15c..f94f3fd9 100644 --- a/test/gstreamer/gstreamer_test.cpp +++ b/test/gstreamer/gstreamer_test.cpp @@ -27,6 +27,18 @@ const char *__asan_default_options() } } +static GstDeviceMonitor * +libcamerasrc_get_device_provider() +{ + GstDeviceMonitor *monitor = gst_device_monitor_new(); + + gst_device_monitor_add_filter(monitor, "Video/Source", NULL); + + gst_device_monitor_start(monitor); + + return monitor; +} + GstreamerTest::GstreamerTest(unsigned int numStreams) : pipeline_(nullptr), libcameraSrc_(nullptr) { @@ -78,6 +90,12 @@ GstreamerTest::GstreamerTest(unsigned int numStreams) return; } + if (!checkCameraEnumeration()) { + g_printerr("Failed to enumerate cameras on GstDeviceProvider\n"); + status_ = TestFail; + return; + } + status_ = TestPass; } @@ -102,6 +120,40 @@ bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStream return cameraFound; } +bool GstreamerTest::checkCameraEnumeration() +{ + GstDeviceMonitor *monitor; + GList *devices, *l; + std::vector<const char *> cameraNames; + std::unique_ptr<libcamera::CameraManager> cm + = std::make_unique<libcamera::CameraManager>(); + + cm->start(); + for (auto &camera : cm->cameras()) + cameraNames.push_back(strdup(camera->id().c_str())); + cm->stop(); + cm.reset(); + + monitor = libcamerasrc_get_device_provider(); + devices = gst_device_monitor_get_devices(monitor); + + for (l = devices; l != NULL; l = g_list_next(l)) { + bool matched = false; + GstDevice *device = GST_DEVICE(l->data); + for (const gchar *name : cameraNames) { + if (strcmp(name, gst_device_get_display_name(device)) == 0) { + matched = true; + break; + } + } + + if (!matched) + return false; + } + + return true; +} + GstreamerTest::~GstreamerTest() { g_clear_object(&pipeline_); diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h index aa2261e2..3479c7c4 100644 --- a/test/gstreamer/gstreamer_test.h +++ b/test/gstreamer/gstreamer_test.h @@ -31,4 +31,5 @@ protected: private: bool checkMinCameraStreamsAndSetCameraName(unsigned int numStreams); + bool checkCameraEnumeration(); };
Test the enumeration of the cameras through GstDeviceProvider against the libcamera camera manager. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- test/gstreamer/gstreamer_test.cpp | 52 +++++++++++++++++++++++++++++++ test/gstreamer/gstreamer_test.h | 1 + 2 files changed, 53 insertions(+)