[libcamera-devel] [PATCH] test: gstreamer: Fix indentation in comments

[Laurent Pinchart]

Hi Nicolas,

On Thu, Dec 07, 2023 at 10:43:02AM -0500, Nicolas Dufresne wrote:
> Le mercredi 06 décembre 2023 à 00:49 +0000, Kieran Bingham via libcamera-devel a écrit :
> > Quoting Laurent Pinchart via libcamera-devel (2023-12-06 00:44:25)
> > > A couple of comments are mis-indented in the gstreamer unit test. Fix
> > > them, and reflow the text while at it.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > I think that's straightforward enough to just merge already.
> 
> Ack. It highlights the code though, which I wasn't aware of. Seems generally a
> bad idea to always disable the protection again a possibly broken/crashing
> plugin init in the system. I might suggest to find a way to only enable it for
> specific "ASAN" issue workaround.

Disabling the fork doesn't disable any protection, so I don't see any
issue there. I may be missing something though.

Are you talking about the code that removes libgstlibcamera.so from the
registry ? The point of that is to make sure we test the plugin from the
build directory, and don't pick a plugin from the system by mistake.

Or are you talking about the __asan_default_options() function in the
same file ? I would like to drop that indeed.

> > > ---
> > >  test/gstreamer/gstreamer_test.cpp | 23 +++++++++++------------
> > >  1 file changed, 11 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp
> > > index 6ad0c15cd0e2..091f7bf70288 100644
> > > --- a/test/gstreamer/gstreamer_test.cpp
> > > +++ b/test/gstreamer/gstreamer_test.cpp
> > > @@ -31,15 +31,14 @@ GstreamerTest::GstreamerTest(unsigned int numStreams)
> > >         : pipeline_(nullptr), libcameraSrc_(nullptr)
> > >  {
> > >         /*
> > > -       * GStreamer by default spawns a process to run the
> > > -       * gst-plugin-scanner helper. If libcamera is compiled with ASan
> > > -       * enabled, and as GStreamer is most likely not, this causes the
> > > -       * ASan link order check to fail when gst-plugin-scanner
> > > -       * dlopen()s the plugin as many libraries will have already been
> > > -       * loaded by then. Fix this issue by disabling spawning of a
> > > -       * child helper process when scanning the build directory for
> > > -       * plugins.
> > > -       */
> > > +        * GStreamer by default spawns a process to run the gst-plugin-scanner
> > > +        * helper. If libcamera is compiled with ASan enabled, and as GStreamer
> > > +        * is most likely not, this causes the ASan link order check to fail
> > > +        * when gst-plugin-scanner dlopen()s the plugin as many libraries will
> > > +        * have already been loaded by then. Fix this issue by disabling
> > > +        * spawning of a child helper process when scanning the build directory
> > > +        * for plugins.
> > > +        */
> > >         gst_registry_fork_set_enabled(false);
> > >  
> > >         /* Initialize GStreamer */
> > > @@ -53,9 +52,9 @@ GstreamerTest::GstreamerTest(unsigned int numStreams)
> > >         }
> > >  
> > >         /*
> > > -       * Remove the system libcamera plugin, if any, and add the
> > > -       * plugin from the build directory.
> > > -       */
> > > +        * Remove the system libcamera plugin, if any, and add the plugin from
> > > +        * the build directory.
> > > +        */
> > >         GstRegistry *registry = gst_registry_get();
> > >         g_autoptr(GstPlugin) plugin = gst_registry_lookup(registry, "libgstlibcamera.so");
> > >         if (plugin)
> > > 
> > > base-commit: 4eba2dc73c096d037a8a6390ff4a91ebbf1cedd4

-- 
Regards,

Laurent Pinchart