[v3,2/4] libcamera: converter_v4l2_m2m: Support crop selection
diff mbox series

Message ID 20240625062327.50940-3-umang.jain@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: rkisp1: Plumb the ConverterDW100 converter
Related show

Commit Message

Umang Jain June 25, 2024, 6:23 a.m. UTC 
Add a helper to set selection rectangle on the video node
to support cropping and scaling capabilites.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 .../internal/converter/converter_v4l2_m2m.h   |  5 ++++
 .../converter/converter_v4l2_m2m.cpp          | 27 +++++++++++++++++++
 2 files changed, 32 insertions(+)

Comments

Kieran Bingham June 25, 2024, 8:51 p.m. UTC | #1 
Quoting Umang Jain (2024-06-25 07:23:25)
> Add a helper to set selection rectangle on the video node
> to support cropping and scaling capabilites.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  .../internal/converter/converter_v4l2_m2m.h   |  5 ++++
>  .../converter/converter_v4l2_m2m.cpp          | 27 +++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> index b9e59899..846e9f9e 100644
> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> @@ -30,6 +30,7 @@ class Size;
>  class SizeRange;
>  class Stream;
>  struct StreamConfiguration;
> +class Rectangle;
>  class V4L2M2MDevice;
>  
>  class V4L2M2MConverter : public Converter
> @@ -57,6 +58,8 @@ public:
>         int queueBuffers(FrameBuffer *input,
>                          const std::map<const Stream *, FrameBuffer *> &outputs);
>  
> +       int setSelection(const Stream *stream, unsigned int target, Rectangle *rect);
> +
>  private:
>         class V4L2M2MStream : protected Loggable
>         {
> @@ -75,6 +78,8 @@ private:
>  
>                 int queueBuffers(FrameBuffer *input, FrameBuffer *output);
>  
> +               int setSelection(unsigned int target, Rectangle *rect);
> +
>         protected:
>                 std::string logPrefix() const override;
>  
> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
> index 2e77872e..c323f677 100644
> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp
> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
> @@ -155,6 +155,15 @@ int V4L2M2MConverter::V4L2M2MStream::queueBuffers(FrameBuffer *input, FrameBuffe
>         return 0;
>  }
>  
> +int V4L2M2MConverter::V4L2M2MStream::setSelection(unsigned int target, Rectangle *rect)
> +{
> +       int ret = m2m_->output()->setSelection(target, rect);

Can selections ever be expected to be set on the input()?

> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
>  std::string V4L2M2MConverter::V4L2M2MStream::logPrefix() const
>  {
>         return stream_->configuration().toString();
> @@ -374,6 +383,24 @@ int V4L2M2MConverter::exportBuffers(const Stream *stream, unsigned int count,
>         return iter->second->exportBuffers(count, buffers);
>  }
>  
> +/**
> + * \brief Set a selection rectangle \a rect for \a target
> + * \param[in] stream Pointer to output stream
> + * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> + * \param[inout] rect The selection rectangle to be applied
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2M2MConverter::setSelection(const Stream *stream, unsigned int target,
> +                                  Rectangle *rect)

I'm not yet sure how far up the V4L2 types should be transferred in this, but as the class name is V4L2M2M - I think we're still good here.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +{
> +       auto iter = streams_.find(stream);
> +       if (iter == streams_.end())
> +               return -EINVAL;
> +
> +       return iter->second->setSelection(target, rect);
> +}
> +
>  /**
>   * \copydoc libcamera::Converter::start
>   */
> -- 
> 2.44.0
>
Umang Jain June 26, 2024, 3:48 a.m. UTC | #2 
Hi Kieran,

On 26/06/24 2:21 am, Kieran Bingham wrote:
> Quoting Umang Jain (2024-06-25 07:23:25)
>> Add a helper to set selection rectangle on the video node
>> to support cropping and scaling capabilites.
>>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   .../internal/converter/converter_v4l2_m2m.h   |  5 ++++
>>   .../converter/converter_v4l2_m2m.cpp          | 27 +++++++++++++++++++
>>   2 files changed, 32 insertions(+)
>>
>> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
>> index b9e59899..846e9f9e 100644
>> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
>> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
>> @@ -30,6 +30,7 @@ class Size;
>>   class SizeRange;
>>   class Stream;
>>   struct StreamConfiguration;
>> +class Rectangle;
>>   class V4L2M2MDevice;
>>   
>>   class V4L2M2MConverter : public Converter
>> @@ -57,6 +58,8 @@ public:
>>          int queueBuffers(FrameBuffer *input,
>>                           const std::map<const Stream *, FrameBuffer *> &outputs);
>>   
>> +       int setSelection(const Stream *stream, unsigned int target, Rectangle *rect);
>> +
>>   private:
>>          class V4L2M2MStream : protected Loggable
>>          {
>> @@ -75,6 +78,8 @@ private:
>>   
>>                  int queueBuffers(FrameBuffer *input, FrameBuffer *output);
>>   
>> +               int setSelection(unsigned int target, Rectangle *rect);
>> +
>>          protected:
>>                  std::string logPrefix() const override;
>>   
>> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
>> index 2e77872e..c323f677 100644
>> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp
>> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
>> @@ -155,6 +155,15 @@ int V4L2M2MConverter::V4L2M2MStream::queueBuffers(FrameBuffer *input, FrameBuffe
>>          return 0;
>>   }
>>   
>> +int V4L2M2MConverter::V4L2M2MStream::setSelection(unsigned int target, Rectangle *rect)
>> +{
>> +       int ret = m2m_->output()->setSelection(target, rect);
> Can selections ever be expected to be set on the input()?

I don't think so - You get -EINVAL here if you try to set selection on 
input (i.e. capture) nodes
>
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       return 0;
>> +}
>> +
>>   std::string V4L2M2MConverter::V4L2M2MStream::logPrefix() const
>>   {
>>          return stream_->configuration().toString();
>> @@ -374,6 +383,24 @@ int V4L2M2MConverter::exportBuffers(const Stream *stream, unsigned int count,
>>          return iter->second->exportBuffers(count, buffers);
>>   }
>>   
>> +/**
>> + * \brief Set a selection rectangle \a rect for \a target
>> + * \param[in] stream Pointer to output stream
>> + * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
>> + * \param[inout] rect The selection rectangle to be applied
>> + *
>> + * \return 0 on success or a negative error code otherwise
>> + */
>> +int V4L2M2MConverter::setSelection(const Stream *stream, unsigned int target,
>> +                                  Rectangle *rect)
> I'm not yet sure how far up the V4L2 types should be transferred in this, but as the class name is V4L2M2M - I think we're still good here.
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
>> +{
>> +       auto iter = streams_.find(stream);
>> +       if (iter == streams_.end())
>> +               return -EINVAL;
>> +
>> +       return iter->second->setSelection(target, rect);
>> +}
>> +
>>   /**
>>    * \copydoc libcamera::Converter::start
>>    */
>> -- 
>> 2.44.0
>>
Laurent Pinchart June 26, 2024, 7:37 a.m. UTC | #3 
On Tue, Jun 25, 2024 at 09:51:54PM +0100, Kieran Bingham wrote:
> Quoting Umang Jain (2024-06-25 07:23:25)
> > Add a helper to set selection rectangle on the video node
> > to support cropping and scaling capabilites.
> > 
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> >  .../internal/converter/converter_v4l2_m2m.h   |  5 ++++
> >  .../converter/converter_v4l2_m2m.cpp          | 27 +++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> > index b9e59899..846e9f9e 100644
> > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
> > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> > @@ -30,6 +30,7 @@ class Size;
> >  class SizeRange;
> >  class Stream;
> >  struct StreamConfiguration;
> > +class Rectangle;
> >  class V4L2M2MDevice;
> >  
> >  class V4L2M2MConverter : public Converter
> > @@ -57,6 +58,8 @@ public:
> >         int queueBuffers(FrameBuffer *input,
> >                          const std::map<const Stream *, FrameBuffer *> &outputs);
> >  
> > +       int setSelection(const Stream *stream, unsigned int target, Rectangle *rect);

I think this feature belongs to the Converter API. The ability to crop
the input isn't restricted to V4L2-based converters. You will then also
need to extend the Converter API to report the supported features, as
not all converters will be able to crop. This also means that you
shouldn't pass a V4L2 selection target, but create functions that focus
on features, not just exposing the V4L2 API to the upper level.

> > +
> >  private:
> >         class V4L2M2MStream : protected Loggable
> >         {
> > @@ -75,6 +78,8 @@ private:
> >  
> >                 int queueBuffers(FrameBuffer *input, FrameBuffer *output);
> >  
> > +               int setSelection(unsigned int target, Rectangle *rect);
> > +
> >         protected:
> >                 std::string logPrefix() const override;
> >  
> > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
> > index 2e77872e..c323f677 100644
> > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp
> > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
> > @@ -155,6 +155,15 @@ int V4L2M2MConverter::V4L2M2MStream::queueBuffers(FrameBuffer *input, FrameBuffe
> >         return 0;
> >  }
> >  
> > +int V4L2M2MConverter::V4L2M2MStream::setSelection(unsigned int target, Rectangle *rect)
> > +{
> > +       int ret = m2m_->output()->setSelection(target, rect);
> 
> Can selections ever be expected to be set on the input()?

Thanks to V4L2's lovely naming scheme, you've confusing input and output
:-) output() here is the input of the M2M device, the other side is
capture().

> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       return 0;
> > +}
> > +
> >  std::string V4L2M2MConverter::V4L2M2MStream::logPrefix() const
> >  {
> >         return stream_->configuration().toString();
> > @@ -374,6 +383,24 @@ int V4L2M2MConverter::exportBuffers(const Stream *stream, unsigned int count,
> >         return iter->second->exportBuffers(count, buffers);
> >  }
> >  
> > +/**
> > + * \brief Set a selection rectangle \a rect for \a target
> > + * \param[in] stream Pointer to output stream
> > + * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> > + * \param[inout] rect The selection rectangle to be applied
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +int V4L2M2MConverter::setSelection(const Stream *stream, unsigned int target,
> > +                                  Rectangle *rect)
> 
> I'm not yet sure how far up the V4L2 types should be transferred in
> this, but as the class name is V4L2M2M - I think we're still good
> here.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> > +{
> > +       auto iter = streams_.find(stream);
> > +       if (iter == streams_.end())
> > +               return -EINVAL;
> > +
> > +       return iter->second->setSelection(target, rect);
> > +}
> > +
> >  /**
> >   * \copydoc libcamera::Converter::start
> >   */

Patch
diff mbox series 

diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
index b9e59899..846e9f9e 100644
--- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
+++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
@@ -30,6 +30,7 @@  class Size;
 class SizeRange;
 class Stream;
 struct StreamConfiguration;
+class Rectangle;
 class V4L2M2MDevice;
 
 class V4L2M2MConverter : public Converter
@@ -57,6 +58,8 @@  public:
 	int queueBuffers(FrameBuffer *input,
 			 const std::map<const Stream *, FrameBuffer *> &outputs);
 
+	int setSelection(const Stream *stream, unsigned int target, Rectangle *rect);
+
 private:
 	class V4L2M2MStream : protected Loggable
 	{
@@ -75,6 +78,8 @@  private:
 
 		int queueBuffers(FrameBuffer *input, FrameBuffer *output);
 
+		int setSelection(unsigned int target, Rectangle *rect);
+
 	protected:
 		std::string logPrefix() const override;
 
diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
index 2e77872e..c323f677 100644
--- a/src/libcamera/converter/converter_v4l2_m2m.cpp
+++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
@@ -155,6 +155,15 @@  int V4L2M2MConverter::V4L2M2MStream::queueBuffers(FrameBuffer *input, FrameBuffe
 	return 0;
 }
 
+int V4L2M2MConverter::V4L2M2MStream::setSelection(unsigned int target, Rectangle *rect)
+{
+	int ret = m2m_->output()->setSelection(target, rect);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 std::string V4L2M2MConverter::V4L2M2MStream::logPrefix() const
 {
 	return stream_->configuration().toString();
@@ -374,6 +383,24 @@  int V4L2M2MConverter::exportBuffers(const Stream *stream, unsigned int count,
 	return iter->second->exportBuffers(count, buffers);
 }
 
+/**
+ * \brief Set a selection rectangle \a rect for \a target
+ * \param[in] stream Pointer to output stream
+ * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
+ * \param[inout] rect The selection rectangle to be applied
+ *
+ * \return 0 on success or a negative error code otherwise
+ */
+int V4L2M2MConverter::setSelection(const Stream *stream, unsigned int target,
+				   Rectangle *rect)
+{
+	auto iter = streams_.find(stream);
+	if (iter == streams_.end())
+		return -EINVAL;
+
+	return iter->second->setSelection(target, rect);
+}
+
 /**
  * \copydoc libcamera::Converter::start
  */