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,

> From: Hans Verkuil [mailto:hverkuil@xxxxxxxxx]
> Sent: Wednesday, February 18, 2015 10:58 AM
> 
> On 02/18/15 10:42, Kamil Debski wrote:
> > 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.
> 
> I think changing io_flags to a bitfield is a good idea.
> 
> 	unsigned fileio_read_once:1;
> 	unsigned fileio_write_immediately:1;
> 	unsigned allow_zero_bytesused:1;
> 
> However, going back to allow_zero_bytesused: this has been broken for
> quite some time without anyone complaining (other than you :-) ). 

If I remember correctly, it was Nicolas who reported to me the problem 
on the IRC.

> Might
> it not be better to just fix this properly by calling
> V4L2_DEC_CMD_STOP, as done here: https://www.mail-archive.com/linux-
> media@xxxxxxxxxxxxxxx/msg84916.html,
> and drop the support for zero bytesused to mark EOS entirely?

I think it would be good to have the backward compatibility for some time.

> I might be too optimistic here. Or perhaps at least add a pr_warn
> telling users to switch to V4L2_DEC_CMD_STOP since this will be removed
> in 2017 or whatever.

Where do you see the pr_warn? I guess it would be good if it was only
displayed once and only when the app uses bytesused == 0 to signal the
EOS. Do you think alike?


> 
> Regards,
> 
> 	Hans

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

--
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