[libcamera-devel] [RFC PATCH v3 1/2] libcamera: camera: Take span of StreamRole instead of vector

[Nicolas Dufresne]

Hi,

Le jeudi 25 mai 2023 à 22:41 +0000, Barnabás Pőcze via libcamera-devel a écrit :
> Hi
> 
> 
> 2023. május 18., csütörtök 18:16 keltezéssel, Kieran Bingham <kieran.bingham at ideasonboard.com> írta:
> 
> > Quoting Barnabás Pőcze (2023-05-18 16:17:41)
> > > Hi
> > > 
> > > 
> > > 2023. május 16., kedd 1:28 keltezéssel, Kieran Bingham <kieran.bingham at ideasonboard.com> írta:
> > > 
> > > > Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:07:57)
> > > > > Change the parameter type of `generateConfiguration()` from `const std::vector&`
> > > > > to `libcamera::Span`. A span is almost always preferable to a const vector ref
> > > > > because it does not force dynamic allocation when none are needed, and it allows
> > > > > any contiguous container to be used.
> > > > 
> > > > Sounds like a reasonable move to me.
> > > > 
> > > > > 
> > > > > A new overload is added that accepts an initializer list so that
> > > > > 
> > > > >   cam->generateConfiguration({ ... })
> > > > > 
> > > > > keeps working.
> > > > > 
> > > > > There is no API break since a span can be constructed from a vector
> > > > > and the initializer list overload takes care of the initializer lists,
> > > > > but this change causes an ABI break.
> > > > 
> > > > That's fine too. I'd be interested how the abi-compliance-checker
> > > > reports this:
> > > > 
> > > >  Creating compatibility report ...
> > > >  Binary compatibility: 100%
> > > >  Source compatibility: 99.9%
> > > >  Total binary compatibility problems: 0, warnings: 0
> > > >  Total source compatibility problems: 2, warnings: 0
> > > >  Report: compat_reports/libcamera/v0.0.5_to_v0.0.5-22-g5c94fbcc9d05/compat_report.html
> > > > 
> > > > Well - at least it noticed something ;-)
> > > 
> > > Unfortunately I think that is incorrect. This is definitely an ABI break.
> > > (And as far as I can tell not an API break.)
> > > 
> > 
> > Hrm... a little worrying for the abi-compliance-checker then...
> 
> Well, to be fair, it *is* an API break as well, but very unlikely to be
> noticed by anyone. I don't know why it does not notice the ABI break,
> the mangled names are completely different...

Removing symbols is an API break. Changing the size of a structure in a way that
both the application and the lib may endup with different size interpretation is
an ABI break.

I think what you are are saying is that this is build compatible with the old
version, avoiding the pain of porting, but a rebuilt is needed in order to use
the new symbols (which in C++ are generated, but they remains API in the context
of generic checkers).

> 
> 
> > [...]
> 
> 
> Regards,
> Barnabás Pőcze