| Message ID | 20240801073339.4061027-3-chenghaoyang@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Chenghao, Thank you for the patch. On Thu, Aug 01, 2024 at 07:30:58AM +0000, Harvey Yang wrote: > The Fatal check of having at least one MediaDevice was to prevent > pipeline handler implementations searching and owning media devices with > custom conventions, instead of using the base function > |acquireMediaDevice|. It also has the assumption that there's at least > one media device to make a camera work. > > Now that the assumption will be broken by the virtual pipeline handler > added in the following patches, and developers should be aware of the > available functions in the base class to handle media devices, the Fatal > check is no longer needed. > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> That's a bit of a hack. It would be nicer to rework the device matching logic to support other types of devices than media devices. That would be quite a lot of yak shaving though, I suppose we can live with this patch for now. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/pipeline_handler.cpp | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 5ea2ca78..125cf9d5 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -605,9 +605,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > { > cameras_.push_back(camera); > > - if (mediaDevices_.empty()) > - LOG(Pipeline, Fatal) > - << "Registering camera with no media devices!"; > + if (mediaDevices_.empty()) { > + /* > + * For virtual devices with no MediaDevice, there are no system > + * devices to register. > + */ > + manager_->_d()->addCamera(std::move(camera)); > + return; > + } > > /* > * Walk the entity list and map the devnums of all capture video nodes
Hi On Sat, Aug 03, 2024 at 11:49:12PM GMT, Laurent Pinchart wrote: > Hi Chenghao, > > Thank you for the patch. > > On Thu, Aug 01, 2024 at 07:30:58AM +0000, Harvey Yang wrote: > > The Fatal check of having at least one MediaDevice was to prevent > > pipeline handler implementations searching and owning media devices with > > custom conventions, instead of using the base function > > |acquireMediaDevice|. It also has the assumption that there's at least > > one media device to make a camera work. > > > > Now that the assumption will be broken by the virtual pipeline handler > > added in the following patches, and developers should be aware of the > > available functions in the base class to handle media devices, the Fatal > > check is no longer needed. > > > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > > That's a bit of a hack. It would be nicer to rework the device matching > logic to support other types of devices than media devices. That would > be quite a lot of yak shaving though, I suppose we can live with this > patch for now. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> More or less 1 year ago I worked on generalizing device matching to support non-UVC compatible USB devices. Is there anything that can be useful here ? https://git.libcamera.org/libcamera/jmondi/libcamera.git/log/?h=jmondi/device-match-generalize > > > --- > > src/libcamera/pipeline_handler.cpp | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index 5ea2ca78..125cf9d5 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -605,9 +605,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > > { > > cameras_.push_back(camera); > > > > - if (mediaDevices_.empty()) > > - LOG(Pipeline, Fatal) > > - << "Registering camera with no media devices!"; > > + if (mediaDevices_.empty()) { > > + /* > > + * For virtual devices with no MediaDevice, there are no system > > + * devices to register. > > + */ > > + manager_->_d()->addCamera(std::move(camera)); > > + return; > > + } > > > > /* > > * Walk the entity list and map the devnums of all capture video nodes > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 5ea2ca78..125cf9d5 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -605,9 +605,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) { cameras_.push_back(camera); - if (mediaDevices_.empty()) - LOG(Pipeline, Fatal) - << "Registering camera with no media devices!"; + if (mediaDevices_.empty()) { + /* + * For virtual devices with no MediaDevice, there are no system + * devices to register. + */ + manager_->_d()->addCamera(std::move(camera)); + return; + } /* * Walk the entity list and map the devnums of all capture video nodes
The Fatal check of having at least one MediaDevice was to prevent pipeline handler implementations searching and owning media devices with custom conventions, instead of using the base function |acquireMediaDevice|. It also has the assumption that there's at least one media device to make a camera work. Now that the assumption will be broken by the virtual pipeline handler added in the following patches, and developers should be aware of the available functions in the base class to handle media devices, the Fatal check is no longer needed. Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> --- src/libcamera/pipeline_handler.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)