Hi, Jonathan


>-----Original Message-----
>From: Jonathan Corbet [mailto:cor...@lwn.net]
>Sent: Monday, 17 December, 2012 00:17
>To: Albert Wang
>Cc: g.liakhovet...@gmx.de; linux-media@vger.kernel.org; Libin Yang
>Subject: Re: [PATCH V3 06/15] [media] marvell-ccic: add new formats support for
>marvell-ccic driver
>
>On Sat, 15 Dec 2012 17:57:55 +0800
>Albert Wang <twan...@marvell.com> wrote:
>
>> From: Libin Yang <lby...@marvell.com>
>>
>> This patch adds the new formats support for marvell-ccic.
>
>Once again, just one second-order comment:
>
>> +static bool mcam_fmt_is_planar(__u32 pfmt)
>> +{
>> +    switch (pfmt) {
>> +    case V4L2_PIX_FMT_YUV422P:
>> +    case V4L2_PIX_FMT_YUV420:
>> +    case V4L2_PIX_FMT_YVU420:
>> +            return true;
>> +    }
>> +    return false;
>> +}
>
>This seems like the kind of thing that would be useful in a number of
>places; I'd be tempted to push it up a level and make it available to all
>V4L2 drivers.  Of course, that means making it work for *all* formats,
>which would be a pain.
>
>But, then, I can see some potential future pain if somebody adds a new
>format and forgets to tweak this function here.  Rather than adding a new
>switch, could you put a "planar" flag into struct mcam_format_struct
>instead?  That would help to keep all this information together.
>
[Albert Wang] Yes, it looks make sense, we will think about it in next version.

>jon


Thanks
Albert Wang
86-21-61092656
--
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

Reply via email to