[vlc-devel] [PATCH] adjust: add 10-bit support (Fixes #9600)

[Tristan Matthews]

On Tue, May 5, 2015 at 11:33 AM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
> On 05 May, Tristan Matthews wrote :
>> ---
>>  modules/video_filter/adjust.c         | 153 ++++++++++++++++++++++++----------
>>  modules/video_filter/adjust_sat_hue.c | 124 +++++++++++++++++++++++----
>>  modules/video_filter/adjust_sat_hue.h |  12 +++
>>  3 files changed, 233 insertions(+), 56 deletions(-)
>>
>> diff --git a/modules/video_filter/adjust.c b/modules/video_filter/adjust.c
>> index 620f36e..9573f72 100644
>> --- a/modules/video_filter/adjust.c
>> +++ b/modules/video_filter/adjust.c
>> @@ -45,6 +45,12 @@
>>  #   define M_PI 3.14159265358979323846
>>  #endif
>>
>> +#define CASE_PLANAR_YUV10                   \
>> +        case VLC_CODEC_I420_10L:            \
>> +        case VLC_CODEC_I420_10B:            \
>> +        case VLC_CODEC_I444_10L:            \
>> +        case VLC_CODEC_I444_10B:
>
> This should go to modules/video_filter/filter_picture.h
>
>> -    int pi_luma[256];
>> -    int pi_gamma[256];
>> +    /* The full range will only be used for 10-bit */
>> +    int pi_luma[1024];
>> +    int pi_gamma[1024];
>
> x4 increase? This is surprising.

These are lookup tables, so their size is equal to the range of
possible values, hence 256 for 8-bit, 1024 for 10bit...a malloc would
also be possible here but maybe that's overkill?

>
>>          /* Fill the gamma lookup table */
>> -        for( unsigned i = 0 ; i < 256 ; i++ )
>> +        for( int i = 0 ; i < i_range; i++ )
>
> Why move from unsigned to signed in a loop ?

It could be left unsigned, it will just take one extra unsigned
variable to be used here instead of i_range.

>
> Why no 9 bits support?

That will be a separate patch.

Best,
Tristan