[libcamera-devel,v1,3/3] pipeline: raspberrypi: Add a Unicam timeout override config options
diff mbox series

Message ID mailman.15.1677156593.25031.libcamera-devel@lists.libcamera.org
State Superseded
Headers show
Series
  • Raspberry Pi: Improving camera timeouts
Related show

Commit Message

Naushir Patuck Feb. 23, 2023, 12:49 p.m. UTC
Add a new parameter to the pipeline handler config file named
"unicam_timeout_value_ms" to allow users to override the automiatically
computed Unicam timeout value.

This value is given in milliseconds, and setting a value of 0 (the
default value) disables the override.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 .../pipeline/raspberrypi/data/example.yaml    | 11 ++++++-
 .../pipeline/raspberrypi/raspberrypi.cpp      | 29 ++++++++++++++-----
 2 files changed, 32 insertions(+), 8 deletions(-)

Comments

David Plowman Feb. 24, 2023, 11:22 a.m. UTC | #1
Hi Naush

Thanks for the patch.

On Thu, 23 Feb 2023 at 12:49, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> Add a new parameter to the pipeline handler config file named
> "unicam_timeout_value_ms" to allow users to override the automiatically

s/automiatically/automatically]

> computed Unicam timeout value.
>
> This value is given in milliseconds, and setting a value of 0 (the
> default value) disables the override.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/data/example.yaml    | 11 ++++++-
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 29 ++++++++++++++-----
>  2 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> index ad5f2344384f..5d4ca997b7a0 100644
> --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml
> +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> @@ -32,6 +32,15 @@
>                  # Override any request from the IPA to drop a number of startup
>                  # frames.
>                  #
> -                # "disable_startup_frame_drops": false
> +                # "disable_startup_frame_drops": false,
> +
> +                # Custom timeout value (in ms) for Unicam to use. This overrides
> +                # the value computed by the pipeline handler based on frame
> +                # durations.
> +                #
> +                # Set this value to 0 to use the pipeline handler computed
> +                # timeout value.
> +                #
> +                # "unicam_timeout_value_ms": 0
>          }
>  }
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 3d04842a2440..7c8c66129014 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -319,6 +319,11 @@ public:
>                  * frames.
>                  */
>                 bool disableStartupFrameDrops;
> +               /*
> +                * Override the Unicam timeout value calculated by the IPA based
> +                * on frame durations.
> +                */
> +               unsigned int unicamTimeoutValue;
>         };
>
>         Config config_;
> @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
>
>         data->state_ = RPiCameraData::State::Idle;
>
> +       if (data->config_.unicamTimeoutValue)
> +               data->setCameraTimeout(data->config_.unicamTimeoutValue);
> +
>         /* Start all streams. */
>         for (auto const stream : data->streams_) {
>                 ret = stream->dev()->streamOn();
> @@ -1715,6 +1723,7 @@ int RPiCameraData::loadPipelineConfiguration()
>                 .minUnicamBuffers = 2,
>                 .minTotalUnicamBuffers = 4,
>                 .disableStartupFrameDrops = false,
> +               .unicamTimeoutValue = 0,
>         };
>
>         char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> @@ -1750,6 +1759,8 @@ int RPiCameraData::loadPipelineConfiguration()
>                 phConfig["min_total_unicam_buffers"].get<unsigned int>(config_.minTotalUnicamBuffers);
>         config_.disableStartupFrameDrops =
>                 phConfig["disable_startup_frame_drops"].get<bool>(config_.disableStartupFrameDrops);
> +       config_.unicamTimeoutValue =
> +               phConfig["unicam_timeout_value_ms"].get<bool>(config_.disableStartupFrameDrops);

Is bool the right type here? Also is the appearance of
"disableStartupFrameDrops" a copy/paste typo?

Apart from these minor things:

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

>
>         if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {
>                 LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers";
> @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)
>
>  void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
>  {
> -       /*
> -        * Set the dequeue timeout to the larger of 5x the maximum reported
> -        * frame length advertised by the IPA over a number of frames. Allow
> -        * a minimum timeout value of 1s.
> -        */
> -       utils::Duration timeout =
> -               std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> +       utils::Duration timeout;
> +
> +       if (!config_.unicamTimeoutValue) {
> +               /*
> +                * Set the dequeue timeout to the larger of 5x the maximum reported
> +                * frame length advertised by the IPA over a number of frames. Allow
> +                * a minimum timeout value of 1s.
> +                */
> +               timeout = std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> +       } else
> +               timeout = config_.unicamTimeoutValue * 1ms;
>
>         LOG(RPI, Debug) << "Setting Unicam timeout to " << timeout;
>         unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);
> --
> 2.25.1
>
Naushir Patuck Feb. 24, 2023, 11:30 a.m. UTC | #2
On Fri, 24 Feb 2023 at 11:22, David Plowman
<david.plowman@raspberrypi.com> wrote:
>
> Hi Naush
>
> Thanks for the patch.
>
> On Thu, 23 Feb 2023 at 12:49, Naushir Patuck via libcamera-devel
> <libcamera-devel@lists.libcamera.org> wrote:
> >
> > Add a new parameter to the pipeline handler config file named
> > "unicam_timeout_value_ms" to allow users to override the automiatically
>
> s/automiatically/automatically]
>
> > computed Unicam timeout value.
> >
> > This value is given in milliseconds, and setting a value of 0 (the
> > default value) disables the override.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  .../pipeline/raspberrypi/data/example.yaml    | 11 ++++++-
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 29 ++++++++++++++-----
> >  2 files changed, 32 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> > index ad5f2344384f..5d4ca997b7a0 100644
> > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml
> > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> > @@ -32,6 +32,15 @@
> >                  # Override any request from the IPA to drop a number of startup
> >                  # frames.
> >                  #
> > -                # "disable_startup_frame_drops": false
> > +                # "disable_startup_frame_drops": false,
> > +
> > +                # Custom timeout value (in ms) for Unicam to use. This overrides
> > +                # the value computed by the pipeline handler based on frame
> > +                # durations.
> > +                #
> > +                # Set this value to 0 to use the pipeline handler computed
> > +                # timeout value.
> > +                #
> > +                # "unicam_timeout_value_ms": 0
> >          }
> >  }
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 3d04842a2440..7c8c66129014 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -319,6 +319,11 @@ public:
> >                  * frames.
> >                  */
> >                 bool disableStartupFrameDrops;
> > +               /*
> > +                * Override the Unicam timeout value calculated by the IPA based
> > +                * on frame durations.
> > +                */
> > +               unsigned int unicamTimeoutValue;
> >         };
> >
> >         Config config_;
> > @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
> >
> >         data->state_ = RPiCameraData::State::Idle;
> >
> > +       if (data->config_.unicamTimeoutValue)
> > +               data->setCameraTimeout(data->config_.unicamTimeoutValue);
> > +
> >         /* Start all streams. */
> >         for (auto const stream : data->streams_) {
> >                 ret = stream->dev()->streamOn();
> > @@ -1715,6 +1723,7 @@ int RPiCameraData::loadPipelineConfiguration()
> >                 .minUnicamBuffers = 2,
> >                 .minTotalUnicamBuffers = 4,
> >                 .disableStartupFrameDrops = false,
> > +               .unicamTimeoutValue = 0,
> >         };
> >
> >         char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> > @@ -1750,6 +1759,8 @@ int RPiCameraData::loadPipelineConfiguration()
> >                 phConfig["min_total_unicam_buffers"].get<unsigned int>(config_.minTotalUnicamBuffers);
> >         config_.disableStartupFrameDrops =
> >                 phConfig["disable_startup_frame_drops"].get<bool>(config_.disableStartupFrameDrops);
> > +       config_.unicamTimeoutValue =
> > +               phConfig["unicam_timeout_value_ms"].get<bool>(config_.disableStartupFrameDrops);
>
> Is bool the right type here? Also is the appearance of
> "disableStartupFrameDrops" a copy/paste typo?

Oh no.  Evidently I submitted the wrong patch :-(   My local changes
show the correct code.
I will fix this and resubmit.

Naush

>
> Apart from these minor things:
>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
>
> Thanks!
> David
>
> >
> >         if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {
> >                 LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers";
> > @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)
> >
> >  void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
> >  {
> > -       /*
> > -        * Set the dequeue timeout to the larger of 5x the maximum reported
> > -        * frame length advertised by the IPA over a number of frames. Allow
> > -        * a minimum timeout value of 1s.
> > -        */
> > -       utils::Duration timeout =
> > -               std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> > +       utils::Duration timeout;
> > +
> > +       if (!config_.unicamTimeoutValue) {
> > +               /*
> > +                * Set the dequeue timeout to the larger of 5x the maximum reported
> > +                * frame length advertised by the IPA over a number of frames. Allow
> > +                * a minimum timeout value of 1s.
> > +                */
> > +               timeout = std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> > +       } else
> > +               timeout = config_.unicamTimeoutValue * 1ms;
> >
> >         LOG(RPI, Debug) << "Setting Unicam timeout to " << timeout;
> >         unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);
> > --
> > 2.25.1
> >
Jacopo Mondi March 1, 2023, 3:17 p.m. UTC | #3
Hi Naush

On Thu, Feb 23, 2023 at 12:49:57PM +0000, Naushir Patuck via libcamera-devel wrote:
> Date: Thu, 23 Feb 2023 12:49:57 +0000
> From: Naushir Patuck <naush@raspberrypi.com>
> To: libcamera-devel@lists.libcamera.org
> Subject: [PATCH v1 3/3] pipeline: raspberrypi: Add a Unicam timeout
>  override config options
> X-Mailer: git-send-email 2.25.1
>
> Add a new parameter to the pipeline handler config file named
> "unicam_timeout_value_ms" to allow users to override the automiatically
> computed Unicam timeout value.
>
> This value is given in milliseconds, and setting a value of 0 (the
> default value) disables the override.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/data/example.yaml    | 11 ++++++-
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 29 ++++++++++++++-----
>  2 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> index ad5f2344384f..5d4ca997b7a0 100644
> --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml
> +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> @@ -32,6 +32,15 @@
>                  # Override any request from the IPA to drop a number of startup
>                  # frames.
>                  #
> -                # "disable_startup_frame_drops": false
> +                # "disable_startup_frame_drops": false,
> +
> +                # Custom timeout value (in ms) for Unicam to use. This overrides
> +                # the value computed by the pipeline handler based on frame
> +                # durations.
> +                #
> +                # Set this value to 0 to use the pipeline handler computed
> +                # timeout value.
> +                #
> +                # "unicam_timeout_value_ms": 0
>          }
>  }
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 3d04842a2440..7c8c66129014 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -319,6 +319,11 @@ public:
>  		 * frames.
>  		 */
>  		bool disableStartupFrameDrops;
> +		/*
> +		 * Override the Unicam timeout value calculated by the IPA based
> +		 * on frame durations.
> +		 */
> +		unsigned int unicamTimeoutValue;
>  	};
>
>  	Config config_;
> @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
>
>  	data->state_ = RPiCameraData::State::Idle;
>
> +	if (data->config_.unicamTimeoutValue)
> +		data->setCameraTimeout(data->config_.unicamTimeoutValue);
> +
>  	/* Start all streams. */
>  	for (auto const stream : data->streams_) {
>  		ret = stream->dev()->streamOn();
> @@ -1715,6 +1723,7 @@ int RPiCameraData::loadPipelineConfiguration()
>  		.minUnicamBuffers = 2,
>  		.minTotalUnicamBuffers = 4,
>  		.disableStartupFrameDrops = false,
> +		.unicamTimeoutValue = 0,
>  	};
>
>  	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> @@ -1750,6 +1759,8 @@ int RPiCameraData::loadPipelineConfiguration()
>  		phConfig["min_total_unicam_buffers"].get<unsigned int>(config_.minTotalUnicamBuffers);
>  	config_.disableStartupFrameDrops =
>  		phConfig["disable_startup_frame_drops"].get<bool>(config_.disableStartupFrameDrops);
> +	config_.unicamTimeoutValue =
> +		phConfig["unicam_timeout_value_ms"].get<bool>(config_.disableStartupFrameDrops);
>
>  	if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {
>  		LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers";
> @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)
>
>  void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
>  {
> -	/*
> -	 * Set the dequeue timeout to the larger of 5x the maximum reported
> -	 * frame length advertised by the IPA over a number of frames. Allow
> -	 * a minimum timeout value of 1s.
> -	 */
> -	utils::Duration timeout =
> -		std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> +	utils::Duration timeout;
> +
> +	if (!config_.unicamTimeoutValue) {

I wonder, when the value comes from configuration file, can it change
at run-time ? If it can't is there any purpose in emitting the signal,
handling it here and setting the same value on the video device ? If
that's the case, I would try to figure out early if unicamTimeoutValue
!= 0 and if that's the case set the value on the video device and not
connected (or disconnect) the signal

> +		/*
> +		 * Set the dequeue timeout to the larger of 5x the maximum reported
> +		 * frame length advertised by the IPA over a number of frames. Allow
> +		 * a minimum timeout value of 1s.
> +		 */
> +		timeout = std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> +	} else
> +		timeout = config_.unicamTimeoutValue * 1ms;
>
>  	LOG(RPI, Debug) << "Setting Unicam timeout to " << timeout;
>  	unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);
> --
> 2.25.1
>
Naushir Patuck March 2, 2023, 8:31 a.m. UTC | #4
Hi Jacopo,

Thank you for the feedback!

On Wed, 1 Mar 2023 at 15:17, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Naush
>
> On Thu, Feb 23, 2023 at 12:49:57PM +0000, Naushir Patuck via libcamera-devel wrote:
> > Date: Thu, 23 Feb 2023 12:49:57 +0000
> > From: Naushir Patuck <naush@raspberrypi.com>
> > To: libcamera-devel@lists.libcamera.org
> > Subject: [PATCH v1 3/3] pipeline: raspberrypi: Add a Unicam timeout
> >  override config options
> > X-Mailer: git-send-email 2.25.1
> >
> > Add a new parameter to the pipeline handler config file named
> > "unicam_timeout_value_ms" to allow users to override the automiatically
> > computed Unicam timeout value.
> >
> > This value is given in milliseconds, and setting a value of 0 (the
> > default value) disables the override.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  .../pipeline/raspberrypi/data/example.yaml    | 11 ++++++-
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 29 ++++++++++++++-----
> >  2 files changed, 32 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> > index ad5f2344384f..5d4ca997b7a0 100644
> > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml
> > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml
> > @@ -32,6 +32,15 @@
> >                  # Override any request from the IPA to drop a number of startup
> >                  # frames.
> >                  #
> > -                # "disable_startup_frame_drops": false
> > +                # "disable_startup_frame_drops": false,
> > +
> > +                # Custom timeout value (in ms) for Unicam to use. This overrides
> > +                # the value computed by the pipeline handler based on frame
> > +                # durations.
> > +                #
> > +                # Set this value to 0 to use the pipeline handler computed
> > +                # timeout value.
> > +                #
> > +                # "unicam_timeout_value_ms": 0
> >          }
> >  }
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 3d04842a2440..7c8c66129014 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -319,6 +319,11 @@ public:
> >                * frames.
> >                */
> >               bool disableStartupFrameDrops;
> > +             /*
> > +              * Override the Unicam timeout value calculated by the IPA based
> > +              * on frame durations.
> > +              */
> > +             unsigned int unicamTimeoutValue;
> >       };
> >
> >       Config config_;
> > @@ -1158,6 +1163,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
> >
> >       data->state_ = RPiCameraData::State::Idle;
> >
> > +     if (data->config_.unicamTimeoutValue)
> > +             data->setCameraTimeout(data->config_.unicamTimeoutValue);
> > +
> >       /* Start all streams. */
> >       for (auto const stream : data->streams_) {
> >               ret = stream->dev()->streamOn();
> > @@ -1715,6 +1723,7 @@ int RPiCameraData::loadPipelineConfiguration()
> >               .minUnicamBuffers = 2,
> >               .minTotalUnicamBuffers = 4,
> >               .disableStartupFrameDrops = false,
> > +             .unicamTimeoutValue = 0,
> >       };
> >
> >       char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> > @@ -1750,6 +1759,8 @@ int RPiCameraData::loadPipelineConfiguration()
> >               phConfig["min_total_unicam_buffers"].get<unsigned int>(config_.minTotalUnicamBuffers);
> >       config_.disableStartupFrameDrops =
> >               phConfig["disable_startup_frame_drops"].get<bool>(config_.disableStartupFrameDrops);
> > +     config_.unicamTimeoutValue =
> > +             phConfig["unicam_timeout_value_ms"].get<bool>(config_.disableStartupFrameDrops);
> >
> >       if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {
> >               LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers";
> > @@ -1953,13 +1964,17 @@ void RPiCameraData::setLensControls(const ControlList &controls)
> >
> >  void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
> >  {
> > -     /*
> > -      * Set the dequeue timeout to the larger of 5x the maximum reported
> > -      * frame length advertised by the IPA over a number of frames. Allow
> > -      * a minimum timeout value of 1s.
> > -      */
> > -     utils::Duration timeout =
> > -             std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> > +     utils::Duration timeout;
> > +
> > +     if (!config_.unicamTimeoutValue) {
>
> I wonder, when the value comes from configuration file, can it change
> at run-time ? If it can't is there any purpose in emitting the signal,
> handling it here and setting the same value on the video device ? If
> that's the case, I would try to figure out early if unicamTimeoutValue
> != 0 and if that's the case set the value on the video device and not
> connected (or disconnect) the signal

Good point. If the value comes from the config file, we will never have a
runtime change. In these cases, I can disconnect the signal.

For simplicity, I'll continue emitting the signal from the IPA, but presumably
that's ok if the handler is disconnected?

Regards,
Naush

>
> > +             /*
> > +              * Set the dequeue timeout to the larger of 5x the maximum reported
> > +              * frame length advertised by the IPA over a number of frames. Allow
> > +              * a minimum timeout value of 1s.
> > +              */
> > +             timeout = std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> > +     } else
> > +             timeout = config_.unicamTimeoutValue * 1ms;
> >
> >       LOG(RPI, Debug) << "Setting Unicam timeout to " << timeout;
> >       unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);
> > --
> > 2.25.1
> >

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml
index ad5f2344384f..5d4ca997b7a0 100644
--- a/src/libcamera/pipeline/raspberrypi/data/example.yaml
+++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml
@@ -32,6 +32,15 @@ 
                 # Override any request from the IPA to drop a number of startup
                 # frames.
                 #
-                # "disable_startup_frame_drops": false
+                # "disable_startup_frame_drops": false,
+
+                # Custom timeout value (in ms) for Unicam to use. This overrides
+                # the value computed by the pipeline handler based on frame
+                # durations.
+                #
+                # Set this value to 0 to use the pipeline handler computed
+                # timeout value.
+                #
+                # "unicam_timeout_value_ms": 0
         }
 }
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 3d04842a2440..7c8c66129014 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -319,6 +319,11 @@  public:
 		 * frames.
 		 */
 		bool disableStartupFrameDrops;
+		/*
+		 * Override the Unicam timeout value calculated by the IPA based
+		 * on frame durations.
+		 */
+		unsigned int unicamTimeoutValue;
 	};
 
 	Config config_;
@@ -1158,6 +1163,9 @@  int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
 
 	data->state_ = RPiCameraData::State::Idle;
 
+	if (data->config_.unicamTimeoutValue)
+		data->setCameraTimeout(data->config_.unicamTimeoutValue);
+
 	/* Start all streams. */
 	for (auto const stream : data->streams_) {
 		ret = stream->dev()->streamOn();
@@ -1715,6 +1723,7 @@  int RPiCameraData::loadPipelineConfiguration()
 		.minUnicamBuffers = 2,
 		.minTotalUnicamBuffers = 4,
 		.disableStartupFrameDrops = false,
+		.unicamTimeoutValue = 0,
 	};
 
 	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
@@ -1750,6 +1759,8 @@  int RPiCameraData::loadPipelineConfiguration()
 		phConfig["min_total_unicam_buffers"].get<unsigned int>(config_.minTotalUnicamBuffers);
 	config_.disableStartupFrameDrops =
 		phConfig["disable_startup_frame_drops"].get<bool>(config_.disableStartupFrameDrops);
+	config_.unicamTimeoutValue =
+		phConfig["unicam_timeout_value_ms"].get<bool>(config_.disableStartupFrameDrops);
 
 	if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {
 		LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers";
@@ -1953,13 +1964,17 @@  void RPiCameraData::setLensControls(const ControlList &controls)
 
 void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
 {
-	/*
-	 * Set the dequeue timeout to the larger of 5x the maximum reported
-	 * frame length advertised by the IPA over a number of frames. Allow
-	 * a minimum timeout value of 1s.
-	 */
-	utils::Duration timeout =
-		std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
+	utils::Duration timeout;
+
+	if (!config_.unicamTimeoutValue) {
+		/*
+		 * Set the dequeue timeout to the larger of 5x the maximum reported
+		 * frame length advertised by the IPA over a number of frames. Allow
+		 * a minimum timeout value of 1s.
+		 */
+		timeout = std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
+	} else
+		timeout = config_.unicamTimeoutValue * 1ms;
 
 	LOG(RPI, Debug) << "Setting Unicam timeout to " << timeout;
 	unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);