[libcamera-devel] [PATCH] cam: Limit file write to payload size

[Niklas Söderlund]

Hi Kieran, Laurent,

On 2020-08-24 10:32:24 +0100, Kieran Bingham wrote:
> Hi Laurent,
> 
> 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?

> 
> 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
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund