[libcamera-devel] [PATCH v2 10/10] libcamera: pipeline: simple: Integrate converter support

[Laurent Pinchart]

Hi Andrey,

On Fri, Mar 20, 2020 at 01:07:06AM +0300, Andrey Konovalov wrote:
> On 17.03.2020 00:43, Laurent Pinchart wrote:
> > Add support for an optional format converter, supported by the
> > SimpleConverter class. If a converter is available for the pipeline, it
> > will be used to expose additional pixel formats.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >   src/libcamera/pipeline/simple/simple.cpp | 192 ++++++++++++++++++++---
> >   1 file changed, 174 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 2126799c54eb..218b28e70eb2 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> 
> <snip>
> 
> > +struct SimplePipelineInfo {
> > +	const char *driver;
> > +	const char *converter;
> > +};
> 
> <snip>
> 
> > @@ -550,16 +630,21 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
> >   
> >   bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> >   {
> > -	static const char * const drivers[] = {
> > -		"imx7-csi",
> > -		"sun6i-csi",
> > +	static const SimplePipelineInfo infos[] = {
> > +		{ "imx7-csi", "pxp" },
> > +		{ "sun6i-csi", nullptr },
> >   	};
> >   
> > -	for (const char *driver : drivers) {
> > -		DeviceMatch dm(driver);
> > +	MediaDevice *converter;
> > +
> > +	for (const SimplePipelineInfo &info : infos) {
> > +		DeviceMatch dm(info.driver);
> >   		media_ = acquireMediaDevice(enumerator, dm);
> > -		if (media_)
> > +		if (media_) {
> > +			DeviceMatch converterMatch(info.converter);
> 
> info.converter being equal to nullptr here results in:
> 
>    what():  basic_string::_M_construct null not valid
> Aborted
> 
> - as DeviceMatch constructor accepts const std::string.
> 
> Maybe use { "sun6i-csi", "" } instead of { "sun6i-csi", nullptr }?

Oops :-/ We could do that, but while testing I've received a warning
from some versions of gcc that the converter variable could be used
uninitialized. I think that's a false positive, but reworking the code
to support nullptr helps fixing that, so I'll go that way.

-- 
Regards,

Laurent Pinchart