Message ID | 20200627030043.2088585-11-niklas.soderlund@ragnatech.se |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Niklas, Thank you for the patch. On Sat, Jun 27, 2020 at 05:00:40AM +0200, Niklas Söderlund wrote: > The active_ flag is only used inside one function, remove the global > flag and handle it inside the single function. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 1f320dc166767bab..b72b631cdc051aab 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -33,13 +33,6 @@ LOG_DEFINE_CATEGORY(IPU3) > > class IPU3Stream : public Stream > { > -public: > - IPU3Stream() > - : active_(false) > - { > - } > - > - bool active_; > }; > > class IPU3CameraData : public CameraData > @@ -486,8 +479,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > return ret; > > /* Apply the format to the configured streams output devices. */ > - outStream->active_ = false; > - vfStream->active_ = false; > + bool outActive = false; > + bool vfActive = false; With an enum for the stream ID (as proposed in 09/13) this could be an array. > > for (unsigned int i = 0; i < config->size(); ++i) { > /* > @@ -499,7 +492,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > IPU3Stream *stream = const_cast<IPU3Stream *>(config->streams()[i]); > StreamConfiguration &cfg = (*config)[i]; > > - stream->active_ = true; > cfg.setStream(stream); > > if (stream == outStream) { > @@ -508,12 +500,14 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > return ret; > > cfg.stride = outputFormat.planes[0].bpl; > + outActive = true; > } else if (stream == vfStream) { > ret = imgu->configureViewfinder(cfg, &outputFormat); > if (ret) > return ret; > > cfg.stride = outputFormat.planes[0].bpl; > + vfActive = true; > } else { > /* > * The RAW still capture stream just copies buffers from > @@ -529,13 +523,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > * the configuration of the active one for that purpose (there should > * be at least one active stream in the configuration request). > */ > - if (!outStream->active_) { > + if (!outActive) { > ret = imgu->configureOutput(config->at(0), &outputFormat); > if (ret) > return ret; > } > > - if (!vfStream->active_) { > + if (!vfActive) { > ret = imgu->configureViewfinder(config->at(0), &outputFormat); > if (ret) > return ret; > @@ -555,7 +549,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > /* Apply the "pipe_mode" control to the ImgU subdevice. */ > ControlList ctrls(imgu->imgu_->controls()); > ctrls.set(V4L2_CID_IPU3_PIPE_MODE, > - static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo : > + static_cast<int32_t>(vfActive ? IPU3PipeModeVideo : > IPU3PipeModeStillCapture)); > ret = imgu->imgu_->setControls(&ctrls); > if (ret) {
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 1f320dc166767bab..b72b631cdc051aab 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -33,13 +33,6 @@ LOG_DEFINE_CATEGORY(IPU3) class IPU3Stream : public Stream { -public: - IPU3Stream() - : active_(false) - { - } - - bool active_; }; class IPU3CameraData : public CameraData @@ -486,8 +479,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) return ret; /* Apply the format to the configured streams output devices. */ - outStream->active_ = false; - vfStream->active_ = false; + bool outActive = false; + bool vfActive = false; for (unsigned int i = 0; i < config->size(); ++i) { /* @@ -499,7 +492,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) IPU3Stream *stream = const_cast<IPU3Stream *>(config->streams()[i]); StreamConfiguration &cfg = (*config)[i]; - stream->active_ = true; cfg.setStream(stream); if (stream == outStream) { @@ -508,12 +500,14 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) return ret; cfg.stride = outputFormat.planes[0].bpl; + outActive = true; } else if (stream == vfStream) { ret = imgu->configureViewfinder(cfg, &outputFormat); if (ret) return ret; cfg.stride = outputFormat.planes[0].bpl; + vfActive = true; } else { /* * The RAW still capture stream just copies buffers from @@ -529,13 +523,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) * the configuration of the active one for that purpose (there should * be at least one active stream in the configuration request). */ - if (!outStream->active_) { + if (!outActive) { ret = imgu->configureOutput(config->at(0), &outputFormat); if (ret) return ret; } - if (!vfStream->active_) { + if (!vfActive) { ret = imgu->configureViewfinder(config->at(0), &outputFormat); if (ret) return ret; @@ -555,7 +549,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) /* Apply the "pipe_mode" control to the ImgU subdevice. */ ControlList ctrls(imgu->imgu_->controls()); ctrls.set(V4L2_CID_IPU3_PIPE_MODE, - static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo : + static_cast<int32_t>(vfActive ? IPU3PipeModeVideo : IPU3PipeModeStillCapture)); ret = imgu->imgu_->setControls(&ctrls); if (ret) {
The active_ flag is only used inside one function, remove the global flag and handle it inside the single function. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)