[libcamera-devel] [PATCH v2] libcamera: pipeline: Add IMX8 ISI pipeline

[Jacopo Mondi]

Hi Laurent

On Wed, Nov 16, 2022 at 10:12:26AM +0200, Laurent Pinchart wrote:

[snip]

> > > > +	/* Adjust all other streams to RAW. */
> > > > +	unsigned int i = 0;
> > > > +	for (StreamConfiguration &cfg : config_) {
> > >
> > > 	for (auto &[i, cfg] : utils::enumerate(config_)) {
> > >
> >
> > Nope, I started with that too
> >
> > ../src/libcamera/pipeline/imx8-isi/imx8-isi.cpp: In member function ‘libcamera::CameraConfiguration::Status libcamera::ISICameraConfiguration::validateRaw(std::set<libcamera::Stream*>&, const libcamera::Size&)’:
> > ../src/libcamera/pipeline/imx8-isi/imx8-isi.cpp:331:55: error: cannot bind non-const lvalue reference of type ‘std::pair<const long unsigned int, libcamera::StreamConfiguration&>&’ to an rvalue of type ‘libcamera::utils::details::enumerate_iterator<__gnu_cxx::__normal_iterator<libcamera::StreamConfiguration*, std::vector<libcamera::StreamConfiguration> > >::value_type’ {aka ‘std::pair<const long unsigned int, libcamera::StreamConfiguration&>’}
> >   331 |         for (auto &[i, cfg] : utils::enumerate(config_)) {
> >
> >   Cannot bind non-const lvalue reference of type ‘std::pair<const long unsigned int, libcamera::StreamConfiguration&>&’
> >   to an rvalue of type std::pair<const long unsigned int, libcamera::StreamConfiguration&>
> >
> > I can fix it by removing the reference, but then the
> > StreamCOnfiguration would be copied.
> >
> > I've not investigated why
>
>  	for (const auto &[i, cfg] : utils::enumerate(config_)) {
>
> Simple, right ? :-)
>
> It's a tricky one, utils::enumerate() returns a class instance
> (enumerate_adapter) that has begin() and end() functions. Those
> functions return an enumerate_iterator instance, whose value_type is a
> std::pair<const std::size_t, base_reference>. The C++ structured binding
> declaration creates a hidden variable e that holds the value of the
> initializer, so without const we would essentially do
>
> 	enumerate_iterator it = ...;
> 	auto &e = *it;
>
> enumerate_iterator::operator*() returns an rvalue, and you can't bind a
> non-const lvalue reference to an rvalue. The const qualifier fixes it.
>
> Then, the i and cfg names are bound to the first and second members of
> the hidden variable e. As e is a std::pair<const std::size_t, base_reference>,
> i takes the type const std::size, and cfg takes the type base_reference
> (which in this case is a StreamConfig &). The const qualifier in the
> expression
>
>  	for (const auto &[i, cfg] : utils::enumerate(config_)) {
>
> doesn't apply to i and cfg, only to the hidden variable e.
>

Ah, I didn't use the const version exactly because I wanted to modify
cfg.. Thanks for the explanation, if not other comments on v3, I can
change this when pushing.

Thanks
  j