[libcamera-devel] [PATCH] test: gstreamer: Fix failure of gstreamer_multistream_test
Vedant Paranjape
vedantparanjape160201 at gmail.com
Mon Sep 12 14:59:29 CEST 2022
Hello Umang,
Thanks for the review.
On Sat, Sep 10, 2022 at 7:59 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> Hi Vedant,
>
> On 9/10/22 12:07 PM, Vedant Paranjape wrote:
> > Multistream test failed with the following logs, to run on Raspberry Pi 4 due
> > to a bug introduced in one of the recent patches refactoring the code
> > that fails to set the camera-name property with a valid camera id
> > string.
>
> Ouch, I am not sure how it slipped since I think it was still passing
> for me when I was working on that patch! It might have had happen, my
> camera didn't support multi-stream and it got skipped :S
A precommit CI would be quite useful, as this had happened to me as well.
> >
> > [5:04:05.990569833] [16751] INFO IPAManager ipa_manager.cpp:141 libcamera is not installed. Adding '/home/libcamera/build/src/ipa' to the IPA search path
> > [5:04:05.994751009] [16751] INFO Camera camera_manager.cpp:293 libcamera v0.0.0+3904-560ceb1e-dirty (2022-09-10T03:53:40+05:30)
> > [5:04:06.058019483] [16752] INFO IPAProxy ipa_proxy.cpp:130 libcamera is not installed. Loading IPA configuration from '/home/libcamera/src/ipa/raspberrypi/data'
> > [5:04:06.085169830] [16752] INFO RPI raspberrypi.cpp:1374 Registered camera /base/soc/i2c0mux/i2c at 1/ov5647 at 36 to Unicam device /dev/media4 and ISP device /dev/media1
> > 0:00:00.178829295 16751 0x1d31e00 DEBUG libcamerasrc gstlibcamerasrc.cpp:763:gst_libcamera_src_request_new_pad:<libcamera> new request pad created
> > 0:00:00.179392613 16751 0x1d31e00 DEBUG libcamerasrc gstlibcamerasrc.cpp:327:gst_libcamera_src_open:<libcamera> Opening camera device ...
> > [5:04:06.094149760] [16751] INFO IPAManager ipa_manager.cpp:141 libcamera is not installed. Adding '/home/libcamera/build/src/ipa' to the IPA search path
> > [5:04:06.097366114] [16751] INFO Camera camera_manager.cpp:293 libcamera v0.0.0+3904-560ceb1e-dirty (2022-09-10T03:53:40+05:30)
> > [5:04:06.152332645] [16755] INFO IPAProxy ipa_proxy.cpp:130 libcamera is not installed. Loading IPA configuration from '/home/libcamera/src/ipa/raspberrypi/data'
> > [5:04:06.173016319] [16755] INFO RPI raspberrypi.cpp:1374 Registered camera /base/soc/i2c0mux/i2c at 1/ov5647 at 36 to Unicam device /dev/media4 and ISP device /dev/media1
> > 0:00:00.259832923 16751 0x1d31e00 WARN libcamerasrc gstlibcamerasrc.cpp:347:gst_libcamera_src_open:<libcamera> error: Could not find a camera named ''.
> > 0:00:00.259883405 16751 0x1d31e00 WARN libcamerasrc gstlibcamerasrc.cpp:347:gst_libcamera_src_open:<libcamera> error: libcamera::CameraMananger::get() returned nullptr
> > Unable to set the pipeline to the playing state.
>
> I think the last three is what's relevant and I would strip other log
> lines here
Sure, could you please edit the commit message while pushing the
change, or should I send a v2 ?
> > 0:00:00.262787739 16751 0x1d31e00 DEBUG libcamerasrc gstlibcamerasrc.cpp:786:gst_libcamera_src_release_pad:<libcamera> Pad <libcamera:libcamerapad0> being released
> >
> > This patch assigns the camera->id() to the variable cameraName_ that is
> > later used to set element property "camera-name" needed to call the
> > specific camera which supports multistreams. Move the code to set
> > element property "camera-name" to base class GstreamerTest.
> >
> > Fixes: 5646849b59fe ("test: gstreamer: Check availability of cameras before running")
> > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > ---
> > test/gstreamer/gstreamer_multi_stream_test.cpp | 3 ---
> > test/gstreamer/gstreamer_test.cpp | 6 ++++--
> > test/gstreamer/gstreamer_test.h | 3 ++-
> > 3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp
> > index b8387c10c65e..cd669308d171 100644
> > --- a/test/gstreamer/gstreamer_multi_stream_test.cpp
> > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp
> > @@ -70,8 +70,6 @@ protected:
> >
> > int run() override
> > {
> > - g_object_set(libcameraSrc_, "camera-name", cameraName_.c_str(), NULL);
> > -
> > /* Build the pipeline */
> > gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,
> > stream0_, stream1_, NULL);
> > @@ -106,7 +104,6 @@ protected:
> > }
> >
> > private:
> > - std::string cameraName_;
> > GstElement *stream0_;
> > GstElement *stream1_;
> > };
> > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp
> > index 4947b7bb2977..fe8e8e157b5b 100644
> > --- a/test/gstreamer/gstreamer_test.cpp
> > +++ b/test/gstreamer/gstreamer_test.cpp
> > @@ -73,7 +73,7 @@ GstreamerTest::GstreamerTest(unsigned int numStreams)
> > * Atleast one camera should be available with numStreams streams,
> > * otherwise skip the test entirely.
> > */
> > - if (!checkMinCameraStreams(numStreams)) {
> > + if (!checkMinCameraStreamsAndSetCameraName(numStreams)) {
> > status_ = TestSkip;
> > return;
> > }
> > @@ -81,7 +81,7 @@ GstreamerTest::GstreamerTest(unsigned int numStreams)
> > status_ = TestPass;
> > }
> >
> > -bool GstreamerTest::checkMinCameraStreams(unsigned int numStreams)
> > +bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStreams)
> > {
> > libcamera::CameraManager cm;
> > bool cameraFound = false;
> > @@ -93,6 +93,7 @@ bool GstreamerTest::checkMinCameraStreams(unsigned int numStreams)
> > continue;
> >
> > cameraFound = true;
> > + cameraName_ = camera->id();
> > break;
> > }
> >
> > @@ -121,6 +122,7 @@ int GstreamerTest::createPipeline()
> > return TestFail;
> > }
> >
> > + g_object_set(libcameraSrc_, "camera-name", cameraName_.c_str(), NULL);
> > g_object_ref_sink(libcameraSrc_);
> >
> > return TestPass;
> > diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h
> > index 6f277cc5d8b2..aa2261e2dd5c 100644
> > --- a/test/gstreamer/gstreamer_test.h
> > +++ b/test/gstreamer/gstreamer_test.h
> > @@ -24,10 +24,11 @@ protected:
> > int processEvent();
> > void printError(GstMessage *msg);
> >
> > + std::string cameraName_;
>
> Looks good, in the sense if any test inheriting from GStreamerTest would
> like to override cameraName_, it would be free to do so as this is
> protected..
maybe a helper function to set the name might make it neat ? or it's
overengineering ?
>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>
> > GstElement *pipeline_;
> > GstElement *libcameraSrc_;
> > int status_;
> >
> > private:
> > - bool checkMinCameraStreams(unsigned int numStreams);
> > + bool checkMinCameraStreamsAndSetCameraName(unsigned int numStreams);
> > };
>
Regards,
Vedant
More information about the libcamera-devel
mailing list