[libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide the default entity
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Feb 8 16:44:17 CET 2019
Hello,
On Thu, Feb 07, 2019 at 11:06:20PM +0100, Jacopo Mondi wrote:
> Hi Kieran,
> ignorant question, is FL_DEFAULT an UVC specific thing?
It isn't, but it isn't widely used either. It would thus be fine using
the default entity in UVC-specific tests or in the UVC pipeline handler,
but not in the rest of the code, at least not without a fallback.
> A quick grep on kernel sources returns:
>
> $ git grep FL_DEFAULT drivers/media/
> drivers/media/platform/omap3isp/ispresizer.c: res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;
> drivers/media/usb/uvc/uvc_entity.c: entity->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;
>
> So I guess not, as omap3 seems also omap3 uses that as well.
>
> If that's not a UVC specificity, I welcome this change.
>
> On Thu, Feb 07, 2019 at 09:21:15PM +0000, Kieran Bingham wrote:
> > Add a helper to identify the default entity from a media device.
> >
> > Utilise it in the two places which iterate the entities manually, to allocated
> > a V4L2Device from the default entity.
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> > src/libcamera/include/media_device.h | 1 +
> > src/libcamera/media_device.cpp | 15 +++++++++++++++
> > src/libcamera/pipeline/uvcvideo.cpp | 10 ++++------
> > test/v4l2_device/v4l2_device_test.cpp | 12 +++++-------
> > 4 files changed, 25 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
> > index 9f038093b2b2..361e8f4a4b86 100644
> > --- a/src/libcamera/include/media_device.h
> > +++ b/src/libcamera/include/media_device.h
> > @@ -42,6 +42,7 @@ public:
> >
> > const std::vector<MediaEntity *> &entities() const { return entities_; }
> > MediaEntity *getEntityByName(const std::string &name) const;
> > + MediaEntity *defaultEntity() const;
This should take an entity type as a parameter as the default flag is
defined as "Default entity for its type". It could also be used, for
instance, to discover the default camera sensor.
> >
> > MediaLink *link(const std::string &sourceName, unsigned int sourceIdx,
> > const std::string &sinkName, unsigned int sinkIdx);
> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> > index 800ed330fe6d..4af90e1590a1 100644
> > --- a/src/libcamera/media_device.cpp
> > +++ b/src/libcamera/media_device.cpp
> > @@ -319,6 +319,21 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const
> > return nullptr;
> > }
> >
> > +/**
> > + * \brief Return the default MediaEntity within a MediaDevice
> > + * \return The default entity if specified, or a nullptr otherwise
> > + */
> > +MediaEntity *MediaDevice::defaultEntity() const
> > +{
> > + for (MediaEntity *entity : entities_) {
> > + if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> > + return entity;
> > + }
>
> You could drop the inner braces { } here
> > + }
> > +
> > + return nullptr;
> > +}
> > +
> > /**
> > * \brief Retrieve the MediaLink connecting two pads, identified by entity
> > * names and pad indexes
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index fc31c52c0ecd..ed8228bb2fc6 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -146,13 +146,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> >
> > media_->acquire();
> >
> > - for (MediaEntity *entity : media_->entities()) {
> > - if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> > - video_ = new V4L2Device(entity);
> > - break;
> > - }
> > - }
> > + MediaEntity *entity = media_->defaultEntity();
> > + if (!entity)
>
> Looking at existing code if you don't find any DEFAULT entity, you
> return false as you do here, but also release the media device.
>
> Should the same be done here?
I think it should, and a message should likely be logged too.
> > + return false;
> >
> > + video_ = new V4L2Device(entity);
> > if (!video_ || video_->open()) {
> > if (!video_)
> > LOG(UVC, Error) << "Could not find a default video device";
> > diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp
> > index 18d014caf4c8..dd28cccada6b 100644
> > --- a/test/v4l2_device/v4l2_device_test.cpp
> > +++ b/test/v4l2_device/v4l2_device_test.cpp
> > @@ -46,15 +46,13 @@ int V4L2DeviceTest::init()
> >
> > media_->acquire();
> >
> > - for (MediaEntity *entity : media_->entities()) {
> > - if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> > - dev_ = new V4L2Device(entity);
> > - break;
> > - }
> > - }
> > + MediaEntity *entity = media_->defaultEntity();
> > + if (!entity)
> > + return TestFail;
Note that we're planning to use vivid instead of uvcvideo for V4L2Device
tests, so this may not be a good idea.
> > + dev_ = new V4L2Device(entity);
> > if (!dev_)
> > - return TestSkip;
> > + return TestFail;
> >
> > return dev_->open();
> > }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list