Guennadi Liakhovetski <g.liakhovetski@xxxxxx> writes:
>> @@ -162,6 +162,8 @@
>> CICR0_PERRM | CICR0_QDM | CICR0_CDM | CICR0_SOFM | \
>> CICR0_EOFM | CICR0_FOM)
>>
>> +#define PIX_YUV422P_ALIGN 16 /* YUV422P pix size should be a multiple of 16 */
>
> What is a "pix size?" Did you mean "picture size?"
Yes. I'll change the comment from "pix size" into "picture size"
>> - /* planar capture requires Y, U and V buffers to be page aligned */
>> - if (pcdev->channels == 3) {
>> - *size = PAGE_ALIGN(icd->width * icd->height); /* Y pages */
>> - *size += PAGE_ALIGN(icd->width * icd->height / 2); /* U pages */
>> - *size += PAGE_ALIGN(icd->width * icd->height / 2); /* V pages */
>> - } else {
>> - *size = icd->width * icd->height *
>> - ((icd->current_fmt->depth + 7) >> 3);
>> - }
>> + if (pcdev->channels == 3)
>> + *size = icd->width * icd->height * 2;
>
> This is not very obvious, why "* 2". Maybe use
>
> pxa_camera_formats[0].depth / 8 or at least add a comment?
Yes.
I was wondering about simplifying the if (removing it actually), and changing :
>> + if (pcdev->channels == 3)
>> + *size = icd->width * icd->height * 2;
>> + else
>> + *size = roundup(icd->width * icd->height *
>> + ((icd->current_fmt->depth + 7) >> 3), 8);
into:
*size = roundup(icd->width * icd->height *
((icd->current_fmt->depth + 7) >> 3), 8);
>> + if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUV422P) {
>> + if (!IS_ALIGNED(pix->width * pix->height, PIX_YUV422P_ALIGN))
>> + pix->height = ALIGN(pix->height, PIX_YUV422P_ALIGN / 2);
>> + if (!IS_ALIGNED(pix->width * pix->height, PIX_YUV422P_ALIGN))
>> + pix->width = ALIGN(pix->width, PIX_YUV422P_ALIGN / 2);
>
> Shouldn't this have been sqrt(PIX_YUV422P_ALIGN) (of course, not
> literally) instead of PIX_YUV422P_ALIGN / 2? At least above you say,
> height and width shall be 4 bytes aligned, not 8.
That's a very good catch.
Maybe 2 defines will fit better, as I'm not very please with log2 logic here ... :
/*
* YUV422P picture size should be a multiple of 16, so the heuristic aligns
* height, width on 4 byte boundaries to reach the 16 multiple for the size.
*/
#define YUV422P_X_Y_ALIGN 4
#define YUV422P_SIZE_ALIGN YUV422P_X_Y_ALIGN * YUV422P_X_Y_ALIGN
Cheers.
--
Robert
--
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
