[libcamera-devel] [RFC 10/12] libcamera: buffer: Store buffer information in separate container
Andrey Konovalov
andrey.konovalov at linaro.org
Wed Nov 6 12:15:44 CET 2019
Hi,
On 06.11.2019 13:18, Jacopo Mondi wrote:
> 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.
Not sure if such a use case for libcamera could ever be possible,
but when people deal with secure playback, the CPU is often has no physical access to the (unencrypted)
buffer contents, but needs to process "the metadata" embedded into the stream. I don't know any details,
but I've heard they have to create a separate copy of those "metadata" just to make them accessible to the CPU.
So if there could be cases when the Buffer and the BufferInfo might have different access permissions,
having the BufferInfo separated from the Buffer, or movable would be a plus.
Thanks,
Andrey
> 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
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list