[libcamera-devel,12/20] pipeline: rpi: Add SW downscale status to RPi::Stream
diff mbox series

Message ID 20231006132000.23504-13-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi: Preliminary PiSP support
Related show

Commit Message

Naushir Patuck Oct. 6, 2023, 1:19 p.m. UTC 
Record if additional software downscaling is needed for a particular
stream in the RPi::Stream class. Additional software downscaling may be
needed if the user required downscale factor is greater than what the
ISP hardware is capable of.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/libcamera/pipeline/rpi/common/rpi_stream.cpp | 10 ++++++++++
 src/libcamera/pipeline/rpi/common/rpi_stream.h   | 11 +++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi Oct. 12, 2023, 9:36 a.m. UTC | #1 
Hi Naush

On Fri, Oct 06, 2023 at 02:19:52PM +0100, Naushir Patuck via libcamera-devel wrote:
> Record if additional software downscaling is needed for a particular
> stream in the RPi::Stream class. Additional software downscaling may be
> needed if the user required downscale factor is greater than what the
> ISP hardware is capable of.

Why this is not a flag ?
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/libcamera/pipeline/rpi/common/rpi_stream.cpp | 10 ++++++++++
>  src/libcamera/pipeline/rpi/common/rpi_stream.h   | 11 +++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> index fd6f2efc6e27..25e6a4383bf6 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> @@ -57,6 +57,16 @@ const std::string &Stream::name() const
>  	return name_;
>  }
>
> +unsigned int Stream::swDownscale() const
> +{
> +	return swDownscale_;
> +}
> +
> +void Stream::setSwDownscale(unsigned int swDownscale)
> +{
> +	swDownscale_ = swDownscale;
> +}
> +
>  void Stream::resetBuffers()
>  {
>  	/* Add all internal buffers to the queue of usable buffers. */
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> index c5e35d134926..fc2bdfe25d4a 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> @@ -86,13 +86,14 @@ public:
>  	using StreamFlags = Flags<StreamFlag>;
>
>  	Stream()
> -		: flags_(StreamFlag::None), id_(0)
> +		: flags_(StreamFlag::None), id_(0), swDownscale_(0)
>  	{
>  	}
>
>  	Stream(const char *name, MediaEntity *dev, StreamFlags flags = StreamFlag::None)
>  		: flags_(flags), name_(name),
> -		  dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0)
> +		  dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0),
> +		  swDownscale_(0)
>  	{
>  	}
>
> @@ -104,6 +105,9 @@ public:
>  	const std::string &name() const;
>  	void resetBuffers();
>
> +	unsigned int swDownscale() const;
> +	void setSwDownscale(unsigned int swDownscale);
> +
>  	void setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>  	const BufferMap &getBuffers() const;
>  	unsigned int getBufferId(FrameBuffer *buffer) const;
> @@ -139,6 +143,9 @@ private:
>  	/* Tracks a unique id key for the bufferMap_ */
>  	unsigned int id_;
>
> +	/* Power of 2 greater than one if software downscaling will be required. */
> +	unsigned int swDownscale_;
> +
>  	/* All frame buffers associated with this device stream. */
>  	BufferMap bufferMap_;
>
> --
> 2.34.1
>
Naushir Patuck Oct. 12, 2023, 9:50 a.m. UTC | #2 
Hi Jacopo,

On Thu, 12 Oct 2023 at 10:36, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Naush
>
> On Fri, Oct 06, 2023 at 02:19:52PM +0100, Naushir Patuck via libcamera-devel wrote:
> > Record if additional software downscaling is needed for a particular
> > stream in the RPi::Stream class. Additional software downscaling may be
> > needed if the user required downscale factor is greater than what the
> > ISP hardware is capable of.
>
> Why this is not a flag ?

Because we need to store the downscale factor used (2/4/8...) instead
of just a boolean.

Regards,
Naush


> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/libcamera/pipeline/rpi/common/rpi_stream.cpp | 10 ++++++++++
> >  src/libcamera/pipeline/rpi/common/rpi_stream.h   | 11 +++++++++--
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > index fd6f2efc6e27..25e6a4383bf6 100644
> > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > @@ -57,6 +57,16 @@ const std::string &Stream::name() const
> >       return name_;
> >  }
> >
> > +unsigned int Stream::swDownscale() const
> > +{
> > +     return swDownscale_;
> > +}
> > +
> > +void Stream::setSwDownscale(unsigned int swDownscale)
> > +{
> > +     swDownscale_ = swDownscale;
> > +}
> > +
> >  void Stream::resetBuffers()
> >  {
> >       /* Add all internal buffers to the queue of usable buffers. */
> > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > index c5e35d134926..fc2bdfe25d4a 100644
> > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > @@ -86,13 +86,14 @@ public:
> >       using StreamFlags = Flags<StreamFlag>;
> >
> >       Stream()
> > -             : flags_(StreamFlag::None), id_(0)
> > +             : flags_(StreamFlag::None), id_(0), swDownscale_(0)
> >       {
> >       }
> >
> >       Stream(const char *name, MediaEntity *dev, StreamFlags flags = StreamFlag::None)
> >               : flags_(flags), name_(name),
> > -               dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0)
> > +               dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0),
> > +               swDownscale_(0)
> >       {
> >       }
> >
> > @@ -104,6 +105,9 @@ public:
> >       const std::string &name() const;
> >       void resetBuffers();
> >
> > +     unsigned int swDownscale() const;
> > +     void setSwDownscale(unsigned int swDownscale);
> > +
> >       void setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> >       const BufferMap &getBuffers() const;
> >       unsigned int getBufferId(FrameBuffer *buffer) const;
> > @@ -139,6 +143,9 @@ private:
> >       /* Tracks a unique id key for the bufferMap_ */
> >       unsigned int id_;
> >
> > +     /* Power of 2 greater than one if software downscaling will be required. */
> > +     unsigned int swDownscale_;
> > +
> >       /* All frame buffers associated with this device stream. */
> >       BufferMap bufferMap_;
> >
> > --
> > 2.34.1
> >
Jacopo Mondi Oct. 12, 2023, 10:07 a.m. UTC | #3 
Hi Naush

On Thu, Oct 12, 2023 at 10:50:01AM +0100, Naushir Patuck via libcamera-devel wrote:
> Hi Jacopo,
>
> On Thu, 12 Oct 2023 at 10:36, Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hi Naush
> >
> > On Fri, Oct 06, 2023 at 02:19:52PM +0100, Naushir Patuck via libcamera-devel wrote:
> > > Record if additional software downscaling is needed for a particular
> > > stream in the RPi::Stream class. Additional software downscaling may be
> > > needed if the user required downscale factor is greater than what the
> > > ISP hardware is capable of.
> >
> > Why this is not a flag ?
>
> Because we need to store the downscale factor used (2/4/8...) instead
> of just a boolean.

The comment reports that too :/

/* Power of 2 greater than one if software downscaling will be required. */

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Sorry for the noise

>
> Regards,
> Naush
>
>
> > >
> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> > > ---
> > >  src/libcamera/pipeline/rpi/common/rpi_stream.cpp | 10 ++++++++++
> > >  src/libcamera/pipeline/rpi/common/rpi_stream.h   | 11 +++++++++--
> > >  2 files changed, 19 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > > index fd6f2efc6e27..25e6a4383bf6 100644
> > > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > > @@ -57,6 +57,16 @@ const std::string &Stream::name() const
> > >       return name_;
> > >  }
> > >
> > > +unsigned int Stream::swDownscale() const
> > > +{
> > > +     return swDownscale_;
> > > +}
> > > +
> > > +void Stream::setSwDownscale(unsigned int swDownscale)
> > > +{
> > > +     swDownscale_ = swDownscale;
> > > +}
> > > +
> > >  void Stream::resetBuffers()
> > >  {
> > >       /* Add all internal buffers to the queue of usable buffers. */
> > > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > > index c5e35d134926..fc2bdfe25d4a 100644
> > > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > > @@ -86,13 +86,14 @@ public:
> > >       using StreamFlags = Flags<StreamFlag>;
> > >
> > >       Stream()
> > > -             : flags_(StreamFlag::None), id_(0)
> > > +             : flags_(StreamFlag::None), id_(0), swDownscale_(0)
> > >       {
> > >       }
> > >
> > >       Stream(const char *name, MediaEntity *dev, StreamFlags flags = StreamFlag::None)
> > >               : flags_(flags), name_(name),
> > > -               dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0)
> > > +               dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0),
> > > +               swDownscale_(0)
> > >       {
> > >       }
> > >
> > > @@ -104,6 +105,9 @@ public:
> > >       const std::string &name() const;
> > >       void resetBuffers();
> > >
> > > +     unsigned int swDownscale() const;
> > > +     void setSwDownscale(unsigned int swDownscale);
> > > +
> > >       void setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> > >       const BufferMap &getBuffers() const;
> > >       unsigned int getBufferId(FrameBuffer *buffer) const;
> > > @@ -139,6 +143,9 @@ private:
> > >       /* Tracks a unique id key for the bufferMap_ */
> > >       unsigned int id_;
> > >
> > > +     /* Power of 2 greater than one if software downscaling will be required. */
> > > +     unsigned int swDownscale_;
> > > +
> > >       /* All frame buffers associated with this device stream. */
> > >       BufferMap bufferMap_;
> > >
> > > --
> > > 2.34.1
> > >

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
index fd6f2efc6e27..25e6a4383bf6 100644
--- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
+++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
@@ -57,6 +57,16 @@  const std::string &Stream::name() const
 	return name_;
 }
 
+unsigned int Stream::swDownscale() const
+{
+	return swDownscale_;
+}
+
+void Stream::setSwDownscale(unsigned int swDownscale)
+{
+	swDownscale_ = swDownscale;
+}
+
 void Stream::resetBuffers()
 {
 	/* Add all internal buffers to the queue of usable buffers. */
diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
index c5e35d134926..fc2bdfe25d4a 100644
--- a/src/libcamera/pipeline/rpi/common/rpi_stream.h
+++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h
@@ -86,13 +86,14 @@  public:
 	using StreamFlags = Flags<StreamFlag>;
 
 	Stream()
-		: flags_(StreamFlag::None), id_(0)
+		: flags_(StreamFlag::None), id_(0), swDownscale_(0)
 	{
 	}
 
 	Stream(const char *name, MediaEntity *dev, StreamFlags flags = StreamFlag::None)
 		: flags_(flags), name_(name),
-		  dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0)
+		  dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(0),
+		  swDownscale_(0)
 	{
 	}
 
@@ -104,6 +105,9 @@  public:
 	const std::string &name() const;
 	void resetBuffers();
 
+	unsigned int swDownscale() const;
+	void setSwDownscale(unsigned int swDownscale);
+
 	void setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
 	const BufferMap &getBuffers() const;
 	unsigned int getBufferId(FrameBuffer *buffer) const;
@@ -139,6 +143,9 @@  private:
 	/* Tracks a unique id key for the bufferMap_ */
 	unsigned int id_;
 
+	/* Power of 2 greater than one if software downscaling will be required. */
+	unsigned int swDownscale_;
+
 	/* All frame buffers associated with this device stream. */
 	BufferMap bufferMap_;