Hello, On Tue, Oct 04, 2022 at 12:23:24PM +0300, Sakari Ailus wrote: > On Tue, Oct 04, 2022 at 11:12:45AM +0200, Hans Verkuil wrote: > > On 10/2/22 01:48, Laurent Pinchart wrote: > > > Hello, > > > > > > While working on fixing occurrences of > > > > > > [ 54.375534] use of bytesused == 0 is deprecated and will be removed in the future, > > > [ 54.388026] use the actual size instead. > > > > > > in libcamera, I realized that the patch that initially introduced the > > > warning and deprecated setting bytesused to 0 didn't change the V4L2 API > > > specification, which still documents bytesused as > > > > > > [...] If the application sets this to 0 for an output stream, then > > > bytesused will be set to the size of the buffer (see the length > > > field of this struct) by the driver. [...] > > > > > > for both v4l2_buffer and v4l2_plane. > > > > > > This deprecated behaviour has been present in the kernel for 7 years and > > > a half now. I'm wondering if it's really deprecated, in which case we > > > should update the API specification, or if it should be considered > > > supported, in which case the warning should be dropped. > > > > It's a good question. > > > > I do believe it should be removed from the spec. It is IMHO a bad idea to > > just leave bytesused at 0 for an output buffer. Applications should be explicit. OK. I'll write a patch. > > But on the other hand, I think we need to keep the current behavior in the > > kernel of replacing bytesused == 0 with the length of the buffer. I don't > > think we can return an error in that case, it would almost certainly break > > userspace. Yes, I don't think we can change the implementation for the time being, the risk of breakages is just too high (I'm fixing the libcamera side though). > > Regarding the warning: I think we need to keep it, to warn applications that > > what they are doing is a bad idea, but that the text should change from: > > > > "use of bytesused == 0 is deprecated and will be removed in the future" > > > > to: > > > > "use of bytesused == 0 is not recommended" > > If we ever intend to drop this support, we should warn we're going to do > so. Otherwise there's little point in recommending against using it. I agree. Just saying it's not recommended is pointless. Either we want to deprecate this behaviour, which means that it may get removed in the future (one option could be to WARN_ONCE() for a few years, although even that may not be enough), or we conclude that removing it will never be possible, in which case I'd drop the message. > The > spec should document it as deprecated and to be removed in the future as > well. (Or alternatively, the warning should be removed altogether.) I wouldn't document it at all, if it's deprecated it doesn't deserve a mention in the spec. It's hard enough for people to understand how to do the right thing when reading documentation without being told about the things that work but shouldn't be done :-) -- Regards, Laurent Pinchart
