RE: [PATCH 1/2] vb2: Add VB2_FILEIO_ALLOW_ZERO_BYTESUSED flag to vb2_fileio_flags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

> From: Hans Verkuil [mailto:hverkuil@xxxxxxxxx]
> Sent: Tuesday, February 17, 2015 10:11 AM
> 
> Hi Kamil,
> 
> On 12/16/14 12:36, Kamil Debski wrote:
> > The vb2: fix bytesused == 0 handling (8a75ffb) patch changed the
> > behavior of __fill_vb2_buffer function, so that if bytesused is 0 it
> > is set to the size of the buffer. However, bytesused set to 0 is used
> > by older codec drivers as as indication used to mark the end of
> stream.
> >
> > To keep backward compatibility, this patch adds a flag passed to the
> > vb2_queue_init function - VB2_FILEIO_ALLOW_ZERO_BYTESUSED. If the
> flag
> > is set upon initialization of the queue, the videobuf2 keeps the
> value
> > of bytesused intact and passes it to the driver.
> 
> What is the status of this patch series?

I have to admit that I had forgotten a bit about this patch, because of
other
important work. Thanks for reminding me :)
 
> Note that io_flags is really the wrong place for this flag, it should
> be io_modes. This flag has nothing to do with file I/O.

What do you think about adding a separate flags field into the vb2_queue
structure? This could be combined with changing io_flags to u8 or a bit
field
to save space.

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland

> 
> Regards,
> 
> 	Hans
> 
> >
> > Reported-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> > Signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx>
> > ---
> >  drivers/media/v4l2-core/videobuf2-core.c |   33
> ++++++++++++++++++++++++------
> >  include/media/videobuf2-core.h           |    3 +++
> >  2 files changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> > b/drivers/media/v4l2-core/videobuf2-core.c
> > index d09a891..1068dbb 100644
> > --- a/drivers/media/v4l2-core/videobuf2-core.c
> > +++ b/drivers/media/v4l2-core/videobuf2-core.c
> > @@ -1276,13 +1276,23 @@ static void __fill_vb2_buffer(struct
> vb2_buffer *vb, const struct v4l2_buffer *b
> >  			 * userspace clearly never bothered to set it and
> >  			 * it's a safe assumption that they really meant to
> >  			 * use the full plane sizes.
> > +			 *
> > +			 * Some drivers, e.g. old codec drivers, use
> bytesused
> > +			 * == 0 as a way to indicate that streaming is
> finished.
> > +			 * In that case, the driver should use the following
> > +			 * io_flag VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep
> old
> > +			 * userspace applications working.
> >  			 */
> >  			for (plane = 0; plane < vb->num_planes; ++plane) {
> >  				struct v4l2_plane *pdst =
&v4l2_planes[plane];
> >  				struct v4l2_plane *psrc =
&b->m.planes[plane];
> >
> > -				pdst->bytesused = psrc->bytesused ?
> > -					psrc->bytesused : pdst->length;
> > +				if (vb->vb2_queue->io_flags &
> > +					VB2_FILEIO_ALLOW_ZERO_BYTESUSED)
> > +					pdst->bytesused = psrc->bytesused;
> > +				else
> > +					pdst->bytesused = psrc->bytesused ?
> > +						psrc->bytesused :
pdst->length;
> >  				pdst->data_offset = psrc->data_offset;
> >  			}
> >  		}
> > @@ -1295,6 +1305,12 @@ static void __fill_vb2_buffer(struct
> vb2_buffer *vb, const struct v4l2_buffer *b
> >  		 *
> >  		 * If bytesused == 0 for the output buffer, then fall back
> >  		 * to the full buffer size as that's a sensible default.
> > +		 *
> > +		 * Some drivers, e.g. old codec drivers, use bytesused == 0
> > +		 * as a way to indicate that streaming is finished. In that
> > +		 * case, the driver should use the following io_flag
> > +		 * VB2_FILEIO_ALLOW_ZERO_BYTESUSED to keep old userspace
> > +		 * applications working.
> >  		 */
> >  		if (b->memory == V4L2_MEMORY_USERPTR) {
> >  			v4l2_planes[0].m.userptr = b->m.userptr; @@ -1306,11
> +1322,16 @@
> > static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct
> v4l2_buffer *b
> >  			v4l2_planes[0].length = b->length;
> >  		}
> >
> > -		if (V4L2_TYPE_IS_OUTPUT(b->type))
> > -			v4l2_planes[0].bytesused = b->bytesused ?
> > -				b->bytesused : v4l2_planes[0].length;
> > -		else
> > +		if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> > +			if (vb->vb2_queue->io_flags &
> > +				VB2_FILEIO_ALLOW_ZERO_BYTESUSED)
> > +				v4l2_planes[0].bytesused = b->bytesused;
> > +			else
> > +				v4l2_planes[0].bytesused = b->bytesused ?
> > +					b->bytesused :
v4l2_planes[0].length;
> > +		} else {
> >  			v4l2_planes[0].bytesused = 0;
> > +		}
> >
> >  	}
> >
> > diff --git a/include/media/videobuf2-core.h
> > b/include/media/videobuf2-core.h index bd2cec2..0540bc3 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -138,10 +138,13 @@ enum vb2_io_modes {
> >   * by default the 'streaming' style is used by the file io emulator
> >   * @VB2_FILEIO_READ_ONCE:	report EOF after reading the first buffer
> >   * @VB2_FILEIO_WRITE_IMMEDIATELY:	queue buffer after each
> write() call
> > + * @VB2_FILEIO_ALLOW_ZERO_BYTESUSED:	the driver setting this flag
> will handle
> > + *					bytesused == 0 as a special case
> >   */
> >  enum vb2_fileio_flags {
> >  	VB2_FILEIO_READ_ONCE		= (1 << 0),
> >  	VB2_FILEIO_WRITE_IMMEDIATELY	= (1 << 1),
> > +	VB2_FILEIO_ALLOW_ZERO_BYTESUSED	= (1 << 2),
> >  };
> >
> >  /**
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux