[libcamera-devel] [PATCH] test: gstreamer: Fix failure of gstreamer_multistream_test

[Umang Jain]

CC Rishi

Hi, Can you please review and add your tags here, I guess you have tested ?

On 9/10/22 7:59 PM, Umang Jain 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
>>
>> [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
>> 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..
>
> 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);
>>   };
>