Em Wed, 29 Oct 2014 14:53:53 +0100 Hans Verkuil <hverk...@xs4all.nl> escreveu:
> 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. Why to have two separate macros Kernelside? Just use VB_MAX_FRAME for both. > 2) Remove the use of VIDEO_MAX_FRAME in v4l-utils. > 3) Patch VB2_MAX_FRAME to 64 and update the spec. Steps (2) and (3) sounds ok to me, provided that we keep using the same macro. If are there any specific issue on some driver that can't support 64 buffers, then we can just add a driver-specific maximum value and use it instead. Regards, Mauro > > Regards, > > Hans > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
