[PATCH v3 2/4] libcamera: converter_v4l2_m2m: Support crop selection

[Umang Jain]

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 at 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 at 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
>>