Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT

[Antti Palosaari <crope@iki.fi>]

Hello Hans!
Actually I has already fixed version according to Andy and Hans V. 
comments, which I was just planning to send for you too, just due to 
that issue format selection issue!



On 16.11.2013 19:27, Hans de Goede wrote:
> Hi,
>> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct
>> v4lconvert_data *data,
>>       { V4L2_PIX_FMT_RGB24,        24,     1,     5,    0 }, \
>>       { V4L2_PIX_FMT_BGR24,        24,     1,     5,    0 }, \
>>       { V4L2_PIX_FMT_YUV420,        12,     6,     1,    0 }, \
>> -    { V4L2_PIX_FMT_YVU420,        12,     6,     1,    0 }
>> +    { V4L2_PIX_FMT_YVU420,        12,     6,     1,    0 }, \
>> +    { V4L2_PIX_FMT_FLOAT,         0,     0,     0,    0 }
>>
>
> This looks wrong, here you claim that V4L2_PIX_FMT_FLOAT is a supported
> destination
> format. which suggests there will be conversion code from any of the
> supported_src_pixfmts to it, which you don't add (and I don't think we
> will want
> to add.
>
>>   static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>>       SUPPORTED_DST_PIXFMTS,
>> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt
>> supported_src_pixfmts[] = {
>>       { V4L2_PIX_FMT_Y6,         8,    20,    20,    0 },
>>       { V4L2_PIX_FMT_Y10BPACK,    10,    20,    20,    0 },
>>       { V4L2_PIX_FMT_Y16,        16,    20,    20,    0 },
>> +    /* SDR formats */
>> +    { V4L2_PIX_FMT_U8,        0,    0,    0,    0 },
>>   };
>
> Likewise this will tell libv4lconvert that it can convert from
> V4L2_PIX_FMT_U8 to
> any of the supported destination formats, which again is not true.
>
> I suggest simply adding a hardcoded test for the SDR formats to relevant
> code paths
> which use supported_src_pixfmts and when seeing V4L2_PIX_FMT_U8 as
> source only
> support V4L2_PIX_FMT_FLOAT as dest, and short-circuit a whole bunch of
> other tests
> done.

Sounds reasonable. I will try implement it after I do some more 
conversions tests.

regards
Antti

-- 
http://palosaari.fi/