[libcamera-devel] [RFC 10/12] libcamera: buffer: Store buffer information in separate container
Jacopo Mondi
jacopo at jmondi.org
Wed Nov 6 11:18:30 CET 2019
Hi Niklas, Kieran,
On Fri, Nov 01, 2019 at 01:15:58PM +0000, Kieran Bingham wrote:
> Hi Niklas,
>
> On 28/10/2019 02:25, Niklas Söderlund wrote:
> > Use the new BufferInfo container to store information from when a buffer
> > is dequeued. This will aid in the ongoing buffer rework and simplify the
> > code.
> >
> > This commits breaks the buffer sharing test case as it only deals with
> > buffer information going out of the video device. The next patch will
> > restore the test as it will address the incoming information. All other
> > tests as well as cam and qcam works as expected on pipelines not require
> > buffer importing.
>
> I certainly like moving all the BufferInfo data to it's own class to
> keep it distinct (and tidy), but I wonder - why not then just put the
> BufferInfo into the Buffer?
>
> Then the association between the Buffer and the BufferInfo is
> maintained, rather than having to provide separate lists and determine
> which BufferInfo belongs to which Buffer?
>
> (Or at the very least, doing so in this patch might prevent the
> temporary test breakage you report?)
>
> Do we have cases where we have a Buffer - but we do not need or care
> about the associated BufferInfo data?
>
I was about to ask the same actually.
The BufferInfo is created by the V4L2VideoDevice at queueBuffer() and
streaOff() time, and associated to a Buffer for its whole lifetime.
I would store a BufferInfo * in a Buffer, but then the question is:
could the pointer be retreived and accessed before it gets initialized
by V4L2VideoDevice::queueBuffer() ? In that case I agree it might make
sense to make the BufferInfo accessible only after a Request or a
Buffer has completed, to guarantee the association is in place.
But then in that case, we should really avoid copies, so I would make
BufferInfo movable, as in Request::completeBuffer() the BufferInfo is
actually copied into the Request::info_ map.
Or we could use the BufferInfo as temporary instances. At
Request::completeBuffer() time, we use the fields of the BufferInfo
received as parameter to create a new BufferInfo and store it in the
map, but at that point, a move constructor might be better. As
BufferInfo has fields which are not trivially movable we need to
define our own move constructor. (right? BufferInfo has a vector, so
the implicitly movable constructor is not generated if I'm not
mistaken)
>
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > include/libcamera/request.h | 6 ++-
> > src/cam/buffer_writer.cpp | 5 ++-
> > src/cam/buffer_writer.h | 4 +-
> > src/cam/capture.cpp | 21 ++++-----
> > src/libcamera/include/pipeline_handler.h | 3 +-
> > src/libcamera/include/v4l2_videodevice.h | 4 +-
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 20 ++++-----
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 40 ++++++++---------
> > src/libcamera/pipeline/uvcvideo.cpp | 6 +--
> > src/libcamera/pipeline/vimc.cpp | 6 +--
> > src/libcamera/pipeline_handler.cpp | 5 ++-
> > src/libcamera/request.cpp | 6 ++-
> > src/libcamera/v4l2_videodevice.cpp | 55 ++++++++----------------
> > src/qcam/main_window.cpp | 18 ++++----
> > src/qcam/main_window.h | 2 +-
> > test/camera/capture.cpp | 4 +-
> > test/v4l2_videodevice/buffer_sharing.cpp | 17 ++++----
> > test/v4l2_videodevice/capture_async.cpp | 4 +-
> > test/v4l2_videodevice/v4l2_m2mdevice.cpp | 8 ++--
> > 19 files changed, 114 insertions(+), 120 deletions(-)
> >
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index 2d5a5964e99eb75f..88ef7bf03fcfb77b 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -12,12 +12,12 @@
> > #include <stdint.h>
> > #include <unordered_set>
> >
> > +#include <libcamera/buffer.h>
> > #include <libcamera/controls.h>
> > #include <libcamera/signal.h>
> >
> > namespace libcamera {
> >
> > -class Buffer;
> > class Camera;
> > class CameraControlValidator;
> > class Stream;
> > @@ -39,6 +39,7 @@ public:
> > ControlList &controls() { return *controls_; }
> > ControlList &metadata() { return *metadata_; }
> > const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
> > + const BufferInfo &info(Buffer *frame) const { return info_.find(frame)->second; };
> > int addBuffer(std::unique_ptr<Buffer> buffer);
> > Buffer *findBuffer(Stream *stream) const;
> >
> > @@ -54,13 +55,14 @@ private:
> > int prepare();
> > void complete();
> >
> > - bool completeBuffer(Buffer *buffer);
> > + bool completeBuffer(Buffer *buffer, const BufferInfo &info);
> >
> > Camera *camera_;
> > CameraControlValidator *validator_;
> > ControlList *controls_;
> > ControlList *metadata_;
> > std::map<Stream *, Buffer *> bufferMap_;
> > + std::map<Buffer *, BufferInfo> info_;
> > std::unordered_set<Buffer *> pending_;
> >
> > const uint64_t cookie_;
> > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp
> > index c33e99c5f8173db8..3ee9e82ba216abb6 100644
> > --- a/src/cam/buffer_writer.cpp
> > +++ b/src/cam/buffer_writer.cpp
> > @@ -21,7 +21,8 @@ BufferWriter::BufferWriter(const std::string &pattern)
> > {
> > }
> >
> > -int BufferWriter::write(Buffer *buffer, const std::string &streamName)
> > +int BufferWriter::write(Buffer *buffer, const BufferInfo &info,
> > + const std::string &streamName)
> > {
> > std::string filename;
> > size_t pos;
> > @@ -32,7 +33,7 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName)
> > if (pos != std::string::npos) {
> > std::stringstream ss;
> > ss << streamName << "-" << std::setw(6)
> > - << std::setfill('0') << buffer->sequence();
> > + << std::setfill('0') << info.sequence();
> > filename.replace(pos, 1, ss.str());
> > }
> >
> > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h
> > index 7bf785d1e83235ff..38f7045c7d4e68a4 100644
> > --- a/src/cam/buffer_writer.h
> > +++ b/src/cam/buffer_writer.h
> > @@ -16,7 +16,9 @@ class BufferWriter
> > public:
> > BufferWriter(const std::string &pattern = "frame-#.bin");
> >
> > - int write(libcamera::Buffer *buffer, const std::string &streamName);
> > + int write(libcamera::Buffer *buffer,
> > + const libcamera::BufferInfo &info,
> > + const std::string &streamName);
> >
> > private:
> > std::string pattern_;
> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > index e665d819fb777a90..251e9f86c86b508d 100644
> > --- a/src/cam/capture.cpp
> > +++ b/src/cam/capture.cpp
> > @@ -138,32 +138,33 @@ void Capture::requestComplete(Request *request)
> > if (request->status() == Request::RequestCancelled)
> > return;
> >
> > - const std::map<Stream *, Buffer *> &buffers = request->buffers();
> > -
> > std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
> > double fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count();
> > fps = last_ != std::chrono::steady_clock::time_point() && fps
> > ? 1000.0 / fps : 0.0;
> > last_ = now;
> >
> > - std::stringstream info;
> > - info << "fps: " << std::fixed << std::setprecision(2) << fps;
> > + std::stringstream infostr;
> > + infostr << "fps: " << std::fixed << std::setprecision(2) << fps;
> >
> > + const std::map<Stream *, Buffer *> &buffers = request->buffers();
> > for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> > Stream *stream = it->first;
> > Buffer *buffer = it->second;
> > + const BufferInfo &info = request->info(buffer);
> > const std::string &name = streamName_[stream];
> >
> > - info << " " << name
> > - << " (" << buffer->index() << ")"
> > - << " seq: " << std::setw(6) << std::setfill('0') << buffer->sequence()
> > - << " bytesused: " << buffer->bytesused();
> > + infostr << " " << name
> > + << " seq: " << std::setw(6) << std::setfill('0') << info.sequence();
> > +
> > + for (unsigned int i = 0; i < info.planes(); i++)
> > + infostr << " bytesused(" << i << "): " << info.plane(i).bytesused;
> >
> > if (writer_)
> > - writer_->write(buffer, name);
> > + writer_->write(buffer, info, name);
> > }
> >
> > - std::cout << info.str() << std::endl;
> > + std::cout << infostr.str() << std::endl;
> >
> > /*
> > * Create a new request and populate it with one buffer for each
> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> > index 6024357e266c2e2b..e6dbd7687021e4ff 100644
> > --- a/src/libcamera/include/pipeline_handler.h
> > +++ b/src/libcamera/include/pipeline_handler.h
> > @@ -81,7 +81,8 @@ public:
> >
> > virtual int queueRequest(Camera *camera, Request *request);
> >
> > - bool completeBuffer(Camera *camera, Request *request, Buffer *buffer);
> > + bool completeBuffer(Camera *camera, Request *request, Buffer *buffer,
> > + const BufferInfo &info);
> > void completeRequest(Camera *camera, Request *request);
> >
> > const char *name() const { return name_; }
> > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> > index 5b178339d0ce7e2c..01b90ab4654bfba2 100644
> > --- a/src/libcamera/include/v4l2_videodevice.h
> > +++ b/src/libcamera/include/v4l2_videodevice.h
> > @@ -12,6 +12,7 @@
> >
> > #include <linux/videodev2.h>
> >
> > +#include <libcamera/buffer.h>
> > #include <libcamera/geometry.h>
> > #include <libcamera/signal.h>
> >
> > @@ -166,7 +167,7 @@ public:
> >
> > int queueBuffer(Buffer *buffer);
> > std::vector<std::unique_ptr<Buffer>> queueAllBuffers();
> > - Signal<Buffer *> bufferReady;
> > + Signal<Buffer *, const BufferInfo &> bufferReady;
> >
> > int streamOn();
> > int streamOff();
> > @@ -194,7 +195,6 @@ private:
> > int createPlane(BufferMemory *buffer, unsigned int index,
> > unsigned int plane, unsigned int length);
> >
> > - Buffer *dequeueBuffer();
> > void bufferAvailable(EventNotifier *notifier);
> >
> > V4L2Capability caps_;
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 8aa5f34febf16585..01064ac09859155d 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -161,9 +161,9 @@ public:
> > delete vfStream_;
> > }
> >
> > - void imguOutputBufferReady(Buffer *buffer);
> > - void imguInputBufferReady(Buffer *buffer);
> > - void cio2BufferReady(Buffer *buffer);
> > + void imguOutputBufferReady(Buffer *buffer, const BufferInfo &info);
> > + void imguInputBufferReady(Buffer *buffer, const BufferInfo &info);
> > + void cio2BufferReady(Buffer *buffer, const BufferInfo &info);
> >
> > CIO2Device cio2_;
> > ImgUDevice *imgu_;
> > @@ -931,10 +931,10 @@ int PipelineHandlerIPU3::registerCameras()
> > * Buffers completed from the ImgU input are immediately queued back to the
> > * CIO2 unit to continue frame capture.
> > */
> > -void IPU3CameraData::imguInputBufferReady(Buffer *buffer)
> > +void IPU3CameraData::imguInputBufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > /* \todo Handle buffer failures when state is set to BufferError. */
> > - if (buffer->status() == Buffer::BufferCancelled)
> > + if (info.status() == BufferInfo::BufferCancelled)
> > return;
> >
> > cio2_.output_->queueBuffer(buffer);
> > @@ -946,15 +946,13 @@ void IPU3CameraData::imguInputBufferReady(Buffer *buffer)
> > *
> > * Buffers completed from the ImgU output are directed to the application.
> > */
> > -void IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
> > +void IPU3CameraData::imguOutputBufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > Request *request = requestFromBuffer(buffer);
> >
> > - if (!pipe_->completeBuffer(camera_, request, buffer))
> > - /* Request not completed yet, return here. */
> > + if (!pipe_->completeBuffer(camera_, request, buffer, info))
> > return;
> >
> > - /* Mark the request as complete. */
> > pipe_->completeRequest(camera_, request);
> > }
> >
> > @@ -965,10 +963,10 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
> > * Buffers completed from the CIO2 are immediately queued to the ImgU unit
> > * for further processing.
> > */
> > -void IPU3CameraData::cio2BufferReady(Buffer *buffer)
> > +void IPU3CameraData::cio2BufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > /* \todo Handle buffer failures when state is set to BufferError. */
> > - if (buffer->status() == Buffer::BufferCancelled)
> > + if (info.status() == BufferInfo::BufferCancelled)
> > return;
> >
> > imgu_->input_->queueBuffer(buffer);
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 0803572754364beb..33a058de18b8cf2e 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -88,7 +88,7 @@ public:
> > setDelay(QueueBuffers, -1, 10);
> > }
> >
> > - void bufferReady(Buffer *buffer)
> > + void bufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > /*
> > * Calculate SOE by taking the end of DMA set by the kernel and applying
> > @@ -99,10 +99,10 @@ public:
> > ASSERT(frameOffset(SOE) == 0);
> >
> > utils::time_point soe = std::chrono::time_point<utils::clock>()
> > - + std::chrono::nanoseconds(buffer->timestamp())
> > + + std::chrono::nanoseconds(info.timestamp())
> > + timeOffset(SOE);
> >
> > - notifyStartOfExposure(buffer->sequence(), soe);
> > + notifyStartOfExposure(info.sequence(), soe);
> > }
> >
> > void setDelay(unsigned int type, int frame, int msdelay)
> > @@ -202,9 +202,9 @@ private:
> > int initLinks();
> > int createCamera(MediaEntity *sensor);
> > void tryCompleteRequest(Request *request);
> > - void bufferReady(Buffer *buffer);
> > - void paramReady(Buffer *buffer);
> > - void statReady(Buffer *buffer);
> > + void bufferReady(Buffer *buffer, const BufferInfo &info);
> > + void paramReady(Buffer *buffer, const BufferInfo &info);
> > + void statReady(Buffer *buffer, const BufferInfo &info);
> >
> > MediaDevice *media_;
> > V4L2Subdevice *dphy_;
> > @@ -987,43 +987,43 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
> > data->frameInfo_.destroy(info->frame);
> > }
> >
> > -void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)
> > +void PipelineHandlerRkISP1::bufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > ASSERT(activeCamera_);
> > RkISP1CameraData *data = cameraData(activeCamera_);
> > Request *request = data->requestFromBuffer(buffer);
> >
> > - data->timeline_.bufferReady(buffer);
> > + data->timeline_.bufferReady(buffer, info);
> >
> > - if (data->frame_ <= buffer->sequence())
> > - data->frame_ = buffer->sequence() + 1;
> > + if (data->frame_ <= info.sequence())
> > + data->frame_ = info.sequence() + 1;
> >
> > - completeBuffer(activeCamera_, request, buffer);
> > + completeBuffer(activeCamera_, request, buffer, info);
> > tryCompleteRequest(request);
> > }
> >
> > -void PipelineHandlerRkISP1::paramReady(Buffer *buffer)
> > +void PipelineHandlerRkISP1::paramReady(Buffer *buffer, const BufferInfo &info)
> > {
> > ASSERT(activeCamera_);
> > RkISP1CameraData *data = cameraData(activeCamera_);
> >
> > - RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> > + RkISP1FrameInfo *rkinfo = data->frameInfo_.find(buffer);
> >
> > - info->paramDequeued = true;
> > - tryCompleteRequest(info->request);
> > + rkinfo->paramDequeued = true;
> > + tryCompleteRequest(rkinfo->request);
> > }
> >
> > -void PipelineHandlerRkISP1::statReady(Buffer *buffer)
> > +void PipelineHandlerRkISP1::statReady(Buffer *buffer, const BufferInfo &info)
> > {
> > ASSERT(activeCamera_);
> > RkISP1CameraData *data = cameraData(activeCamera_);
> >
> > - RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> > - if (!info)
> > + RkISP1FrameInfo *rkinfo = data->frameInfo_.find(buffer);
> > + if (!rkinfo)
> > return;
> >
> > - unsigned int frame = info->frame;
> > - unsigned int statid = RKISP1_STAT_BASE | info->statBuffer->index();
> > + unsigned int frame = rkinfo->frame;
> > + unsigned int statid = RKISP1_STAT_BASE | rkinfo->statBuffer->index();
> >
> > IPAOperationData op;
> > op.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index 679d82d38227b991..ef2e8c9734f844ce 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -42,7 +42,7 @@ public:
> > }
> >
> > int init(MediaEntity *entity);
> > - void bufferReady(Buffer *buffer);
> > + void bufferReady(Buffer *buffer, const BufferInfo &info);
> >
> > V4L2VideoDevice *video_;
> > Stream *stream_;
> > @@ -373,11 +373,11 @@ int UVCCameraData::init(MediaEntity *entity)
> > return 0;
> > }
> >
> > -void UVCCameraData::bufferReady(Buffer *buffer)
> > +void UVCCameraData::bufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > Request *request = requestFromBuffer(buffer);
> >
> > - pipe_->completeBuffer(camera_, request, buffer);
> > + pipe_->completeBuffer(camera_, request, buffer, info);
> > pipe_->completeRequest(camera_, request);
> > }
> >
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index 56898716a8cde074..e3eefc49135179f2 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -56,7 +56,7 @@ public:
> > }
> >
> > int init(MediaDevice *media);
> > - void bufferReady(Buffer *buffer);
> > + void bufferReady(Buffer *buffer, const BufferInfo &info);
> >
> > CameraSensor *sensor_;
> > V4L2Subdevice *debayer_;
> > @@ -458,11 +458,11 @@ int VimcCameraData::init(MediaDevice *media)
> > return 0;
> > }
> >
> > -void VimcCameraData::bufferReady(Buffer *buffer)
> > +void VimcCameraData::bufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > Request *request = requestFromBuffer(buffer);
> >
> > - pipe_->completeBuffer(camera_, request, buffer);
> > + pipe_->completeBuffer(camera_, request, buffer, info);
> > pipe_->completeRequest(camera_, request);
> > }
> >
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index d70e286661aded8e..9ce9432449e0e133 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -393,10 +393,11 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)
> > * otherwise
> > */
> > bool PipelineHandler::completeBuffer(Camera *camera, Request *request,
> > - Buffer *buffer)
> > + Buffer *buffer, const BufferInfo &info)
> > {
> > + bool ret = request->completeBuffer(buffer, info);
> > camera->bufferCompleted.emit(request, buffer);
> > - return request->completeBuffer(buffer);
> > + return ret;
> > }
> >
> > /**
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index a9468ed4b0512a7f..9a0e439a3d05d780 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -252,12 +252,14 @@ void Request::complete()
> > * \return True if all buffers contained in the request have completed, false
> > * otherwise
> > */
> > -bool Request::completeBuffer(Buffer *buffer)
> > +bool Request::completeBuffer(Buffer *buffer, const BufferInfo &info)
> > {
> > int ret = pending_.erase(buffer);
> > ASSERT(ret == 1);
> >
> > - if (buffer->status() == Buffer::BufferCancelled)
> > + info_.emplace(buffer, info);
> > +
> > + if (info.status() == BufferInfo::BufferCancelled)
> > cancelled_ = true;
> >
> > return !hasPendingBuffers();
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 8bc2e439e4faeb68..97c6722b8c4c98cf 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -1119,14 +1119,16 @@ std::vector<std::unique_ptr<Buffer>> V4L2VideoDevice::queueAllBuffers()
> > }
> >
> > /**
> > - * \brief Dequeue the next available buffer from the video device
> > + * \brief Slot to handle completed buffer events from the V4L2 video device
> > + * \param[in] notifier The event notifier
> > *
> > - * This method dequeues the next available buffer from the device. If no buffer
> > - * is available to be dequeued it will return nullptr immediately.
> > + * When this slot is called, a Buffer has become available from the device, and
> > + * will be emitted through the bufferReady Signal.
> > *
> > - * \return A pointer to the dequeued buffer on success, or nullptr otherwise
> > + * For Capture video devices the Buffer will contain valid data.
> > + * For Output video devices the Buffer can be considered empty.
> > */
> > -Buffer *V4L2VideoDevice::dequeueBuffer()
> > +void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier)
> > {
> > struct v4l2_buffer buf = {};
> > struct v4l2_plane planes[VIDEO_MAX_PLANES] = {};
> > @@ -1144,10 +1146,10 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
> > if (ret < 0) {
> > LOG(V4L2, Error)
> > << "Failed to dequeue buffer: " << strerror(-ret);
> > - return nullptr;
> > + return;
> > }
> >
> > - ASSERT(buf.index < bufferPool_->count());
> > + LOG(V4L2, Debug) << "Buffer " << buf.index << " is available";
> >
> > auto it = queuedBuffers_.find(buf.index);
> > Buffer *buffer = it->second;
> > @@ -1156,37 +1158,16 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
> > if (queuedBuffers_.empty())
> > fdEvent_->setEnabled(false);
> >
> > - buffer->index_ = buf.index;
> > - buffer->bytesused_ = buf.bytesused;
> > - buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL
> > - + buf.timestamp.tv_usec * 1000ULL;
> > - buffer->sequence_ = buf.sequence;
> > - buffer->status_ = buf.flags & V4L2_BUF_FLAG_ERROR
> > - ? Buffer::BufferError : Buffer::BufferSuccess;
> > -
> > - return buffer;
> > -}
> > + BufferInfo::Status status = buf.flags & V4L2_BUF_FLAG_ERROR
> > + ? BufferInfo::BufferError : BufferInfo::BufferSuccess;
> > + uint64_t timestamp = buf.timestamp.tv_sec * 1000000000ULL
> > + + buf.timestamp.tv_usec * 1000ULL;
> >
> > -/**
> > - * \brief Slot to handle completed buffer events from the V4L2 video device
> > - * \param[in] notifier The event notifier
> > - *
> > - * When this slot is called, a Buffer has become available from the device, and
> > - * will be emitted through the bufferReady Signal.
> > - *
> > - * For Capture video devices the Buffer will contain valid data.
> > - * For Output video devices the Buffer can be considered empty.
> > - */
> > -void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier)
> > -{
> > - Buffer *buffer = dequeueBuffer();
> > - if (!buffer)
> > - return;
> > -
> > - LOG(V4L2, Debug) << "Buffer " << buffer->index() << " is available";
> > + BufferInfo info(status, buf.sequence, timestamp, { { buf.bytesused } });
> > + buffer->index_ = buf.index;
> >
> > /* Notify anyone listening to the device. */
> > - bufferReady.emit(buffer);
> > + bufferReady.emit(buffer, info);
> > }
> >
> > /**
> > @@ -1238,9 +1219,11 @@ int V4L2VideoDevice::streamOff()
> > unsigned int index = it.first;
> > Buffer *buffer = it.second;
> >
> > + BufferInfo info(BufferInfo::BufferCancelled, 0, 0, { {} });
> > +
> > buffer->index_ = index;
> > buffer->cancel();
> > - bufferReady.emit(buffer);
> > + bufferReady.emit(buffer, info);
> > }
> >
> > queuedBuffers_.clear();
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index cca7365ae75687f9..2d7f4ba84fbb6838 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -258,19 +258,19 @@ void MainWindow::requestComplete(Request *request)
> > framesCaptured_++;
> >
> > Buffer *buffer = buffers.begin()->second;
> > + const BufferInfo &info = request->info(buffer);
> >
> > - double fps = buffer->timestamp() - lastBufferTime_;
> > + double fps = info.timestamp() - lastBufferTime_;
> > fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0;
> > - lastBufferTime_ = buffer->timestamp();
> > + lastBufferTime_ = info.timestamp();
> >
> > - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence()
> > - << " buf: " << buffer->index()
> > - << " bytesused: " << buffer->bytesused()
> > - << " timestamp: " << buffer->timestamp()
> > + std::cout << "seq: " << std::setw(6) << std::setfill('0') << info.sequence()
> > + << " bytesused: " << info.plane(0).bytesused
> > + << " timestamp: " << info.timestamp()
> > << " fps: " << std::fixed << std::setprecision(2) << fps
> > << std::endl;
> >
> > - display(buffer);
> > + display(buffer, info);
> >
> > request = camera_->createRequest();
> > if (!request) {
> > @@ -295,7 +295,7 @@ void MainWindow::requestComplete(Request *request)
> > camera_->queueRequest(request);
> > }
> >
> > -int MainWindow::display(Buffer *buffer)
> > +int MainWindow::display(Buffer *buffer, const BufferInfo &info)
> > {
> > BufferMemory *mem = buffer->mem();
> > if (mem->planes().size() != 1)
> > @@ -303,7 +303,7 @@ int MainWindow::display(Buffer *buffer)
> >
> > Plane &plane = mem->planes().front();
> > unsigned char *raw = static_cast<unsigned char *>(plane.mem());
> > - viewfinder_->display(raw, buffer->bytesused());
> > + viewfinder_->display(raw, info.plane(0).bytesused);
> >
> > return 0;
> > }
> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> > index 78511581a8d5c025..8cbd72eb0d63cbea 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -50,7 +50,7 @@ private:
> > void stopCapture();
> >
> > void requestComplete(Request *request);
> > - int display(Buffer *buffer);
> > + int display(Buffer *frame, const BufferInfo &info);
> >
> > QString title_;
> > QTimer titleTimer_;
> > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> > index 83f974749affd3cd..3cf5b798448d01db 100644
> > --- a/test/camera/capture.cpp
> > +++ b/test/camera/capture.cpp
> > @@ -21,7 +21,9 @@ protected:
> >
> > void bufferComplete(Request *request, Buffer *buffer)
> > {
> > - if (buffer->status() != Buffer::BufferSuccess)
> > + const BufferInfo &info = request->info(buffer);
> > +
> > + if (info.status() != BufferInfo::BufferSuccess)
> > return;
> >
> > completeBuffersCount_++;
> > diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp
> > index 1629f34cfa6c79fe..3609c331cf10e056 100644
> > --- a/test/v4l2_videodevice/buffer_sharing.cpp
> > +++ b/test/v4l2_videodevice/buffer_sharing.cpp
> > @@ -90,24 +90,25 @@ protected:
> > return 0;
> > }
> >
> > - void captureBufferReady(Buffer *buffer)
> > + void captureBufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > - std::cout << "Received capture buffer: " << buffer->index()
> > - << " sequence " << buffer->sequence() << std::endl;
> > + std::cout << "Received capture buffer sequence "
> > + << info.sequence() << std::endl;
> >
> > - if (buffer->status() != Buffer::BufferSuccess)
> > + if (info.status() != BufferInfo::BufferSuccess)
> > return;
> >
> > + BufferInfo infocopy = info;
> > output_->queueBuffer(buffer);
> > framesCaptured_++;
> > }
> >
> > - void outputBufferReady(Buffer *buffer)
> > + void outputBufferReady(Buffer *buffer, const BufferInfo &info)
> > {
> > - std::cout << "Received output buffer: " << buffer->index()
> > - << " sequence " << buffer->sequence() << std::endl;
> > + std::cout << "Received output buffer sequence "
> > + << info.sequence() << std::endl;
> >
> > - if (buffer->status() != Buffer::BufferSuccess)
> > + if (info.status() != BufferInfo::BufferSuccess)
> > return;
> >
> > capture_->queueBuffer(buffer);
> > diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp
> > index 442a4fe56eace57e..26ea1d17fd901ef8 100644
> > --- a/test/v4l2_videodevice/capture_async.cpp
> > +++ b/test/v4l2_videodevice/capture_async.cpp
> > @@ -20,9 +20,9 @@ public:
> > CaptureAsyncTest()
> > : V4L2VideoDeviceTest("vimc", "Raw Capture 0"), frames(0) {}
> >
> > - void receiveBuffer(Buffer *buffer)
> > + void receiveBuffer(Buffer *buffer, const BufferInfo &info)
> > {
> > - std::cout << "Received buffer " << buffer->index() << std::endl;
> > + std::cout << "Received buffer" << std::endl;
> > frames++;
> >
> > /* Requeue the buffer for further use. */
> > diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp
> > index 4d3644c2d28792f1..e6ca90a4604dfcda 100644
> > --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp
> > +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp
> > @@ -29,9 +29,9 @@ public:
> > {
> > }
> >
> > - void outputBufferComplete(Buffer *buffer)
> > + void outputBufferComplete(Buffer *buffer, const BufferInfo &info)
> > {
> > - cout << "Received output buffer " << buffer->index() << endl;
> > + cout << "Received output buffer" << endl;
> >
> > outputFrames_++;
> >
> > @@ -39,9 +39,9 @@ public:
> > vim2m_->output()->queueBuffer(buffer);
> > }
> >
> > - void receiveCaptureBuffer(Buffer *buffer)
> > + void receiveCaptureBuffer(Buffer *buffer, const BufferInfo &info)
> > {
> > - cout << "Received capture buffer " << buffer->index() << endl;
> > + cout << "Received capture buffer" << endl;
>
> I actually liked seeing the indexes in my output - so I can see which
> buffers are in use... I guess we no longer have anything that maps to
> the index though...
>
> Perhaps at least making that info->sequence() would be better to keep
> some sort of a view of active data processing.
>
>
> > captureFrames_++;
> >
> >
>
> --
> Regards
> --
> Kieran
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191106/beb4c687/attachment-0001.sig>
More information about the libcamera-devel
mailing list