[libcamera-devel] [PATCH] test: gstreamer: Fix failure of gstreamer_multistream_test
Umang Jain
umang.jain at ideasonboard.com
Mon Sep 12 17:57:09 CEST 2022
Hi,
On 9/12/22 6:29 PM, Vedant Paranjape wrote:
> 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 ?
I can handle it while pushing :)
>
>>> 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 ?
C++ has already given that with protected: access specifier, I think
Usually a helper function like that comes when member is private and
needs to set from outside the class. In this case, (future) gstreamer
tests inherit from this class, so they can set it cameraName_ on their
own (if they want to ... )
>
>> 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