[libcamera-devel] [RFC PATCH v3 2/2] libcamera: Remove `StreamRoles` alias

[Kieran Bingham]

Quoting Barnabás Pőcze (2023-05-18 16:29:10)
> Hi
> 
> 
> 2023. május 16., kedd 1:09 keltezéssel, Kieran Bingham <kieran.bingham at ideasonboard.com> írta:
> 
> > Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:15:43)
> > > Now that `Camera::generateConfiguration()` takes a `libcamera::Span`
> > > of `StreamRole`, remove the `StreamRoles` type, which was an alias
> > > to `std::vector<libcamera::StreamRole>`.
> > >
> > > The removal has two reasons:
> > >  - it is no longer strictly necessary,
> > >  - its presence may suggest that that is the preferred (or correct)
> > >    way to build/pass a list of `StreamRole`.
> > >
> > > Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> > > ---
> > >  include/libcamera/stream.h             | 2 --
> > >  src/apps/cam/camera_session.cpp        | 2 +-
> > >  src/apps/common/stream_options.cpp     | 4 ++--
> > >  src/apps/common/stream_options.h       | 2 +-
> > >  src/apps/qcam/main_window.cpp          | 2 +-
> > >  src/gstreamer/gstlibcameraprovider.cpp | 5 +++--
> > >  src/gstreamer/gstlibcamerasrc.cpp      | 2 +-
> > >  src/libcamera/stream.cpp               | 5 -----
> > >  8 files changed, 9 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > > index 29235ddf..4e94187d 100644
> > > --- a/include/libcamera/stream.h
> > > +++ b/include/libcamera/stream.h
> > > @@ -69,8 +69,6 @@ enum class StreamRole {
> > >         Viewfinder,
> > >  };
> > >
> > > -using StreamRoles = std::vector<StreamRole>;
> > > -
> > 
> > I was curious how we might handle this moving forwards if we need to
> > remove things. (I think removing this is the right thing to do, so this
> > is just 'how' we remove it)
> > 
> > I wonder if we should have an include/libcamera/deprecated.h to place
> > things like:
> > 
> > using StreamRoles [[deprecated("Use a span, array or vector directly")]]
> >         = std::vector<StreamRole>;
> > 
> > Perhaps even with a reference to something that makes it clearer what to
> > do to migrate with the issue.
> > 
> > StreamRoles has been in all the examples so far, so I think all apps
> > that use libcamera are probably already using this.
> > 
> > I know we explicitly don't have ABI/API stability - but if we break
> > things perhaps it would be helpful to have a system that lets us inform
> > the user what they need to do next to fix it again.
> > 
> > This probably gets quite relevant to how we handle things with:
> > https://patchwork.libcamera.org/project/libcamera/list/?series=3877 so
> > any thoughts on that series are welcome!
> 
> Maybe it could be kept in the same header file? Otherwise users will still
> encounter a compiler error and have to investigate what happened, no?

If we go down the 'deprecated' route then I would have a new
deprecated.h which is included by the main libcamera.h header so the
definitions would be available to applications without them changing -
except they would now get the deprecated warning (or error).

But by moving it - it would be easier to locate to remove old
deprecations at each release point.

 Developer makes breaking change
  - Warning/update moved to deprecated.h to inform applciations

 Release n+1 made with that in deprecated.h

 .... more commits .... / time ....

 change removed from deprecated.h

 Release n+2 made. No longer carried in deprecated.h


--
Kieran



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