| Message ID | 20240301212121.9072-10-laurent.pinchart@ideasonboard.com |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent On Fri, Mar 01, 2024 at 11:20:58PM +0200, Laurent Pinchart wrote: > The V4L2Subdevice class deals with streams in two places: > > - In routing tables, streams as expressed as a pad number and a stream > number in a v4l2_subdev_route instance. > - In the format and selection get and set functions, streams as > expressed using the Stream structure, which binds the pad number and > stream number. > > Expressing streams in different ways requires pipeline handlers and > other helpers to convert between the two representations. This isn't > much of an issue yet as libcamera has little stream-aware code, but it > is expected to increasingly become a burden. > > To simplify the API, introduce a V4L2Subdevice::Route structure that > mimicks the kernel v4l2_subdev_route structure but represents streams as > V4L2Subdevice::Stream instances. This will improve seamless integration > of routes, formats and selection rectangles. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > Changes since v1: > > - Clear routing at the beginning of getRouting() and setRouting() > --- > include/libcamera/internal/v4l2_subdevice.h | 19 +++- > src/libcamera/pipeline/simple/simple.cpp | 8 +- > src/libcamera/v4l2_subdevice.cpp | 106 ++++++++++++++++++-- > 3 files changed, 118 insertions(+), 15 deletions(-) > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h > index 2939dc2411c6..01ed4c2fc397 100644 > --- a/include/libcamera/internal/v4l2_subdevice.h > +++ b/include/libcamera/internal/v4l2_subdevice.h > @@ -95,7 +95,23 @@ public: > unsigned int stream; > }; > > - using Routing = std::vector<struct v4l2_subdev_route>; > + struct Route { > + Route() > + : flags(0) > + { > + } > + > + Route(const Stream &snk, const Stream &src, uint32_t f) > + : sink(snk), source(src), flags(f) > + { > + } > + > + Stream sink; > + Stream source; > + uint32_t flags; > + }; > + > + using Routing = std::vector<Route>; > > explicit V4L2Subdevice(const MediaEntity *entity); > ~V4L2Subdevice(); > @@ -174,6 +190,7 @@ static inline bool operator!=(const V4L2Subdevice::Stream &lhs, > } > > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream); > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route); > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index feea26fd124f..01f2a97798ba 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -852,17 +852,17 @@ std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink) > > std::vector<const MediaPad *> pads; > > - for (const struct v4l2_subdev_route &route : routing) { > - if (sink->index() != route.sink_pad || > + for (const V4L2Subdevice::Route &route : routing) { > + if (sink->index() != route.sink.pad || > !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE)) > continue; > > - const MediaPad *pad = entity->getPadByIndex(route.source_pad); > + const MediaPad *pad = entity->getPadByIndex(route.source.pad); > if (!pad) { > LOG(SimplePipeline, Warning) > << "Entity " << entity->name() > << " has invalid route source pad " > - << route.source_pad; > + << route.source.pad; > } > > pads.push_back(pad); > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > index 5dedfde4107f..3653f57a7d10 100644 > --- a/src/libcamera/v4l2_subdevice.cpp > +++ b/src/libcamera/v4l2_subdevice.cpp > @@ -870,6 +870,53 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) > return out; > } > > +/** > + * \class V4L2Subdevice::Route > + * \brief V4L2 subdevice routing table entry > + * > + * This class models a route in the subdevice routing table. It is similar to > + * the v4l2_subdev_route structure, but uses the V4L2Subdevice::Stream class > + * for easier usage with the V4L2Subdevice stream-aware functions. > + * > + * \var V4L2Subdevice::Route::sink > + * \brief The sink stream of the route > + * > + * \var V4L2Subdevice::Route::source > + * \brief The source stream of the route > + * > + * \var V4L2Subdevice::Route::flags > + * \brief The route flags (V4L2_SUBDEV_ROUTE_FL_*) > + */ > + > +/** > + * \fn V4L2Subdevice::Route::Route() > + * \brief Construct a Route with default streams > + */ > + > +/** > + * \fn V4L2Subdevice::Route::Route(const Stream &sink, const Stream &source, > + * uint32_t flags) > + * \brief Construct a Route from \a sink to \a source > + * \param[in] sink The sink stream > + * \param[in] source The source stream > + * \param[in] flags The route flags > + */ > + > +/** > + * \brief Insert a text representation of a V4L2Subdevice::Route into an > + * output stream Still not sure if this is intentional. > + * \param[in] out The output stream > + * \param[in] route The V4L2Subdevice::Route > + * \return The output stream \a out > + */ > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route) > +{ > + out << route.sink << " -> " << route.source > + << " (" << utils::hex(route.flags) << ")"; > + > + return out; > +} > + > /** > * \typedef V4L2Subdevice::Routing > * \brief V4L2 subdevice routing table > @@ -887,10 +934,7 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing) > { > for (const auto &[i, route] : utils::enumerate(routing)) { > - out << "[" << i << "] " > - << route.sink_pad << "/" << route.sink_stream << " -> " > - << route.source_pad << "/" << route.source_stream > - << " (" << utils::hex(route.flags) << ")"; > + out << "[" << i << "] " << route; > if (i != routing.size() - 1) > out << ", "; > } > @@ -1244,6 +1288,30 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format, > * \return 0 on success or a negative error code otherwise > */ > > +namespace { > + > +void routeFromKernel(V4L2Subdevice::Route &route, > + const struct v4l2_subdev_route &kroute) > +{ > + route.sink.pad = kroute.sink_pad; > + route.sink.stream = kroute.sink_stream; > + route.source.pad = kroute.source_pad; > + route.source.stream = kroute.source_stream; > + route.flags = kroute.flags; > +} > + > +void routeToKernel(const V4L2Subdevice::Route &route, > + struct v4l2_subdev_route &kroute) > +{ > + kroute.sink_pad = route.sink.pad; > + kroute.sink_stream = route.sink.stream; > + kroute.source_pad = route.source.pad; > + kroute.source_stream = route.source.stream; > + kroute.flags = route.flags; > +} Static helpers always feel a bit meh to me.. have you considered Route::fromKernelRoute(const struct v4l2_subdev_route &kroute); Route::toKernelRoute(struct v4l2_subdev_route *kroute); and decided static helpers are better ? > + > +} /* namespace */ > + > /** > * \brief Retrieve the subdevice's internal routing table > * \param[out] routing The routing table > @@ -1254,6 +1322,8 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format, > */ > int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > { > + routing->clear(); > + > if (!caps_.hasStreams()) > return 0; > > @@ -1272,8 +1342,8 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > return ret; > } > > - routing->resize(rt.num_routes); > - rt.routes = reinterpret_cast<uintptr_t>(routing->data()); > + std::vector<struct v4l2_subdev_route> routes{ rt.num_routes }; > + rt.routes = reinterpret_cast<uintptr_t>(routes.data()); > > ret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt); > if (ret) { > @@ -1282,11 +1352,16 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > return ret; > } > > - if (rt.num_routes != routing->size()) { > + if (rt.num_routes != routes.size()) { > LOG(V4L2, Error) << "Invalid number of routes"; > return -EINVAL; > } > > + routing->resize(rt.num_routes); > + > + for (const auto &[i, route] : utils::enumerate(routes)) > + routeFromKernel((*routing)[i], route); > + > return 0; > } > > @@ -1304,13 +1379,20 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > */ > int V4L2Subdevice::setRouting(Routing *routing, Whence whence) > { > - if (!caps_.hasStreams()) > + if (!caps_.hasStreams()) { > + routing->clear(); > return 0; > + } > + > + std::vector<struct v4l2_subdev_route> routes{ routing->size() }; > + > + for (const auto &[i, route] : utils::enumerate(*routing)) > + routeToKernel(route, routes[i]); > > struct v4l2_subdev_routing rt = {}; > rt.which = whence; > - rt.num_routes = routing->size(); > - rt.routes = reinterpret_cast<uintptr_t>(routing->data()); > + rt.num_routes = routes.size(); > + rt.routes = reinterpret_cast<uintptr_t>(routes.data()); > > int ret = ioctl(VIDIOC_SUBDEV_S_ROUTING, &rt); > if (ret) { > @@ -1318,8 +1400,12 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence) > return ret; > } > > + routes.resize(rt.num_routes); > routing->resize(rt.num_routes); > > + for (const auto &[i, route] : utils::enumerate(routes)) > + routeFromKernel((*routing)[i], route); > + > return 0; With or without the static helpers Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > } > > -- > Regards, > > Laurent Pinchart >
On Tue, Mar 05, 2024 at 09:55:58AM +0100, Jacopo Mondi wrote: > Hi Laurent > > On Fri, Mar 01, 2024 at 11:20:58PM +0200, Laurent Pinchart wrote: > > The V4L2Subdevice class deals with streams in two places: > > > > - In routing tables, streams as expressed as a pad number and a stream > > number in a v4l2_subdev_route instance. > > - In the format and selection get and set functions, streams as > > expressed using the Stream structure, which binds the pad number and > > stream number. > > > > Expressing streams in different ways requires pipeline handlers and > > other helpers to convert between the two representations. This isn't > > much of an issue yet as libcamera has little stream-aware code, but it > > is expected to increasingly become a burden. > > > > To simplify the API, introduce a V4L2Subdevice::Route structure that > > mimicks the kernel v4l2_subdev_route structure but represents streams as > > V4L2Subdevice::Stream instances. This will improve seamless integration > > of routes, formats and selection rectangles. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > Changes since v1: > > > > - Clear routing at the beginning of getRouting() and setRouting() > > --- > > include/libcamera/internal/v4l2_subdevice.h | 19 +++- > > src/libcamera/pipeline/simple/simple.cpp | 8 +- > > src/libcamera/v4l2_subdevice.cpp | 106 ++++++++++++++++++-- > > 3 files changed, 118 insertions(+), 15 deletions(-) > > > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h > > index 2939dc2411c6..01ed4c2fc397 100644 > > --- a/include/libcamera/internal/v4l2_subdevice.h > > +++ b/include/libcamera/internal/v4l2_subdevice.h > > @@ -95,7 +95,23 @@ public: > > unsigned int stream; > > }; > > > > - using Routing = std::vector<struct v4l2_subdev_route>; > > + struct Route { > > + Route() > > + : flags(0) > > + { > > + } > > + > > + Route(const Stream &snk, const Stream &src, uint32_t f) > > + : sink(snk), source(src), flags(f) > > + { > > + } > > + > > + Stream sink; > > + Stream source; > > + uint32_t flags; > > + }; > > + > > + using Routing = std::vector<Route>; > > > > explicit V4L2Subdevice(const MediaEntity *entity); > > ~V4L2Subdevice(); > > @@ -174,6 +190,7 @@ static inline bool operator!=(const V4L2Subdevice::Stream &lhs, > > } > > > > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream); > > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route); > > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing); > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index feea26fd124f..01f2a97798ba 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -852,17 +852,17 @@ std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink) > > > > std::vector<const MediaPad *> pads; > > > > - for (const struct v4l2_subdev_route &route : routing) { > > - if (sink->index() != route.sink_pad || > > + for (const V4L2Subdevice::Route &route : routing) { > > + if (sink->index() != route.sink.pad || > > !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE)) > > continue; > > > > - const MediaPad *pad = entity->getPadByIndex(route.source_pad); > > + const MediaPad *pad = entity->getPadByIndex(route.source.pad); > > if (!pad) { > > LOG(SimplePipeline, Warning) > > << "Entity " << entity->name() > > << " has invalid route source pad " > > - << route.source_pad; > > + << route.source.pad; > > } > > > > pads.push_back(pad); > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > index 5dedfde4107f..3653f57a7d10 100644 > > --- a/src/libcamera/v4l2_subdevice.cpp > > +++ b/src/libcamera/v4l2_subdevice.cpp > > @@ -870,6 +870,53 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) > > return out; > > } > > > > +/** > > + * \class V4L2Subdevice::Route > > + * \brief V4L2 subdevice routing table entry > > + * > > + * This class models a route in the subdevice routing table. It is similar to > > + * the v4l2_subdev_route structure, but uses the V4L2Subdevice::Stream class > > + * for easier usage with the V4L2Subdevice stream-aware functions. > > + * > > + * \var V4L2Subdevice::Route::sink > > + * \brief The sink stream of the route > > + * > > + * \var V4L2Subdevice::Route::source > > + * \brief The source stream of the route > > + * > > + * \var V4L2Subdevice::Route::flags > > + * \brief The route flags (V4L2_SUBDEV_ROUTE_FL_*) > > + */ > > + > > +/** > > + * \fn V4L2Subdevice::Route::Route() > > + * \brief Construct a Route with default streams > > + */ > > + > > +/** > > + * \fn V4L2Subdevice::Route::Route(const Stream &sink, const Stream &source, > > + * uint32_t flags) > > + * \brief Construct a Route from \a sink to \a source > > + * \param[in] sink The sink stream > > + * \param[in] source The source stream > > + * \param[in] flags The route flags > > + */ > > + > > +/** > > + * \brief Insert a text representation of a V4L2Subdevice::Route into an > > + * output stream > > Still not sure if this is intentional. > > > + * \param[in] out The output stream > > + * \param[in] route The V4L2Subdevice::Route > > + * \return The output stream \a out > > + */ > > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route) > > +{ > > + out << route.sink << " -> " << route.source > > + << " (" << utils::hex(route.flags) << ")"; > > + > > + return out; > > +} > > + > > /** > > * \typedef V4L2Subdevice::Routing > > * \brief V4L2 subdevice routing table > > @@ -887,10 +934,7 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) > > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing) > > { > > for (const auto &[i, route] : utils::enumerate(routing)) { > > - out << "[" << i << "] " > > - << route.sink_pad << "/" << route.sink_stream << " -> " > > - << route.source_pad << "/" << route.source_stream > > - << " (" << utils::hex(route.flags) << ")"; > > + out << "[" << i << "] " << route; > > if (i != routing.size() - 1) > > out << ", "; > > } > > @@ -1244,6 +1288,30 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format, > > * \return 0 on success or a negative error code otherwise > > */ > > > > +namespace { > > + > > +void routeFromKernel(V4L2Subdevice::Route &route, > > + const struct v4l2_subdev_route &kroute) > > +{ > > + route.sink.pad = kroute.sink_pad; > > + route.sink.stream = kroute.sink_stream; > > + route.source.pad = kroute.source_pad; > > + route.source.stream = kroute.source_stream; > > + route.flags = kroute.flags; > > +} > > + > > +void routeToKernel(const V4L2Subdevice::Route &route, > > + struct v4l2_subdev_route &kroute) > > +{ > > + kroute.sink_pad = route.sink.pad; > > + kroute.sink_stream = route.sink.stream; > > + kroute.source_pad = route.source.pad; > > + kroute.source_stream = route.source.stream; > > + kroute.flags = route.flags; > > +} > > Static helpers always feel a bit meh to me.. have you considered > > Route::fromKernelRoute(const struct v4l2_subdev_route &kroute); > Route::toKernelRoute(struct v4l2_subdev_route *kroute); > > and decided static helpers are better ? The idea is to make those internal, I don't think they need to be exposed in the API. The whole point of the Route structure is to avoid exposing v4l2_subdev_route :-) > > + > > +} /* namespace */ > > + > > /** > > * \brief Retrieve the subdevice's internal routing table > > * \param[out] routing The routing table > > @@ -1254,6 +1322,8 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format, > > */ > > int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > > { > > + routing->clear(); > > + > > if (!caps_.hasStreams()) > > return 0; > > > > @@ -1272,8 +1342,8 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > > return ret; > > } > > > > - routing->resize(rt.num_routes); > > - rt.routes = reinterpret_cast<uintptr_t>(routing->data()); > > + std::vector<struct v4l2_subdev_route> routes{ rt.num_routes }; > > + rt.routes = reinterpret_cast<uintptr_t>(routes.data()); > > > > ret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt); > > if (ret) { > > @@ -1282,11 +1352,16 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > > return ret; > > } > > > > - if (rt.num_routes != routing->size()) { > > + if (rt.num_routes != routes.size()) { > > LOG(V4L2, Error) << "Invalid number of routes"; > > return -EINVAL; > > } > > > > + routing->resize(rt.num_routes); > > + > > + for (const auto &[i, route] : utils::enumerate(routes)) > > + routeFromKernel((*routing)[i], route); > > + > > return 0; > > } > > > > @@ -1304,13 +1379,20 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) > > */ > > int V4L2Subdevice::setRouting(Routing *routing, Whence whence) > > { > > - if (!caps_.hasStreams()) > > + if (!caps_.hasStreams()) { > > + routing->clear(); > > return 0; > > + } > > + > > + std::vector<struct v4l2_subdev_route> routes{ routing->size() }; > > + > > + for (const auto &[i, route] : utils::enumerate(*routing)) > > + routeToKernel(route, routes[i]); > > > > struct v4l2_subdev_routing rt = {}; > > rt.which = whence; > > - rt.num_routes = routing->size(); > > - rt.routes = reinterpret_cast<uintptr_t>(routing->data()); > > + rt.num_routes = routes.size(); > > + rt.routes = reinterpret_cast<uintptr_t>(routes.data()); > > > > int ret = ioctl(VIDIOC_SUBDEV_S_ROUTING, &rt); > > if (ret) { > > @@ -1318,8 +1400,12 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence) > > return ret; > > } > > > > + routes.resize(rt.num_routes); > > routing->resize(rt.num_routes); > > > > + for (const auto &[i, route] : utils::enumerate(routes)) > > + routeFromKernel((*routing)[i], route); > > + > > return 0; > > With or without the static helpers > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > } > >
diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h index 2939dc2411c6..01ed4c2fc397 100644 --- a/include/libcamera/internal/v4l2_subdevice.h +++ b/include/libcamera/internal/v4l2_subdevice.h @@ -95,7 +95,23 @@ public: unsigned int stream; }; - using Routing = std::vector<struct v4l2_subdev_route>; + struct Route { + Route() + : flags(0) + { + } + + Route(const Stream &snk, const Stream &src, uint32_t f) + : sink(snk), source(src), flags(f) + { + } + + Stream sink; + Stream source; + uint32_t flags; + }; + + using Routing = std::vector<Route>; explicit V4L2Subdevice(const MediaEntity *entity); ~V4L2Subdevice(); @@ -174,6 +190,7 @@ static inline bool operator!=(const V4L2Subdevice::Stream &lhs, } std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream); +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route); std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index feea26fd124f..01f2a97798ba 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -852,17 +852,17 @@ std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink) std::vector<const MediaPad *> pads; - for (const struct v4l2_subdev_route &route : routing) { - if (sink->index() != route.sink_pad || + for (const V4L2Subdevice::Route &route : routing) { + if (sink->index() != route.sink.pad || !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE)) continue; - const MediaPad *pad = entity->getPadByIndex(route.source_pad); + const MediaPad *pad = entity->getPadByIndex(route.source.pad); if (!pad) { LOG(SimplePipeline, Warning) << "Entity " << entity->name() << " has invalid route source pad " - << route.source_pad; + << route.source.pad; } pads.push_back(pad); diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp index 5dedfde4107f..3653f57a7d10 100644 --- a/src/libcamera/v4l2_subdevice.cpp +++ b/src/libcamera/v4l2_subdevice.cpp @@ -870,6 +870,53 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) return out; } +/** + * \class V4L2Subdevice::Route + * \brief V4L2 subdevice routing table entry + * + * This class models a route in the subdevice routing table. It is similar to + * the v4l2_subdev_route structure, but uses the V4L2Subdevice::Stream class + * for easier usage with the V4L2Subdevice stream-aware functions. + * + * \var V4L2Subdevice::Route::sink + * \brief The sink stream of the route + * + * \var V4L2Subdevice::Route::source + * \brief The source stream of the route + * + * \var V4L2Subdevice::Route::flags + * \brief The route flags (V4L2_SUBDEV_ROUTE_FL_*) + */ + +/** + * \fn V4L2Subdevice::Route::Route() + * \brief Construct a Route with default streams + */ + +/** + * \fn V4L2Subdevice::Route::Route(const Stream &sink, const Stream &source, + * uint32_t flags) + * \brief Construct a Route from \a sink to \a source + * \param[in] sink The sink stream + * \param[in] source The source stream + * \param[in] flags The route flags + */ + +/** + * \brief Insert a text representation of a V4L2Subdevice::Route into an + * output stream + * \param[in] out The output stream + * \param[in] route The V4L2Subdevice::Route + * \return The output stream \a out + */ +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route) +{ + out << route.sink << " -> " << route.source + << " (" << utils::hex(route.flags) << ")"; + + return out; +} + /** * \typedef V4L2Subdevice::Routing * \brief V4L2 subdevice routing table @@ -887,10 +934,7 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing) { for (const auto &[i, route] : utils::enumerate(routing)) { - out << "[" << i << "] " - << route.sink_pad << "/" << route.sink_stream << " -> " - << route.source_pad << "/" << route.source_stream - << " (" << utils::hex(route.flags) << ")"; + out << "[" << i << "] " << route; if (i != routing.size() - 1) out << ", "; } @@ -1244,6 +1288,30 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format, * \return 0 on success or a negative error code otherwise */ +namespace { + +void routeFromKernel(V4L2Subdevice::Route &route, + const struct v4l2_subdev_route &kroute) +{ + route.sink.pad = kroute.sink_pad; + route.sink.stream = kroute.sink_stream; + route.source.pad = kroute.source_pad; + route.source.stream = kroute.source_stream; + route.flags = kroute.flags; +} + +void routeToKernel(const V4L2Subdevice::Route &route, + struct v4l2_subdev_route &kroute) +{ + kroute.sink_pad = route.sink.pad; + kroute.sink_stream = route.sink.stream; + kroute.source_pad = route.source.pad; + kroute.source_stream = route.source.stream; + kroute.flags = route.flags; +} + +} /* namespace */ + /** * \brief Retrieve the subdevice's internal routing table * \param[out] routing The routing table @@ -1254,6 +1322,8 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format, */ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) { + routing->clear(); + if (!caps_.hasStreams()) return 0; @@ -1272,8 +1342,8 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) return ret; } - routing->resize(rt.num_routes); - rt.routes = reinterpret_cast<uintptr_t>(routing->data()); + std::vector<struct v4l2_subdev_route> routes{ rt.num_routes }; + rt.routes = reinterpret_cast<uintptr_t>(routes.data()); ret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt); if (ret) { @@ -1282,11 +1352,16 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) return ret; } - if (rt.num_routes != routing->size()) { + if (rt.num_routes != routes.size()) { LOG(V4L2, Error) << "Invalid number of routes"; return -EINVAL; } + routing->resize(rt.num_routes); + + for (const auto &[i, route] : utils::enumerate(routes)) + routeFromKernel((*routing)[i], route); + return 0; } @@ -1304,13 +1379,20 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence) */ int V4L2Subdevice::setRouting(Routing *routing, Whence whence) { - if (!caps_.hasStreams()) + if (!caps_.hasStreams()) { + routing->clear(); return 0; + } + + std::vector<struct v4l2_subdev_route> routes{ routing->size() }; + + for (const auto &[i, route] : utils::enumerate(*routing)) + routeToKernel(route, routes[i]); struct v4l2_subdev_routing rt = {}; rt.which = whence; - rt.num_routes = routing->size(); - rt.routes = reinterpret_cast<uintptr_t>(routing->data()); + rt.num_routes = routes.size(); + rt.routes = reinterpret_cast<uintptr_t>(routes.data()); int ret = ioctl(VIDIOC_SUBDEV_S_ROUTING, &rt); if (ret) { @@ -1318,8 +1400,12 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence) return ret; } + routes.resize(rt.num_routes); routing->resize(rt.num_routes); + for (const auto &[i, route] : utils::enumerate(routes)) + routeFromKernel((*routing)[i], route); + return 0; }
The V4L2Subdevice class deals with streams in two places: - In routing tables, streams as expressed as a pad number and a stream number in a v4l2_subdev_route instance. - In the format and selection get and set functions, streams as expressed using the Stream structure, which binds the pad number and stream number. Expressing streams in different ways requires pipeline handlers and other helpers to convert between the two representations. This isn't much of an issue yet as libcamera has little stream-aware code, but it is expected to increasingly become a burden. To simplify the API, introduce a V4L2Subdevice::Route structure that mimicks the kernel v4l2_subdev_route structure but represents streams as V4L2Subdevice::Stream instances. This will improve seamless integration of routes, formats and selection rectangles. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- Changes since v1: - Clear routing at the beginning of getRouting() and setRouting() --- include/libcamera/internal/v4l2_subdevice.h | 19 +++- src/libcamera/pipeline/simple/simple.cpp | 8 +- src/libcamera/v4l2_subdevice.cpp | 106 ++++++++++++++++++-- 3 files changed, 118 insertions(+), 15 deletions(-)