| Message ID | 20230105043726.679968-4-chenghaoyang@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Harvey, I'm currently using your patch set as the basis for my own work and I've found a bug while using it. The function `createDevice` doesn't create a MediaDevice, but a `MediaDeviceBase` instead during device enumeration. As such previously working non-virtual cameras aren't detected by the pipeline handlers anymore and the whole system doesn't work. On my local branch I've been able to fix this by making `createDevice` a templated function to allow selection which child of MediaDeviceBase should be initiated. I would disencourage any movement of pinning createDevice down to only be able to initiate a `MediaDevice` object, as I use your work in order to make USB connected devices discoverable, which are independent of `MediaDevice` On 05/01/2023 05:37, Cheng-Hao Yang wrote: > Use MediaDeviceBase instead of MediaDevice in other base classes > like PipelineHandler and DeviceEnumerator. > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > --- > .../libcamera/internal/device_enumerator.h | 12 ++++---- > .../internal/device_enumerator_sysfs.h | 4 +-- > .../internal/device_enumerator_udev.h | 8 ++--- > include/libcamera/internal/pipeline_handler.h | 12 ++++---- > src/libcamera/device_enumerator.cpp | 30 +++++++++---------- > src/libcamera/device_enumerator_sysfs.cpp | 4 +-- > src/libcamera/device_enumerator_udev.cpp | 4 +-- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 2 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-- > .../pipeline/raspberrypi/raspberrypi.cpp | 4 +-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/pipeline/simple/simple.cpp | 4 +-- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc/vimc.cpp | 2 +- > src/libcamera/pipeline_handler.cpp | 18 +++++------ > 15 files changed, 56 insertions(+), 56 deletions(-) > > diff --git a/include/libcamera/internal/device_enumerator.h b/include/libcamera/internal/device_enumerator.h > index 72ec9a60..87a2b5ce 100644 > --- a/include/libcamera/internal/device_enumerator.h > +++ b/include/libcamera/internal/device_enumerator.h > @@ -15,7 +15,7 @@ > > namespace libcamera { > > -class MediaDevice; > +class MediaDeviceBase; > > class DeviceMatch > { > @@ -24,7 +24,7 @@ public: > > void add(const std::string &entity); > > - bool match(const MediaDevice *device) const; > + bool match(const MediaDeviceBase *device) const; > > private: > std::string driver_; > @@ -41,17 +41,17 @@ public: > virtual int init() = 0; > virtual int enumerate() = 0; > > - std::shared_ptr<MediaDevice> search(const DeviceMatch &dm); > + std::shared_ptr<MediaDeviceBase> search(const DeviceMatch &dm); > > Signal<> devicesAdded; > > protected: > - std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode); > - void addDevice(std::unique_ptr<MediaDevice> media); > + std::unique_ptr<MediaDeviceBase> createDevice(const std::string &deviceNode); > + void addDevice(std::unique_ptr<MediaDeviceBase> media); > void removeDevice(const std::string &deviceNode); > > private: > - std::vector<std::shared_ptr<MediaDevice>> devices_; > + std::vector<std::shared_ptr<MediaDeviceBase>> devices_; > }; > > } /* namespace libcamera */ > diff --git a/include/libcamera/internal/device_enumerator_sysfs.h b/include/libcamera/internal/device_enumerator_sysfs.h > index 3e84b83f..920fd984 100644 > --- a/include/libcamera/internal/device_enumerator_sysfs.h > +++ b/include/libcamera/internal/device_enumerator_sysfs.h > @@ -12,7 +12,7 @@ > > #include "libcamera/internal/device_enumerator.h" > > -class MediaDevice; > +class MediaDeviceBase; > > namespace libcamera { > > @@ -23,7 +23,7 @@ public: > int enumerate(); > > private: > - int populateMediaDevice(MediaDevice *media); > + int populateMediaDevice(MediaDeviceBase *media); > std::string lookupDeviceNode(int major, int minor); > }; > > diff --git a/include/libcamera/internal/device_enumerator_udev.h b/include/libcamera/internal/device_enumerator_udev.h > index 1b3360df..a833f7a6 100644 > --- a/include/libcamera/internal/device_enumerator_udev.h > +++ b/include/libcamera/internal/device_enumerator_udev.h > @@ -23,7 +23,7 @@ struct udev_monitor; > namespace libcamera { > > class EventNotifier; > -class MediaDevice; > +class MediaDeviceBase; > class MediaEntity; > > class DeviceEnumeratorUdev final : public DeviceEnumerator > @@ -39,7 +39,7 @@ private: > using DependencyMap = std::map<dev_t, std::list<MediaEntity *>>; > > struct MediaDeviceDeps { > - MediaDeviceDeps(std::unique_ptr<MediaDevice> media, > + MediaDeviceDeps(std::unique_ptr<MediaDeviceBase> media, > DependencyMap deps) > : media_(std::move(media)), deps_(std::move(deps)) > { > @@ -50,12 +50,12 @@ private: > return media_ == other.media_; > } > > - std::unique_ptr<MediaDevice> media_; > + std::unique_ptr<MediaDeviceBase> media_; > DependencyMap deps_; > }; > > int addUdevDevice(struct udev_device *dev); > - int populateMediaDevice(MediaDevice *media, DependencyMap *deps); > + int populateMediaDevice(MediaDeviceBase *media, DependencyMap *deps); > std::string lookupDeviceNode(dev_t devnum); > > int addV4L2Device(dev_t devnum); > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > index ec4f662d..1223b1cb 100644 > --- a/include/libcamera/internal/pipeline_handler.h > +++ b/include/libcamera/internal/pipeline_handler.h > @@ -30,7 +30,7 @@ class CameraManager; > class DeviceEnumerator; > class DeviceMatch; > class FrameBuffer; > -class MediaDevice; > +class MediaDeviceBase; > class PipelineHandler; > class Request; > > @@ -42,8 +42,8 @@ public: > virtual ~PipelineHandler(); > > virtual bool match(DeviceEnumerator *enumerator) = 0; > - MediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator, > - const DeviceMatch &dm); > + MediaDeviceBase *acquireMediaDevice(DeviceEnumerator *enumerator, > + const DeviceMatch &dm); > > bool acquire(); > void release(Camera *camera); > @@ -69,7 +69,7 @@ public: > > protected: > void registerCamera(std::shared_ptr<Camera> camera); > - void hotplugMediaDevice(MediaDevice *media); > + void hotplugMediaDevice(MediaDeviceBase *media); > > virtual int queueRequestDevice(Camera *camera, Request *request) = 0; > virtual void stopDevice(Camera *camera) = 0; > @@ -81,13 +81,13 @@ protected: > private: > void unlockMediaDevices(); > > - void mediaDeviceDisconnected(MediaDevice *media); > + void mediaDeviceDisconnected(MediaDeviceBase *media); > virtual void disconnect(); > > void doQueueRequest(Request *request); > void doQueueRequests(); > > - std::vector<std::shared_ptr<MediaDevice>> mediaDevices_; > + std::vector<std::shared_ptr<MediaDeviceBase>> mediaDevices_; > std::vector<std::weak_ptr<Camera>> cameras_; > > std::queue<Request *> waitingRequests_; > diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp > index f2e055de..ed5d7af3 100644 > --- a/src/libcamera/device_enumerator.cpp > +++ b/src/libcamera/device_enumerator.cpp > @@ -13,7 +13,7 @@ > > #include "libcamera/internal/device_enumerator_sysfs.h" > #include "libcamera/internal/device_enumerator_udev.h" > -#include "libcamera/internal/media_device.h" > +#include "libcamera/internal/media_device_base.h" > > /** > * \file device_enumerator.h > @@ -25,12 +25,12 @@ > * At the core of the enumeration is the DeviceEnumerator class, responsible > * for enumerating all media devices in the system. It handles all interactions > * with the operating system in a platform-specific way. For each media device > - * found an instance of MediaDevice is created to store information about the > + * found an instance of MediaDeviceBase is created to store information about the > * device gathered from the kernel through the Media Controller API. > * > * The DeviceEnumerator can enumerate all or specific media devices in the > * system. When a new media device is added the enumerator creates a > - * corresponding MediaDevice instance. > + * corresponding MediaDeviceBase instance. > * > * The enumerator supports searching among enumerated devices based on criteria > * expressed in a DeviceMatch object. > @@ -91,7 +91,7 @@ void DeviceMatch::add(const std::string &entity) > * > * \return true if the media device matches the search pattern, false otherwise > */ > -bool DeviceMatch::match(const MediaDevice *device) const > +bool DeviceMatch::match(const MediaDeviceBase *device) const > { > if (driver_ != device->driver()) > return false; > @@ -120,7 +120,7 @@ bool DeviceMatch::match(const MediaDevice *device) const > * The DeviceEnumerator class is responsible for all interactions with the > * operating system related to media devices. It enumerates all media devices > * in the system, and for each device found creates an instance of the > - * MediaDevice class and stores it internally. The list of media devices can > + * MediaDeviceBase class and stores it internally. The list of media devices can > * then be searched using DeviceMatch search patterns. > * > * The enumerator also associates media device entities with device node paths. > @@ -161,7 +161,7 @@ std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create() > > DeviceEnumerator::~DeviceEnumerator() > { > - for (const std::shared_ptr<MediaDevice> &media : devices_) { > + for (const std::shared_ptr<MediaDeviceBase> &media : devices_) { > if (media->busy()) > LOG(DeviceEnumerator, Error) > << "Removing media device " << media->deviceNode() > @@ -209,9 +209,9 @@ DeviceEnumerator::~DeviceEnumerator() > * > * \return Created media device instance on success, or nullptr otherwise > */ > -std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode) > +std::unique_ptr<MediaDeviceBase> DeviceEnumerator::createDevice(const std::string &deviceNode) > { > - std::unique_ptr<MediaDevice> media = std::make_unique<MediaDevice>(deviceNode); > + std::unique_ptr<MediaDeviceBase> media = std::make_unique<MediaDeviceBase>(deviceNode); You create here a MediaDeviceBase instead of a MediaDevice (or like). Due to this the device population and enumeration doesn't work for children of MediaDeviceBase (e.g. MediaDevice). You should make `createDevice` function a template and allow the selection of the class to create by the users of `createDevice`. This has at least reliably worked for me. > > int ret = media->populate(); > if (ret < 0) { > @@ -247,7 +247,7 @@ std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d > * This function shall be called after all members of the entities of the > * media graph have been confirmed to be initialized. > */ > -void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> media) > +void DeviceEnumerator::addDevice(std::unique_ptr<MediaDeviceBase> media) > { > LOG(DeviceEnumerator, Debug) > << "Added device " << media->deviceNode() << ": " << media->driver(); > @@ -263,12 +263,12 @@ void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> media) > * \param[in] deviceNode Path to the media device to remove > * > * Remove the media device identified by \a deviceNode previously added to the > - * enumerator with addDevice(). The media device's MediaDevice::disconnected > + * enumerator with addDevice(). The media device's MediaDeviceBase::disconnected > * signal is emitted. > */ > void DeviceEnumerator::removeDevice(const std::string &deviceNode) > { > - std::shared_ptr<MediaDevice> media; > + std::shared_ptr<MediaDeviceBase> media; > > for (auto iter = devices_.begin(); iter != devices_.end(); ++iter) { > if ((*iter)->deviceNode() == deviceNode) { > @@ -297,14 +297,14 @@ void DeviceEnumerator::removeDevice(const std::string &deviceNode) > * > * Search in the enumerated media devices that are not already in use for a > * match described in \a dm. If a match is found and the caller intends to use > - * it the caller is responsible for acquiring the MediaDevice object and > + * it the caller is responsible for acquiring the MediaDeviceBase object and > * releasing it when done with it. > * > - * \return pointer to the matching MediaDevice, or nullptr if no match is found > + * \return pointer to the matching MediaDeviceBase, or nullptr if no match is found > */ > -std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm) > +std::shared_ptr<MediaDeviceBase> DeviceEnumerator::search(const DeviceMatch &dm) > { > - for (std::shared_ptr<MediaDevice> &media : devices_) { > + for (std::shared_ptr<MediaDeviceBase> &media : devices_) { > if (media->busy()) > continue; > > diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp > index 686bb809..22bbe44f 100644 > --- a/src/libcamera/device_enumerator_sysfs.cpp > +++ b/src/libcamera/device_enumerator_sysfs.cpp > @@ -73,7 +73,7 @@ int DeviceEnumeratorSysfs::enumerate() > continue; > } > > - std::unique_ptr<MediaDevice> media = createDevice(devnode); > + std::unique_ptr<MediaDeviceBase> media = createDevice(devnode); i.e. std::unique_ptr<MediaDeviceBase> media = createDevice<MediaDevice>(devnode); > if (!media) > continue; > > @@ -93,7 +93,7 @@ int DeviceEnumeratorSysfs::enumerate() > return 0; > } > > -int DeviceEnumeratorSysfs::populateMediaDevice(MediaDevice *media) > +int DeviceEnumeratorSysfs::populateMediaDevice(MediaDeviceBase *media) > { > /* Associate entities to device node paths. */ > for (MediaEntity *entity : media->entities()) { > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp > index 5317afbd..8be8a8d2 100644 > --- a/src/libcamera/device_enumerator_udev.cpp > +++ b/src/libcamera/device_enumerator_udev.cpp > @@ -76,7 +76,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev) > return -ENODEV; > > if (!strcmp(subsystem, "media")) { > - std::unique_ptr<MediaDevice> media = > + std::unique_ptr<MediaDeviceBase> media = > createDevice(udev_device_get_devnode(dev)); > if (!media) > return -ENODEV; > @@ -193,7 +193,7 @@ done: > return 0; > } > > -int DeviceEnumeratorUdev::populateMediaDevice(MediaDevice *media, DependencyMap *deps) > +int DeviceEnumeratorUdev::populateMediaDevice(MediaDeviceBase *media, DependencyMap *deps) > { > std::set<dev_t> children; > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index 0c67e35d..f41e10c3 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -865,7 +865,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) > dm.add("mxc_isi.0"); > dm.add("mxc_isi.0.capture"); > > - isiDev_ = acquireMediaDevice(enumerator, dm); > + isiDev_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); > if (!isiDev_) > return false; > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index e4d79ea4..c3f3c815 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -923,11 +923,11 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) > imgu_dm.add("ipu3-imgu 1 viewfinder"); > imgu_dm.add("ipu3-imgu 1 3a stat"); > > - cio2MediaDev_ = acquireMediaDevice(enumerator, cio2_dm); > + cio2MediaDev_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, cio2_dm)); > if (!cio2MediaDev_) > return false; > > - imguMediaDev_ = acquireMediaDevice(enumerator, imgu_dm); > + imguMediaDev_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, imgu_dm)); > if (!imguMediaDev_) > return false; > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 8569df17..355cd556 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1160,7 +1160,7 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request) > bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) > { > DeviceMatch unicam("unicam"); > - MediaDevice *unicamDevice = acquireMediaDevice(enumerator, unicam); > + MediaDevice *unicamDevice = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, unicam)); > > if (!unicamDevice) { > LOG(RPI, Debug) << "Unable to acquire a Unicam instance"; > @@ -1168,7 +1168,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) > } > > DeviceMatch isp("bcm2835-isp"); > - MediaDevice *ispDevice = acquireMediaDevice(enumerator, isp); > + MediaDevice *ispDevice = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, isp)); > > if (!ispDevice) { > LOG(RPI, Debug) << "Unable to acquire ISP instance"; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index eb9ad65c..c14711ef 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1146,7 +1146,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) > dm.add("rkisp1_stats"); > dm.add("rkisp1_params"); > > - media_ = acquireMediaDevice(enumerator, dm); > + media_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); > if (!media_) > return false; > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index a32de7f3..e6f4fad3 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1391,7 +1391,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) > > for (const SimplePipelineInfo &inf : supportedDevices) { > DeviceMatch dm(inf.driver); > - media_ = acquireMediaDevice(enumerator, dm); > + media_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); > if (media_) { > info = &inf; > break; > @@ -1403,7 +1403,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) > > for (const auto &[name, streams] : info->converters) { > DeviceMatch converterMatch(name); > - converter_ = acquireMediaDevice(enumerator, converterMatch); > + converter_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, converterMatch)); > if (converter_) { > numStreams = streams; > break; > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 277465b7..b97ec95e 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -389,7 +389,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) > MediaDevice *media; > DeviceMatch dm("uvcvideo"); > > - media = acquireMediaDevice(enumerator, dm); > + media = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); > if (!media) > return false; > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index 204f5ad7..0aaaa628 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -454,7 +454,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) > dm.add("RGB/YUV Input"); > dm.add("Scaler"); > > - MediaDevice *media = acquireMediaDevice(enumerator, dm); > + MediaDevice *media = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); > if (!media) > return false; > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index cfade490..613698a1 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -73,7 +73,7 @@ PipelineHandler::PipelineHandler(CameraManager *manager) > > PipelineHandler::~PipelineHandler() > { > - for (std::shared_ptr<MediaDevice> media : mediaDevices_) > + for (std::shared_ptr<MediaDeviceBase> media : mediaDevices_) > media->release(); > } > > @@ -126,10 +126,10 @@ PipelineHandler::~PipelineHandler() > * > * \return A pointer to the matching MediaDevice, or nullptr if no match is found > */ > -MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, > - const DeviceMatch &dm) > +MediaDeviceBase *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, > + const DeviceMatch &dm) > { > - std::shared_ptr<MediaDevice> media = enumerator->search(dm); > + std::shared_ptr<MediaDeviceBase> media = enumerator->search(dm); > if (!media) > return nullptr; > > @@ -170,7 +170,7 @@ bool PipelineHandler::acquire() > return true; > } > > - for (std::shared_ptr<MediaDevice> &media : mediaDevices_) { > + for (std::shared_ptr<MediaDeviceBase> &media : mediaDevices_) { > if (!media->lock()) { > unlockMediaDevices(); > return false; > @@ -223,7 +223,7 @@ void PipelineHandler::releaseDevice([[maybe_unused]] Camera *camera) > > void PipelineHandler::unlockMediaDevices() > { > - for (std::shared_ptr<MediaDevice> &media : mediaDevices_) > + for (std::shared_ptr<MediaDeviceBase> &media : mediaDevices_) > media->unlock(); > } > > @@ -556,7 +556,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > * to the camera. > */ > std::vector<dev_t> devnums; > - for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) { > + for (const std::shared_ptr<MediaDeviceBase> &media : mediaDevices_) { > for (const MediaEntity *entity : media->entities()) { > if (entity->pads().size() == 1 && > (entity->pads()[0]->flags() & MEDIA_PAD_FL_SINK) && > @@ -582,7 +582,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > * handler gets notified and automatically disconnects all the cameras it has > * registered without requiring any manual intervention. > */ > -void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > +void PipelineHandler::hotplugMediaDevice(MediaDeviceBase *media) > { > media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); > } > @@ -590,7 +590,7 @@ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > /** > * \brief Slot for the MediaDevice disconnected signal > */ > -void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) > +void PipelineHandler::mediaDeviceDisconnected(MediaDeviceBase *media) > { > media->disconnected.disconnect(this); > Best regards Sophie
diff --git a/include/libcamera/internal/device_enumerator.h b/include/libcamera/internal/device_enumerator.h index 72ec9a60..87a2b5ce 100644 --- a/include/libcamera/internal/device_enumerator.h +++ b/include/libcamera/internal/device_enumerator.h @@ -15,7 +15,7 @@ namespace libcamera { -class MediaDevice; +class MediaDeviceBase; class DeviceMatch { @@ -24,7 +24,7 @@ public: void add(const std::string &entity); - bool match(const MediaDevice *device) const; + bool match(const MediaDeviceBase *device) const; private: std::string driver_; @@ -41,17 +41,17 @@ public: virtual int init() = 0; virtual int enumerate() = 0; - std::shared_ptr<MediaDevice> search(const DeviceMatch &dm); + std::shared_ptr<MediaDeviceBase> search(const DeviceMatch &dm); Signal<> devicesAdded; protected: - std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode); - void addDevice(std::unique_ptr<MediaDevice> media); + std::unique_ptr<MediaDeviceBase> createDevice(const std::string &deviceNode); + void addDevice(std::unique_ptr<MediaDeviceBase> media); void removeDevice(const std::string &deviceNode); private: - std::vector<std::shared_ptr<MediaDevice>> devices_; + std::vector<std::shared_ptr<MediaDeviceBase>> devices_; }; } /* namespace libcamera */ diff --git a/include/libcamera/internal/device_enumerator_sysfs.h b/include/libcamera/internal/device_enumerator_sysfs.h index 3e84b83f..920fd984 100644 --- a/include/libcamera/internal/device_enumerator_sysfs.h +++ b/include/libcamera/internal/device_enumerator_sysfs.h @@ -12,7 +12,7 @@ #include "libcamera/internal/device_enumerator.h" -class MediaDevice; +class MediaDeviceBase; namespace libcamera { @@ -23,7 +23,7 @@ public: int enumerate(); private: - int populateMediaDevice(MediaDevice *media); + int populateMediaDevice(MediaDeviceBase *media); std::string lookupDeviceNode(int major, int minor); }; diff --git a/include/libcamera/internal/device_enumerator_udev.h b/include/libcamera/internal/device_enumerator_udev.h index 1b3360df..a833f7a6 100644 --- a/include/libcamera/internal/device_enumerator_udev.h +++ b/include/libcamera/internal/device_enumerator_udev.h @@ -23,7 +23,7 @@ struct udev_monitor; namespace libcamera { class EventNotifier; -class MediaDevice; +class MediaDeviceBase; class MediaEntity; class DeviceEnumeratorUdev final : public DeviceEnumerator @@ -39,7 +39,7 @@ private: using DependencyMap = std::map<dev_t, std::list<MediaEntity *>>; struct MediaDeviceDeps { - MediaDeviceDeps(std::unique_ptr<MediaDevice> media, + MediaDeviceDeps(std::unique_ptr<MediaDeviceBase> media, DependencyMap deps) : media_(std::move(media)), deps_(std::move(deps)) { @@ -50,12 +50,12 @@ private: return media_ == other.media_; } - std::unique_ptr<MediaDevice> media_; + std::unique_ptr<MediaDeviceBase> media_; DependencyMap deps_; }; int addUdevDevice(struct udev_device *dev); - int populateMediaDevice(MediaDevice *media, DependencyMap *deps); + int populateMediaDevice(MediaDeviceBase *media, DependencyMap *deps); std::string lookupDeviceNode(dev_t devnum); int addV4L2Device(dev_t devnum); diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index ec4f662d..1223b1cb 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -30,7 +30,7 @@ class CameraManager; class DeviceEnumerator; class DeviceMatch; class FrameBuffer; -class MediaDevice; +class MediaDeviceBase; class PipelineHandler; class Request; @@ -42,8 +42,8 @@ public: virtual ~PipelineHandler(); virtual bool match(DeviceEnumerator *enumerator) = 0; - MediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator, - const DeviceMatch &dm); + MediaDeviceBase *acquireMediaDevice(DeviceEnumerator *enumerator, + const DeviceMatch &dm); bool acquire(); void release(Camera *camera); @@ -69,7 +69,7 @@ public: protected: void registerCamera(std::shared_ptr<Camera> camera); - void hotplugMediaDevice(MediaDevice *media); + void hotplugMediaDevice(MediaDeviceBase *media); virtual int queueRequestDevice(Camera *camera, Request *request) = 0; virtual void stopDevice(Camera *camera) = 0; @@ -81,13 +81,13 @@ protected: private: void unlockMediaDevices(); - void mediaDeviceDisconnected(MediaDevice *media); + void mediaDeviceDisconnected(MediaDeviceBase *media); virtual void disconnect(); void doQueueRequest(Request *request); void doQueueRequests(); - std::vector<std::shared_ptr<MediaDevice>> mediaDevices_; + std::vector<std::shared_ptr<MediaDeviceBase>> mediaDevices_; std::vector<std::weak_ptr<Camera>> cameras_; std::queue<Request *> waitingRequests_; diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index f2e055de..ed5d7af3 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -13,7 +13,7 @@ #include "libcamera/internal/device_enumerator_sysfs.h" #include "libcamera/internal/device_enumerator_udev.h" -#include "libcamera/internal/media_device.h" +#include "libcamera/internal/media_device_base.h" /** * \file device_enumerator.h @@ -25,12 +25,12 @@ * At the core of the enumeration is the DeviceEnumerator class, responsible * for enumerating all media devices in the system. It handles all interactions * with the operating system in a platform-specific way. For each media device - * found an instance of MediaDevice is created to store information about the + * found an instance of MediaDeviceBase is created to store information about the * device gathered from the kernel through the Media Controller API. * * The DeviceEnumerator can enumerate all or specific media devices in the * system. When a new media device is added the enumerator creates a - * corresponding MediaDevice instance. + * corresponding MediaDeviceBase instance. * * The enumerator supports searching among enumerated devices based on criteria * expressed in a DeviceMatch object. @@ -91,7 +91,7 @@ void DeviceMatch::add(const std::string &entity) * * \return true if the media device matches the search pattern, false otherwise */ -bool DeviceMatch::match(const MediaDevice *device) const +bool DeviceMatch::match(const MediaDeviceBase *device) const { if (driver_ != device->driver()) return false; @@ -120,7 +120,7 @@ bool DeviceMatch::match(const MediaDevice *device) const * The DeviceEnumerator class is responsible for all interactions with the * operating system related to media devices. It enumerates all media devices * in the system, and for each device found creates an instance of the - * MediaDevice class and stores it internally. The list of media devices can + * MediaDeviceBase class and stores it internally. The list of media devices can * then be searched using DeviceMatch search patterns. * * The enumerator also associates media device entities with device node paths. @@ -161,7 +161,7 @@ std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create() DeviceEnumerator::~DeviceEnumerator() { - for (const std::shared_ptr<MediaDevice> &media : devices_) { + for (const std::shared_ptr<MediaDeviceBase> &media : devices_) { if (media->busy()) LOG(DeviceEnumerator, Error) << "Removing media device " << media->deviceNode() @@ -209,9 +209,9 @@ DeviceEnumerator::~DeviceEnumerator() * * \return Created media device instance on success, or nullptr otherwise */ -std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode) +std::unique_ptr<MediaDeviceBase> DeviceEnumerator::createDevice(const std::string &deviceNode) { - std::unique_ptr<MediaDevice> media = std::make_unique<MediaDevice>(deviceNode); + std::unique_ptr<MediaDeviceBase> media = std::make_unique<MediaDeviceBase>(deviceNode); int ret = media->populate(); if (ret < 0) { @@ -247,7 +247,7 @@ std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d * This function shall be called after all members of the entities of the * media graph have been confirmed to be initialized. */ -void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> media) +void DeviceEnumerator::addDevice(std::unique_ptr<MediaDeviceBase> media) { LOG(DeviceEnumerator, Debug) << "Added device " << media->deviceNode() << ": " << media->driver(); @@ -263,12 +263,12 @@ void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> media) * \param[in] deviceNode Path to the media device to remove * * Remove the media device identified by \a deviceNode previously added to the - * enumerator with addDevice(). The media device's MediaDevice::disconnected + * enumerator with addDevice(). The media device's MediaDeviceBase::disconnected * signal is emitted. */ void DeviceEnumerator::removeDevice(const std::string &deviceNode) { - std::shared_ptr<MediaDevice> media; + std::shared_ptr<MediaDeviceBase> media; for (auto iter = devices_.begin(); iter != devices_.end(); ++iter) { if ((*iter)->deviceNode() == deviceNode) { @@ -297,14 +297,14 @@ void DeviceEnumerator::removeDevice(const std::string &deviceNode) * * Search in the enumerated media devices that are not already in use for a * match described in \a dm. If a match is found and the caller intends to use - * it the caller is responsible for acquiring the MediaDevice object and + * it the caller is responsible for acquiring the MediaDeviceBase object and * releasing it when done with it. * - * \return pointer to the matching MediaDevice, or nullptr if no match is found + * \return pointer to the matching MediaDeviceBase, or nullptr if no match is found */ -std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm) +std::shared_ptr<MediaDeviceBase> DeviceEnumerator::search(const DeviceMatch &dm) { - for (std::shared_ptr<MediaDevice> &media : devices_) { + for (std::shared_ptr<MediaDeviceBase> &media : devices_) { if (media->busy()) continue; diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp index 686bb809..22bbe44f 100644 --- a/src/libcamera/device_enumerator_sysfs.cpp +++ b/src/libcamera/device_enumerator_sysfs.cpp @@ -73,7 +73,7 @@ int DeviceEnumeratorSysfs::enumerate() continue; } - std::unique_ptr<MediaDevice> media = createDevice(devnode); + std::unique_ptr<MediaDeviceBase> media = createDevice(devnode); if (!media) continue; @@ -93,7 +93,7 @@ int DeviceEnumeratorSysfs::enumerate() return 0; } -int DeviceEnumeratorSysfs::populateMediaDevice(MediaDevice *media) +int DeviceEnumeratorSysfs::populateMediaDevice(MediaDeviceBase *media) { /* Associate entities to device node paths. */ for (MediaEntity *entity : media->entities()) { diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index 5317afbd..8be8a8d2 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -76,7 +76,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev) return -ENODEV; if (!strcmp(subsystem, "media")) { - std::unique_ptr<MediaDevice> media = + std::unique_ptr<MediaDeviceBase> media = createDevice(udev_device_get_devnode(dev)); if (!media) return -ENODEV; @@ -193,7 +193,7 @@ done: return 0; } -int DeviceEnumeratorUdev::populateMediaDevice(MediaDevice *media, DependencyMap *deps) +int DeviceEnumeratorUdev::populateMediaDevice(MediaDeviceBase *media, DependencyMap *deps) { std::set<dev_t> children; diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 0c67e35d..f41e10c3 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -865,7 +865,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator) dm.add("mxc_isi.0"); dm.add("mxc_isi.0.capture"); - isiDev_ = acquireMediaDevice(enumerator, dm); + isiDev_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); if (!isiDev_) return false; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index e4d79ea4..c3f3c815 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -923,11 +923,11 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) imgu_dm.add("ipu3-imgu 1 viewfinder"); imgu_dm.add("ipu3-imgu 1 3a stat"); - cio2MediaDev_ = acquireMediaDevice(enumerator, cio2_dm); + cio2MediaDev_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, cio2_dm)); if (!cio2MediaDev_) return false; - imguMediaDev_ = acquireMediaDevice(enumerator, imgu_dm); + imguMediaDev_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, imgu_dm)); if (!imguMediaDev_) return false; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 8569df17..355cd556 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1160,7 +1160,7 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request) bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) { DeviceMatch unicam("unicam"); - MediaDevice *unicamDevice = acquireMediaDevice(enumerator, unicam); + MediaDevice *unicamDevice = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, unicam)); if (!unicamDevice) { LOG(RPI, Debug) << "Unable to acquire a Unicam instance"; @@ -1168,7 +1168,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) } DeviceMatch isp("bcm2835-isp"); - MediaDevice *ispDevice = acquireMediaDevice(enumerator, isp); + MediaDevice *ispDevice = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, isp)); if (!ispDevice) { LOG(RPI, Debug) << "Unable to acquire ISP instance"; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index eb9ad65c..c14711ef 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1146,7 +1146,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) dm.add("rkisp1_stats"); dm.add("rkisp1_params"); - media_ = acquireMediaDevice(enumerator, dm); + media_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); if (!media_) return false; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index a32de7f3..e6f4fad3 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1391,7 +1391,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) for (const SimplePipelineInfo &inf : supportedDevices) { DeviceMatch dm(inf.driver); - media_ = acquireMediaDevice(enumerator, dm); + media_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); if (media_) { info = &inf; break; @@ -1403,7 +1403,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) for (const auto &[name, streams] : info->converters) { DeviceMatch converterMatch(name); - converter_ = acquireMediaDevice(enumerator, converterMatch); + converter_ = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, converterMatch)); if (converter_) { numStreams = streams; break; diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 277465b7..b97ec95e 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -389,7 +389,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) MediaDevice *media; DeviceMatch dm("uvcvideo"); - media = acquireMediaDevice(enumerator, dm); + media = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); if (!media) return false; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 204f5ad7..0aaaa628 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -454,7 +454,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) dm.add("RGB/YUV Input"); dm.add("Scaler"); - MediaDevice *media = acquireMediaDevice(enumerator, dm); + MediaDevice *media = dynamic_cast<MediaDevice *>(acquireMediaDevice(enumerator, dm)); if (!media) return false; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index cfade490..613698a1 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -73,7 +73,7 @@ PipelineHandler::PipelineHandler(CameraManager *manager) PipelineHandler::~PipelineHandler() { - for (std::shared_ptr<MediaDevice> media : mediaDevices_) + for (std::shared_ptr<MediaDeviceBase> media : mediaDevices_) media->release(); } @@ -126,10 +126,10 @@ PipelineHandler::~PipelineHandler() * * \return A pointer to the matching MediaDevice, or nullptr if no match is found */ -MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, - const DeviceMatch &dm) +MediaDeviceBase *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, + const DeviceMatch &dm) { - std::shared_ptr<MediaDevice> media = enumerator->search(dm); + std::shared_ptr<MediaDeviceBase> media = enumerator->search(dm); if (!media) return nullptr; @@ -170,7 +170,7 @@ bool PipelineHandler::acquire() return true; } - for (std::shared_ptr<MediaDevice> &media : mediaDevices_) { + for (std::shared_ptr<MediaDeviceBase> &media : mediaDevices_) { if (!media->lock()) { unlockMediaDevices(); return false; @@ -223,7 +223,7 @@ void PipelineHandler::releaseDevice([[maybe_unused]] Camera *camera) void PipelineHandler::unlockMediaDevices() { - for (std::shared_ptr<MediaDevice> &media : mediaDevices_) + for (std::shared_ptr<MediaDeviceBase> &media : mediaDevices_) media->unlock(); } @@ -556,7 +556,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) * to the camera. */ std::vector<dev_t> devnums; - for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) { + for (const std::shared_ptr<MediaDeviceBase> &media : mediaDevices_) { for (const MediaEntity *entity : media->entities()) { if (entity->pads().size() == 1 && (entity->pads()[0]->flags() & MEDIA_PAD_FL_SINK) && @@ -582,7 +582,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) * handler gets notified and automatically disconnects all the cameras it has * registered without requiring any manual intervention. */ -void PipelineHandler::hotplugMediaDevice(MediaDevice *media) +void PipelineHandler::hotplugMediaDevice(MediaDeviceBase *media) { media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); } @@ -590,7 +590,7 @@ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) /** * \brief Slot for the MediaDevice disconnected signal */ -void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) +void PipelineHandler::mediaDeviceDisconnected(MediaDeviceBase *media) { media->disconnected.disconnect(this);
Use MediaDeviceBase instead of MediaDevice in other base classes like PipelineHandler and DeviceEnumerator. Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> --- .../libcamera/internal/device_enumerator.h | 12 ++++---- .../internal/device_enumerator_sysfs.h | 4 +-- .../internal/device_enumerator_udev.h | 8 ++--- include/libcamera/internal/pipeline_handler.h | 12 ++++---- src/libcamera/device_enumerator.cpp | 30 +++++++++---------- src/libcamera/device_enumerator_sysfs.cpp | 4 +-- src/libcamera/device_enumerator_udev.cpp | 4 +-- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 2 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-- .../pipeline/raspberrypi/raspberrypi.cpp | 4 +-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/simple/simple.cpp | 4 +-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc/vimc.cpp | 2 +- src/libcamera/pipeline_handler.cpp | 18 +++++------ 15 files changed, 56 insertions(+), 56 deletions(-)