On 10/29/14 14:17, Laurent Pinchart wrote: > Hi Mauro, > > On Wednesday 29 October 2014 11:05:34 Mauro Carvalho Chehab wrote: >> Em Wed, 29 Oct 2014 14:46:55 +0200 Laurent Pinchart escreveu: >>>>> Hmm, so you think VIDEO_MAX_FRAME should just be updated to 64? >>>> >>>> Yes. >>>> >>>>> I am a bit afraid that that might break applications (especially if >>>>> there are any that use bits in a 32-bit unsigned variable). >>>> >>>> What 32-bits have to do with that? This is just the maximum number of >>>> buffers, and not the number of bits. >>> >>> Applications might use a bitmask to track buffers. >> >> True, but then it should be limiting the max buffer to 32, if the >> implementation won't support more than 32 bits at its bitmask >> implementation. >> >> Anyway, we need to double check if nothing will break at the open >> source apps before being able to change its value. > > I don't think we should change the value of VIDEO_MAX_FRAME. Applications that > rely on it will thus allocate a maximum of 32 buffers, nothing should break > (provided that no driver requires a minimum number of buffers higher than 32). > >>>>> Should userspace know about this at all? I think that the maximum >>>>> number of frames is driver dependent, and in fact one of the future >>>>> vb2 improvements would be to stop hardcoding this and leave the >>>>> maximum up to the driver. >>>> >>>> It is not driver dependent. It basically depends on the streaming logic. >>>> Both VB and VB2 are free to set whatever size it is needed. They can >>>> even change the logic to use a linked list, to avoid pre-allocating >>>> anything. >>>> >>>> Ok, there's actually a hardware limit, with is the maximum amount of >>>> memory that could be used for DMA on a given hardware/architecture. >>>> >>>> The 32 limit was just a random number that was chosen. >>> >>> So, can't we just mark VIDEO_MAX_FRAME as deprecated ? We can't remove it >>> as applications might depend on it, but it's pretty useless otherwise. >> >> As I pointed below, even the applications _we_ wrote at v4l-utils use >> it. The good news is that I double-checked xawtv3, xawtv4 and tvtime: >> none of them use it. Perhaps we're lucky enough, but I wouldn't count >> with that. >> >> Ok, we can always write a note there saying that this is deprecated, >> but the same symbol is still used internally on the drivers. >> >> If we're willing to deprecate, we should do something like: >> >> #ifndef __KERNEL__ >> /* This define is deprecated because (...) */ >> #define VIDEO_MAX_FRAME 32 >> #endif >> >> And then remove all occurrences of it at Kernelspace. No problem. > > Agreed. > >> We should also first fix v4l-utils no not use it, as v4l-utils is currently >> the reference code for users. > > That sounds reasonable to me. There's no urgency, as nothing will break if an > application uses VIDEO_MAX_FRAME set to 32 while VB2 can support 64, but we > should still remove references to VIDEO_MAX_FRAME from v4l-utils. > >> Please notice, however, that v4l-compliance depends on it. I suspect that it >> wants/needs to test the maximum buffer size. What would be a reasonable way >> to replace it, and still be able to test the maximum buffer limit? > > I'll let Hans comment on that. None of this is difficult to do. And it would most likely improve the code as well. OK. How about this: 1) Remove the use of VIDEO_MAX_FRAME in the kernel by introducing VB1_MAX_FRAME and VB2_MAX_FRAME defines, initially both are set to 32. 2) Remove the use of VIDEO_MAX_FRAME in v4l-utils. 3) Patch VB2_MAX_FRAME to 64 and update the spec. Regards, Hans -- 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
