[libcamera-devel] [PATCH] libcamera: pipeline: vimc: create camera for Sensor B

[Niklas Söderlund]

Hi Kieran,

Thanks for your feedback.

On 2019-02-04 16:40:43 +0100, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 01/02/2019 16:13, Niklas Söderlund wrote:
> > Create a camera for Sensor B of the vimc pipeline. At this stage only
> > Sensor B is exposed while the are in total two sensors in the graph.
> > Going forward Sensor A should also be exposed as another camera. Sensor
> > B was chosen since the graph is configured correctly for it out of the
> > box. As we lack control over sub devices formats it's not really
> > possible to use Sensor A yet with the library.
> > 
> > Once both cameras are registered with the library they should be
> > extended to provide two streams each, one for the raw output and one for
> > the output which uses the shared scaler.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  src/libcamera/pipeline/vimc.cpp | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index 6ed069edec550a61..6ab6e203785499ec 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -12,6 +12,7 @@
> >  #include "log.h"
> >  #include "media_device.h"
> >  #include "pipeline_handler.h"
> > +#include "v4l2_device.h"
> >  
> >  namespace libcamera {
> >  
> > @@ -33,16 +34,20 @@ public:
> >  
> >  private:
> >  	std::shared_ptr<MediaDevice> media_;
> > +	V4L2Device *video_;
> >  	Stream stream_;
> >  };
> >  
> >  PipeHandlerVimc::PipeHandlerVimc(CameraManager *manager)
> > -	: PipelineHandler(manager), media_(nullptr)
> > +	: PipelineHandler(manager), media_(nullptr), video_(nullptr)
> >  {
> >  }
> >  
> >  PipeHandlerVimc::~PipeHandlerVimc()
> >  {
> > +	if (video_)
> > +		delete video_;
> 
> I don't think you need the if (video_) here.
> delete (nullptr); should be safe.
> 
> Other than that,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

With this fixed pushed to master.

> 
> 
> > +
> >  	if (media_)
> >  		media_->release();
> >  }
> > @@ -93,8 +98,16 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
> >  
> >  	media_->acquire();
> >  
> > +	video_ = new V4L2Device(*media_->getEntityByName("Raw Capture 1"));
> > +
> > +	if (video_->open()) {
> > +		media_->release();
> > +		return false;
> > +	}
> > +
> >  	std::vector<Stream *> streams{ &stream_ };
> > -	std::shared_ptr<Camera> camera = Camera::create(this, "Dummy VIMC Camera", streams);
> > +	std::shared_ptr<Camera> camera = Camera::create(this, "VIMC Sensor B",
> > +							streams);
> >  	registerCamera(std::move(camera));
> >  
> >  	return true;
> > 
> 
> -- 
> Regards
> --
> Kieran

-- 
Regards,
Niklas Söderlund