<div dir="ltr">Okay, sounds good.<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 10, 2021 at 4:38 PM Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Vedant,<br>
<br>
On 10/08/2021 11:56, Vedant Paranjape wrote:<br>
> Hi Kieran,<br>
> Thanks for the review.<br>
> <br>
> On Tue, Aug 10, 2021 at 3:35 PM Kieran Bingham<br>
> <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a><br>
> <mailto:<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>>> wrote:<br>
> <br>
>     Hi Vedant,<br>
> <br>
>     Thank you for this, I'm very glad to see gstreamer tests being<br>
>     introduced!<br>
> <br>
> <br>
>     On 10/08/2021 10:38, <a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a><br>
>     <mailto:<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>> wrote:<br>
> <br>
>     <snip><br>
> <br>
>     >> --- /dev/null<br>
>     >> +++ b/test/gstreamer/meson.build<br>
>     >> @@ -0,0 +1,15 @@<br>
>     >> +# SPDX-License-Identifier: CC0-1.0<br>
>     >> +<br>
>     >> +gstreamer_tests = [<br>
>     >> +    ['single_stream_test',   'gstreamer_single_stream_test.cpp'],<br>
>     >> +]<br>
>     >> +gstreamer_dep = dependency('gstreamer-1.0', required: true)<br>
> <br>
>     instead of required: true, we should depend upon whether we are built<br>
>     with gstreamer_support.<br>
> <br>
>     I think this would do it, but please check/test.<br>
> <br>
>     gstreamer_dep =<br>
>      dependency('gstreamer-1.0', required: get_option('gstreamer'))<br>
> <br>
>     And then we would want to skip all tests from being built if there is no<br>
>     gstreamer:<br>
> <br>
> <br>
>     if not gstreamer_dep.found()gstallocator_dep.found()<br>
>         subdir_done()<br>
>     endif<br>
> <br>
> <br>
> This is needs to be added by my patch ? or it will added sometime in<br>
> future ?<br>
<br>
Your patch should add it.<br>
<br>
But Laurent's suggestion is subtly different and better.<br>
<br>
So you should start the meson.build by checking:<br>
<br>
if not gst_enabled<br>
        subdir_done()<br>
endif<br>
<br>
<br>
Then your<br>
<br>
        gstreamer_dep = dependency('gstreamer-1.0', required: true)<br>
<br>
line can stay as it is, because if gst_enabled is true, you /do/ require<br>
the gstreamer_dep anyway .<br>
<br>
So it's just Lauren'ts three lines to add to the beginning of the<br>
meson.build.<br>
<br>
<br>
--<br>
Kieran<br>
<br>
> <br>
>     I would put the dependency checking, and this subdir_done() addition at<br>
>     the beginning, before even listing the tests.<br>
> <br>
>     --<br>
>     Kieran<br>
> <br>
> <br>
>     >> +<br>
>     >> +foreach t : gstreamer_tests<br>
>     >> +    exe = executable(t[0], t[1],<br>
>     >> +                     dependencies : [libcamera_private,<br>
>     gstreamer_dep],<br>
>     >> +                     link_with : test_libraries,<br>
>     >> +                     include_directories : test_includes_internal)<br>
>     >> +<br>
>     >> +    test(t[0], exe, suite : 'gstreamer', is_parallel : false)<br>
>     >> +endforeach<br>
>     >> diff --git a/test/meson.build b/test/meson.build<br>
>     >> index 3bceb5df..d0466f17 100644<br>
>     >> --- a/test/meson.build<br>
>     >> +++ b/test/meson.build<br>
>     >> @@ -11,6 +11,7 @@ subdir('libtest')<br>
>     >> <br>
>     >>  subdir('camera')<br>
>     >>  subdir('controls')<br>
>     >> +subdir('gstreamer')<br>
>     >>  subdir('ipa')<br>
>     >>  subdir('ipc')<br>
>     >>  subdir('log')<br>
>     >> --<br>
>     >> 2.25.1<br>
>     >><br>
> <br>
</blockquote></div>