Trent Piepho <xy...@speakeasy.org> writes: > Mauro, > > Please pull from http://linuxtv.org/hg/~tap/v4l-dvb > > This series adds a function for bounding and alignment image sizes and > modifies a number of drivers to use it. It came up when the pxa patches to > deal with the alignment issues for that driver were posted. I haven't > tested these patches for pxa.
Hi Trent, I didn't see the review of that serie, I'm curious what others said. As for my comments, I'll inline your code, sorry about that. > 02/14: v4l2: Create helper function for bounding and aligning images > http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=b4d3ec8d363d > +static unsigned int clamp_align(unsigned int x, unsigned int min, + unsigned int max, unsigned int align) +{ + /* Bits that must be zero to be aligned */ + unsigned int mask = ~((1 << align) - 1); + + /* Round to nearest aligned value */ + if (align) + x = (x + (1 << (align - 1))) & mask; If I'm not mistaken, these lines are an equivalent of : balign = 1 << align; if (align) x = ALIGN(x + 1 - balign/2, balign); Isn't that simpler to read ? + + /* Clamp to aligned value of min and max */ + if (x < min) + x = (min + ~mask) & mask; + else if (x > max) + x = max & mask; + + return x; +} > 03/14: pxa-camera: Use v4l bounding/alignment function > http://linuxtv.org/hg/~tap/v4l-dvb?cmd=changeset;node=cb48209c1841 + /* Limit to pxa hardware capabilities. YUV422P planar format requires + * images size to be a multiple of 16 bytes. If not, zeros will be + * inserted between Y and U planes, and U and V planes, which violates + * the YUV422P standard. */ The multiple lines comment format, according to the Coding Style, would be rather : /* * */ That is a question for Guennadi, as he is the maintainer of pxa_camera. + v4l2_bound_align_image(&pix->width, 48, 2048, 1, + &pix->height, 32, 2048, 0, + xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUV422P ? 4 : 0); Cheers. -- Robert -- 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
