[libcamera-devel] [PATCH] libcamera: pipeline: simple: Use std::make_tuple()

[Laurent Pinchart]

Hi Kieran,

On Thu, Jul 16, 2020 at 03:06:08PM +0100, Kieran Bingham wrote:
> On 15/07/2020 10:35, Laurent Pinchart wrote:
> > On Wed, Jul 15, 2020 at 08:20:08AM +0200, Niklas Söderlund wrote:
> >> On 2020-07-15 02:33:25 +0300, Laurent Pinchart wrote:
> >>> Prior to N4387 ([1]), the std::tuple::tuple(const Types&...) constructor
> >>> was unconditionally explicit. gcc 5 doesn't implement N4387, leading to
> >>> a compilation error in the simple pipeline handler. Use
> >>> std::make_tuple() to fix it.
> >>>
> >>> [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4387.html
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>
> >> Maybe it's time we considered to drop support for gcc5 and possibly even 
> >> gcc6? But that is a topic for a another thread,
> > 
> > I wouldn't be against that.
> 
> I can't recall if we discussed on IRC or a thread, but indeed, if a
> distribution has a kernel older than we can support, then I don't think
> we need to support the build on that system either.
> 
> - Ubuntu only brought in kernel v5.0 at 18.04.3, and 18.04 ships with
> gcc v7.
> 
> - On Debian, Stretch(9) uses kernel v4.14, and buster(10) has v5.4.
> Buster ships with GCC v8.
> 
> - Fedora picks up a v5.0 kernel in Fedora 30, which ships gcc v9.
> 
> So I don't foresee much reason to keep gcc 5/6 support around ...
> Does anyone know of a distribution that uses a kernel >= 5.0 which ships
> gcc <= 6?

You've started a separate mail thread on that topic, so let's discuss it
there.

> >> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >>
> >>> ---
> >>>  src/libcamera/pipeline/simple/converter.cpp | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> >>> index dc7c046329f1..75fb297ebd58 100644
> >>> --- a/src/libcamera/pipeline/simple/converter.cpp
> >>> +++ b/src/libcamera/pipeline/simple/converter.cpp
> >>> @@ -271,9 +271,9 @@ SimpleConverter::strideAndFrameSize(const Size &size,
> >>>  
> >>>  	int ret = m2m_->capture()->tryFormat(&format);
> >>>  	if (ret < 0)
> >>> -		return { 0, 0 };
> >>> +		return std::make_tuple(0, 0);
> >>>  
> >>> -	return { format.planes[0].bpl, format.planes[0].size };
> >>> +	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
> 
> GCC support aside, I don't see anything wrong here in this patch though
> if you still want to integrate it...
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> >>>  }
> >>>  
> >>>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart