[libcamera-devel] [PATCH v2 5/5] tests: Introduce hotplug hot-unplug unit test
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jun 7 19:21:23 CEST 2020
Hi Umang,
On Tue, May 19, 2020 at 02:15:26PM +0000, Umang Jain wrote:
> On Mon, 2020-05-18 at 16:57 +0100, Kieran Bingham wrote:
> > Hi Umang,
> >
> > I'm happy to see a hotplug test here, and I know this one is quite a
> > pain to write ;-)
> >
> > On 13/05/2020 18:30, Umang Jain wrote:
> >
> > A bit of blurb about what happens in the test is always useful in the
> > commit message here.
> >
> > It's also particularly worth mentioning that the test will skip if not
> > run as root due to the requirement to modify the bind state of the
> > device, and of course that it requires a UVC device to be connected for
> > test.
> >
> > > Signed-off-by: Umang Jain <email at uajain.com>
> > > ---
> > > test/hotplug-cameras.cpp | 152 +++++++++++++++++++++++++++++++++++++++
> > > test/meson.build | 1 +
> > > 2 files changed, 153 insertions(+)
> > > create mode 100644 test/hotplug-cameras.cpp
> > >
> > > diff --git a/test/hotplug-cameras.cpp b/test/hotplug-cameras.cpp
> > > new file mode 100644
> > > index 0000000..cbf68af
> > > --- /dev/null
> > > +++ b/test/hotplug-cameras.cpp
> > > @@ -0,0 +1,152 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/*
> > > + * Copyright (C) 2020, Umang Jain <email at uajain.com>
> > > + *
> > > + * hotplug-cameras.cpp - Emulate cameraAdded/cameraRemoved signals
> > > in CameraManager
> > > + */
> > > +
> > > +#include <dirent.h>
> > > +#include <fstream>
> > > +#include <iostream>
> > > +#include <string.h>
> > > +#include <unistd.h>
> > > +
> > > +#include <fcntl.h>
> > > +#include <sys/stat.h>
> > > +#include <sys/types.h>
> > > +
> > > +#include <libcamera/camera.h>
> > > +#include <libcamera/camera_manager.h>
> > > +#include <libcamera/event_dispatcher.h>
> > > +#include <libcamera/timer.h>
> > > +
> > > +#include "file.h"
> > > +#include "test.h"
> > > +#include "thread.h"
> > > +
> > > +using namespace std;
> > > +using namespace libcamera;
> > > +
> > > +class HotplugTest : public Test
> > > +{
> > > +protected:
> > > + void cameraAddedHandler(std::shared_ptr<Camera> cam)
> > > + {
> > > + cameraAddedPass_ = true;
> > > + }
> > > +
> > > + void cameraRemovedHandler(std::shared_ptr<Camera> cam)
> > > + {
> > > + cameraRemovedPass_ = true;
> > > + }
> > > +
> > > + int init()
> > > + {
> > > + if (!File::exists("/sys/module/uvcvideo")) {
> > > + std::cout << "uvcvideo driver is not loaded,
> > > skipping" << std::endl;
> > > + return TestSkip;
> > > + }
> > > +
> > > + if (geteuid() != 0) {
> > > + std::cout << "This test requires root permissions, skipping" << std::endl;
> > > + return TestSkip;
> > > + }
> > > +
> > > + cm_ = new CameraManager();
> > > + if (cm_->start()) {
> > > + std::cout << "Failed to start camera manager" << std::endl;
> > > + return TestFail;
> > > + }
> > > +
> > > + cameraAddedPass_ = false;
> > > + cameraRemovedPass_ = false;
> > > +
> > > + cm_->cameraAdded.connect(this, &HotplugTest::cameraAddedHandler);
> > > + cm_->cameraRemoved.connect(this, &HotplugTest::cameraRemovedHandler);
> > > +
> > > + uvc_toplevel_ = "/sys/module/uvcvideo/drivers/usb:uvcvideo/";
> > > +
> > > + return 0;
> > > + }
> > > +
> > > + int run()
> > > + {
> > > + DIR *dir;
> > > + struct dirent *dirent, *dirent2;
> > > + std::string uvc_driver_dir;
> > > + bool uvc_driver_found = false;
> > > +
> > > + dir = opendir(uvc_toplevel_.c_str());
> > > + /* Find a UVC device driver symlink, which we can bind/unbind */
> > > + while ((dirent = readdir(dir)) != nullptr) {
> > > + if (dirent->d_type != DT_LNK)
> > > + continue;
> > > +
> > > + std::string child_dir = uvc_toplevel_ + dirent- >d_name;
> > > + DIR *device_driver = opendir(child_dir.c_str());
> > > + while ((dirent2 = readdir(device_driver)) != nullptr) {
> > > + if (strncmp(dirent2->d_name, "video4linux", 11) == 0) {
> > > + uvc_driver_dir = dirent- >d_name;
> > > + uvc_driver_found = true;
> > > + break;
> > > + }
> > > + }
> > > + closedir(device_driver);
> > > +
> > > + if (uvc_driver_found)
> > > + break;
> > > + }
> > > + closedir(dir);
> > > +
> > > + /* If no UVC driver found, skip */
> > > + if (!uvc_driver_found)
> > > + return TestSkip;
> > > +
> > > + /* Unbind a camera, process events */
> > > + int fd1 = open("/sys/module/uvcvideo/drivers/usb:uvcvideo/unbind", O_WRONLY);
> > > + write(fd1, uvc_driver_dir.c_str(), uvc_driver_dir.size());
> > > + close(fd1);
> > > + Timer timer;
> > > + timer.start(1000);
> > > + while (timer.isRunning())
> > > + Thread::current()->eventDispatcher()- >processEvents();
> > > +
> > > + /* \todo: Fix this workaround of stopping and starting the camera-manager.
> > > + * We need to do this, so that cm_ release all references to the uvc media symlinks.
> >
> > Eep, I can't get my head around this just yet. You mean having the
> > camera manager running, it has open handles to the media symlinks in
> > sysfs perhaps?
> >
> > I guess it's implying that something in the cm_ is preventing the clean
> > unbind of the camera - but I don't understand what that is yet to know
> > if we must fix that, or if it's something unavoidable?
>
> Yes, let me explain with an observation from the 'master' branch of
> libcamera(i.e. without these patches).
>
> I have a UVC camera and is denoted by "1-1:1.0" and "1-1:1.1" symlinks
> in '/sys/module/uvcvideo/drivers/usb:uvcvideo'. Now, with qcam
> streaming from this device, try to unbind and re-bind the "1-1:1.0"
> symlink (which is the primary video device node). Obviously, qcam
> won't pick up the re-binding again but notice what happens when you
> exit the qcam and try to re-run qcam again; qcam won't notice the
> device when it's restarted.
>
> Yet another observation while following the above mentioned process,
> when I have qcam running and I unbind "1-1:1.0", the symlink "1-1:1.1"
> doesn't get dis-appeared(apparently it should). It only gets disappered
> (i.e. device has been cleanly unbind) only when qcam is closed at this
> point; which explains CameraManager is holding some kind of ref on this
> symlink.
>
> > But we're not going to expect applications to stop and restart the
> > camera manager, so I certainly hope this is just because of the fact
> > we're using the unbind/bind mechanism?
>
> Yes, that's my observation as well. This problem(and hence the
> workaround in the test) is specific to bind/unbind mechanism. I tested
> with a handy USB webcam around, and it doesn't leave any dangling
> symlinks.
>
> > If not - then we really need to fix it ;-)
>
> I am still looking into it, it might be a silly leak somewhere but
> getting hard to find :)
Thanks for the analysis. Have you managed to figure out why this
happens, or is it still an open question ?
> > > + */
> > > + cm_->stop();
> > > + if (cm_->start()) {
> > > + std::cout << "Failed to restart camera-manager" << std::endl;
> > > + return TestFail;
> > > + }
> > > +
> > > + /* Bind the camera again, process events */
> > > + int fd2 = open("/sys/module/uvcvideo/drivers/usb:uvcvideo/bind", O_WRONLY);
> > > + write(fd2, uvc_driver_dir.c_str(), uvc_driver_dir.size());
> > > + close(fd2);
> > > +
> > > + timer.start(1000);
> > > + while (timer.isRunning())
> > > + Thread::current()->eventDispatcher()- >processEvents();
> > > +
> > > + if (cameraAddedPass_ && cameraRemovedPass_)
> > > + return TestPass;
> > > + else
> > > + return TestFail;
> > > + }
> > > +
> > > + void cleanup()
> > > + {
> > > + cm_->stop();
> > > + delete cm_;
> > > + }
> > > +
> > > +private:
> > > + CameraManager *cm_;
> > > + std::string uvc_toplevel_;
> > > + bool cameraRemovedPass_;
> > > + bool cameraAddedPass_;
> > > +};
> > > +
> > > +TEST_REGISTER(HotplugTest)
> > > +
> > > diff --git a/test/meson.build b/test/meson.build
> > > index 5a45a85..383a7ea 100644
> > > --- a/test/meson.build
> > > +++ b/test/meson.build
> > > @@ -29,6 +29,7 @@ internal_tests = [
> > > ['file', 'file.cpp'],
> > > ['file-descriptor', 'file-descriptor.cpp'],
> > > ['message', 'message.cpp'],
> > > + ['hotplug-cameras', 'hotplug-cameras.cpp'],
> > > ['object', 'object.cpp'],
> > > ['object-invoke', 'object-invoke.cpp'],
> > > ['signal-threads', 'signal-threads.cpp'],
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list