[libcamera-devel] qcam: Print whole stream configuration when adjusted

3323 diff mbox series

When the validate() function adjusts the stream configuration, we print
the adjusted size for debugging purpose. Switch to printing the whole
configuration, as the pixel format may be useful too.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/main_window.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Hi Laurent,

On 26/03/2020 01:35, Laurent Pinchart wrote:
> When the validate() function adjusts the stream configuration, we print
> the adjusted size for debugging purpose. Switch to printing the whole
> configuration, as the pixel format may be useful too.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 565732698a5e..cf39ed7aceca 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -281,6 +281,7 @@ int MainWindow::startCapture()
>  	config_ = camera_->generateConfiguration({ StreamRole::Viewfinder });
>  
>  	StreamConfiguration &cfg = config_->at(0);
> +
>  	if (options_.isSet(OptSize)) {
>  		const std::vector<OptionValue> &sizeOptions =
>  			options_[OptSize].toArray();
> @@ -316,10 +317,9 @@ int MainWindow::startCapture()
>  		return -EINVAL;
>  	}
>  
> -	if (validation == CameraConfiguration::Adjusted) {
> -		qInfo() << "Stream size adjusted to"
> -			<< cfg.size.toString().c_str();
> -	}
> +	if (validation == CameraConfiguration::Adjusted)
> +		qInfo() << "Stream configuration adjusted to "
> +			<< cfg.toString().c_str();
>  


Should we make this same change in the pipelines validate() call too?
(as a separate patch from this I expect)

But for this patch,

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

>  	ret = camera_->configure(config_.get());
>  	if (ret < 0) {
>

Hi Kieran,

On Thu, Mar 26, 2020 at 08:56:42AM +0000, Kieran Bingham wrote:
> On 26/03/2020 01:35, Laurent Pinchart wrote:
> > When the validate() function adjusts the stream configuration, we print
> > the adjusted size for debugging purpose. Switch to printing the whole
> > configuration, as the pixel format may be useful too.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/qcam/main_window.cpp | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 565732698a5e..cf39ed7aceca 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -281,6 +281,7 @@ int MainWindow::startCapture()
> >  	config_ = camera_->generateConfiguration({ StreamRole::Viewfinder });
> >  
> >  	StreamConfiguration &cfg = config_->at(0);
> > +
> >  	if (options_.isSet(OptSize)) {
> >  		const std::vector<OptionValue> &sizeOptions =
> >  			options_[OptSize].toArray();
> > @@ -316,10 +317,9 @@ int MainWindow::startCapture()
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (validation == CameraConfiguration::Adjusted) {
> > -		qInfo() << "Stream size adjusted to"
> > -			<< cfg.size.toString().c_str();
> > -	}
> > +	if (validation == CameraConfiguration::Adjusted)
> > +		qInfo() << "Stream configuration adjusted to "
> > +			<< cfg.toString().c_str();
> >  
> 
> Should we make this same change in the pipelines validate() call too?
> (as a separate patch from this I expect)

I wonder if they would actually simplify pipeline handlers, as they
would need to keep a copy of the StreamConfiguration to be able to print
the old and new values. Something to think about separately indeed
anyway.

> But for this patch,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >  	ret = camera_->configure(config_.get());
> >  	if (ret < 0) {
> >