[libcamera-devel] [PATCH] gstreamer: Do not lookup controls by id

[Laurent Pinchart]

Hello,

On Fri, Nov 18, 2022 at 11:59:01PM +0530, Umang Jain wrote:
> On 11/18/22 9:51 PM, Jacopo Mondi via libcamera-devel wrote:
> > The libcamerasrc element looks for the availability of the
> > FrameDurationLimits control by looking it up by numeric control id.
> >
> > The ControlinfoMap::find(unsigned int i) function searches the control
> > numerical identifier on the ContorlInfoMap::idMap_ class member, which
> > might be not initialized if the pipeline handler does not register
> > any control, causing an invalid memory access.
> >
> > Avoid looking up the control by numerical id and use the ControlId
> > instance instead to prevent that.
> 
> Nice spot.
> 
> I wonder if it's specific to ISI Pipeline-handler (probably yes) that 
> you hit this non-initialization situation.

It's because the ISI pipeline handler doesn't expose any control. That's
not supposed to be a common situation, and will likely be fixed, but
it's still better to use find(Control *) than find(int).

> > Fixes: ccfe0a1af77c ("gstreamer: Provide framerate support for libcamerasrc")
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > ---
> >   src/gstreamer/gstlibcamera-utils.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > index 64d197010008..36b9564c4cb4 100644
> > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > @@ -436,7 +436,7 @@ void gst_libcamera_clamp_and_set_frameduration(ControlList &initCtrls,
> >   	if (!gst_structure_has_field_typed(element_caps, "framerate", GST_TYPE_FRACTION))
> >   		return;
> >
> > -	auto iterFrameDuration = cam_ctrls.find(controls::FrameDurationLimits.id());
> > +	auto iterFrameDuration = cam_ctrls.find(&controls::FrameDurationLimits);
> >   	if (iterFrameDuration == cam_ctrls.end()) {
> >   		GST_WARNING("FrameDurationLimits not found in camera controls.");
> >   		return;

-- 
Regards,

Laurent Pinchart