| Message ID | 20200314235728.15495-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2020-03-15 01:57:20 +0200, Laurent Pinchart wrote: > There's no need to keep the video device open after allocating buffers, > as V4L2 supports buffer orphaning and the exported buffers will still be > usable. Close the device right after allocation to avoid the need for > delayed cleanups. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > test/libtest/buffer_source.cpp | 24 +++++++++--------------- > test/libtest/buffer_source.h | 1 - > 2 files changed, 9 insertions(+), 16 deletions(-) > > diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp > index 066049d342a4..0c33200b47ad 100644 > --- a/test/libtest/buffer_source.cpp > +++ b/test/libtest/buffer_source.cpp > @@ -8,26 +8,18 @@ > #include "buffer_source.h" > > #include <iostream> > +#include <memory> > > #include "device_enumerator.h" > > #include "test.h" > > BufferSource::BufferSource() > - : video_(nullptr) > { > } > > BufferSource::~BufferSource() > { > - if (video_) { > - video_->releaseBuffers(); > - video_->close(); > - } > - > - delete video_; > - video_ = nullptr; > - > if (media_) > media_->release(); > } > @@ -58,37 +50,39 @@ int BufferSource::allocate(const StreamConfiguration &config) > return TestSkip; > } > > - video_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName); > - if (!video_) { > + std::unique_ptr<V4L2VideoDevice> video{ V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName) }; > + if (!video) { > std::cout << "Failed to get video device from entity " > << videoDeviceName << std::endl; > return TestFail; > } > > - if (video_->open()) { > + if (video->open()) { > std::cout << "Unable to open " << videoDeviceName << std::endl; > return TestFail; > } > > /* Configure the format. */ > V4L2DeviceFormat format; > - if (video_->getFormat(&format)) { > + if (video->getFormat(&format)) { > std::cout << "Failed to get format on output device" << std::endl; > return TestFail; > } > > format.size = config.size; > format.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false); > - if (video_->setFormat(&format)) { > + if (video->setFormat(&format)) { > std::cout << "Failed to set format on output device" << std::endl; > return TestFail; > } > > - if (video_->exportBuffers(config.bufferCount, &buffers_) < 0) { > + if (video->exportBuffers(config.bufferCount, &buffers_) < 0) { > std::cout << "Failed to export buffers" << std::endl; > return TestFail; > } > > + video->close(); > + > return TestPass; > } > > diff --git a/test/libtest/buffer_source.h b/test/libtest/buffer_source.h > index 2d8fc5acf6d7..ae0879c99480 100644 > --- a/test/libtest/buffer_source.h > +++ b/test/libtest/buffer_source.h > @@ -25,7 +25,6 @@ public: > > private: > std::shared_ptr<MediaDevice> media_; > - V4L2VideoDevice *video_; > std::vector<std::unique_ptr<FrameBuffer>> buffers_; > }; > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp index 066049d342a4..0c33200b47ad 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -8,26 +8,18 @@ #include "buffer_source.h" #include <iostream> +#include <memory> #include "device_enumerator.h" #include "test.h" BufferSource::BufferSource() - : video_(nullptr) { } BufferSource::~BufferSource() { - if (video_) { - video_->releaseBuffers(); - video_->close(); - } - - delete video_; - video_ = nullptr; - if (media_) media_->release(); } @@ -58,37 +50,39 @@ int BufferSource::allocate(const StreamConfiguration &config) return TestSkip; } - video_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName); - if (!video_) { + std::unique_ptr<V4L2VideoDevice> video{ V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName) }; + if (!video) { std::cout << "Failed to get video device from entity " << videoDeviceName << std::endl; return TestFail; } - if (video_->open()) { + if (video->open()) { std::cout << "Unable to open " << videoDeviceName << std::endl; return TestFail; } /* Configure the format. */ V4L2DeviceFormat format; - if (video_->getFormat(&format)) { + if (video->getFormat(&format)) { std::cout << "Failed to get format on output device" << std::endl; return TestFail; } format.size = config.size; format.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false); - if (video_->setFormat(&format)) { + if (video->setFormat(&format)) { std::cout << "Failed to set format on output device" << std::endl; return TestFail; } - if (video_->exportBuffers(config.bufferCount, &buffers_) < 0) { + if (video->exportBuffers(config.bufferCount, &buffers_) < 0) { std::cout << "Failed to export buffers" << std::endl; return TestFail; } + video->close(); + return TestPass; } diff --git a/test/libtest/buffer_source.h b/test/libtest/buffer_source.h index 2d8fc5acf6d7..ae0879c99480 100644 --- a/test/libtest/buffer_source.h +++ b/test/libtest/buffer_source.h @@ -25,7 +25,6 @@ public: private: std::shared_ptr<MediaDevice> media_; - V4L2VideoDevice *video_; std::vector<std::unique_ptr<FrameBuffer>> buffers_; };
There's no need to keep the video device open after allocating buffers, as V4L2 supports buffer orphaning and the exported buffers will still be usable. Close the device right after allocation to avoid the need for delayed cleanups. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- test/libtest/buffer_source.cpp | 24 +++++++++--------------- test/libtest/buffer_source.h | 1 - 2 files changed, 9 insertions(+), 16 deletions(-)