Re: [PATCH v2 2/2] libv4lconvert: Support for RGB32 and BGR32 format

[Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>]

ping?

On Sun, Aug 4, 2013 at 10:05 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Hello Gregor
>
> Thanks for your comments. I have replied inline.
>
> On Sat, Aug 3, 2013 at 6:42 PM, Gregor Jasny <gjasny@googlemail.com> wrote:
>> On 8/3/13 12:42 AM, Ricardo Ribalda Delgado wrote:
>>>
>>> +       case V4L2_PIX_FMT_RGB32:
>>> +               switch (dest_pix_fmt) {
>>> +               case V4L2_PIX_FMT_RGB24:
>>> +                       v4lconvert_rgb32_to_rgb24(src, dest, width,
>>> height, 0);
>>> +                       break;
>>> +               case V4L2_PIX_FMT_BGR24:
>>> +                       v4lconvert_rgb32_to_rgb24(src, dest, width,
>>> height, 1);
>>> +                       break;
>>> +               case V4L2_PIX_FMT_YUV420:
>>> +                       v4lconvert_rgb24_to_yuv420(src, dest, fmt, 0, 0,
>>> 4);
>>> +                       break;
>>> +               case V4L2_PIX_FMT_YVU420:
>>> +                       v4lconvert_rgb24_to_yuv420(src, dest, fmt, 0, 1,
>>> 4);
>>> +                       break;
>>> +               }
>>> +               if (src_size < (width * height * 4)) {
>>> +                       V4LCONVERT_ERR("short rgb32 data frame\n");
>>> +                       errno = EPIPE;
>>> +                       result = -1;
>>> +               }
>>> +               break;
>>
>>
>> I have not looked at the whole function but shouldn't this sanity check
>> happen before the actual work?
>
> Yes, but it is how it is done in the whole library with all the
> formats. Please grep for "short " on libv4lconvert.c
>
>> Also aren't you applying the condition here
>> also for rgb24_to_xxx which should have only three bpp?
>>
>
> I have modified the function rgb24_to_yuv420 to support other bytes per pixel.
>
>>
>>> +       case V4L2_PIX_FMT_BGR32:
>>> +               switch (dest_pix_fmt) {
>>> +               case V4L2_PIX_FMT_RGB24:
>>> +                       v4lconvert_rgb32_to_rgb24(src, dest, width,
>>> height, 1);
>>> +                       break;
>>> +               case V4L2_PIX_FMT_BGR24:
>>> +                       v4lconvert_rgb32_to_rgb24(src, dest, width,
>>> height, 0);
>>> +                       break;
>>> +               case V4L2_PIX_FMT_YUV420:
>>> +                       v4lconvert_rgb24_to_yuv420(src, dest, fmt, 1, 0,
>>> 4);
>>> +                       break;
>>> +               case V4L2_PIX_FMT_YVU420:
>>> +                       v4lconvert_rgb24_to_yuv420(src, dest, fmt, 1, 1,
>>> 4);
>>> +                       break;
>>> +               }
>>> +               if (src_size < (width * height * 4)) {
>>> +                       V4LCONVERT_ERR("short bgr32 data frame\n");
>>> +                       errno = EPIPE;
>>> +                       result = -1;
>>> +               }
>>> +               break;
>>
>>
>> Same here. And also in the other patch.
>>
>>
>
> Thanks again
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda