On Mon, May 2, 2011 at 3:11 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > NACK. > > For two reasons: first of all it is not signed off by Andy Walls, the cx18 > maintainer. I know he has had other things on his plate recently which is > probably why he hasn't had the chance to review this. > > Secondly, while doing a quick scan myself I noticed that this code does a > conversion from UYVY format to YUYV *in the driver*. Format conversion is > not allowed in the kernel, we have libv4lconvert for that. So at the minimum > this conversion code must be removed first. Hi Hans, Cutting the code that does UYVY to YUYV shouldn't be a problem, since there are other devices which only support UYVY and thus applications do support the format (the HVR-950q for one). Should just need to remove the offending code block and adjust the advertised formats list. That said, Andy hasn't provided any feedback onlist at all, which is a bit disconcerting (and probably calls for "why won't Andy comment?" instead of an arbitrary NACK). I did speak to Andy about this patch series several months ago, and he was generally not in favor of it because he was planning on converting to videobuf2. While I agree this would be good in the long term, this patch provides a great deal of value in the meantime, and I've always been a fan of the notion that "perfect is the enemy of good". Who knows when we'll actually see a videobuf2 conversion, and this patch doesn't really prevent any of that from happening. I would hate to see yet another situation where a solution stays out-of-tree for years because of some totally awesome better approach which might possibly get integrated at some unknown point in the future. In other words, let's get this merged in (sans the UYVY/YUYV conversion), and if/when Andy eventually does a videobuf2 conversion, then we will all rejoice. Actually, nobody except us driver developers will rejoice since it's an internal architecture change which provides no user-visible value. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
