[libcamera-devel] [PATCH] cam: Limit file write to payload size
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Aug 25 10:50:21 CEST 2020
On 24/08/2020 23:53, Laurent Pinchart wrote:
> On Tue, Aug 25, 2020 at 12:04:11AM +0200, Niklas Söderlund wrote:
>> On 2020-08-24 10:32:24 +0100, Kieran Bingham wrote:
>>> On 22/08/2020 14:40, Laurent Pinchart wrote:
>>>> The payload size in a captured framebuffer is usually equal to the
>>>> buffer size. However, for compressed formats, the payload may be
>>>> smaller Only write the payload when capturing to a file.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>> ---
>>>> src/cam/buffer_writer.cpp | 12 ++++++++++--
>>>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp
>>>> index c5a5eb46224a..6305958924a4 100644
>>>> --- a/src/cam/buffer_writer.cpp
>>>> +++ b/src/cam/buffer_writer.cpp
>>>> @@ -64,9 +64,17 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)
>>>> if (fd == -1)
>>>> return -errno;
>>>>
>>>> - for (const FrameBuffer::Plane &plane : buffer->planes()) {
>>>> + for (unsigned int i = 0; i < buffer->planes().size(); ++i) {
>>>> + const FrameBuffer::Plane &plane = buffer->planes()[i];
>>>> + const FrameMetadata::Plane &meta = buffer->metadata().planes[i];
>>>> +
>>>> void *data = mappedBuffers_[plane.fd.fd()].first;
>>>> - unsigned int length = plane.length;
>>>> + unsigned int length = std::min(meta.bytesused, plane.length);
>>>> +
>>>> + if (length < meta.bytesused)
>>>> + std::cerr << "payload size " << meta.bytesused
>>>> + << " smaller than plane size " << plane.length
>>>> + << std::endl;
>>>
>>> Is the cerr necessary? If saving out an MJPEG stream from an MJPEG UVC
>>> camera - this would print every frame...
>>
>> Now I'm confused :-)
>>
>> Would not the cerr only print if length is set to plane.length while
>> meta.bytesused is larger? I read this as plane.length being the buffer's
>> max allocated size while if the cerr triggers the driver is reporitng
>> that more then the max size is used as bytesused is larger then the
>> plane?
>
> The check is correct, but the message isn't. And the check could
> actually be be made clearer.
>
> if (meta.bytesused > plane.length)
> std::cerr << "payload size " << meta.bytesused
> << " larger than plane size " << plane.length
> << std::endl;
Ahh, now that would be a lot easier to parse.
+1
--
Kieran
>
>>> Which is however the use case that makes me believe this patch is useful...
>>>
>>> So with that considered either way,
>>>
>>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>>
>>>>
>>>> ret = ::write(fd, data, length);
>>>> if (ret < 0) {
>>>>
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list