| Message ID | 20221118162129.22866-1-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. 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. > > Fixes: ccfe0a1af77c ("gstreamer: Provide framerate support for libcamerasrc") > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Umang Jain <umang.jain@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; > -- > 2.38.1 >
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@jmondi.org> > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@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;
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;
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. Fixes: ccfe0a1af77c ("gstreamer: Provide framerate support for libcamerasrc") Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/gstreamer/gstlibcamera-utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.38.1