Hi!
> >>> Make lookup table size configurable at compile-time.
> >>
> >> I don't think I'll take this patch. The problem is that if we really add
> >> support for 10 or 12 bit lookup tables in the future, then just changing
> >> LSIZE isn't enough.
> >>
> >> This patch doesn't really add anything as it stands.
> >
> > Well, currently we have 256, 255 and 0xff sprinkled through the code,
> > when it means to say "lookup table size". That is quite wrong (because
> > you can't really grep "what depends on the table size).
> >
> > And BTW with the LSIZE set to 1024, 10 bit processing seems to
> > work. So it is already useful, at least for me.
> >
> > But now I noticed the patch is subtly wrong:
> >
> >>> -#define CLIP256(color) (((color) > 0xff) ? 0xff : (((color) < 0) ? 0 : (color)))
> >>> +#define CLIPLSIZE(color) (((color) > LSIZE) ? LSIZE : (((color) <
> > 0) ? 0 : (color)))
> >
> > This should be LSIZE-1.
> >
> > So I need to adjust the patch. But I'd still like you to take (fixed
> > version) for documentation purposes...
>
> I much rather do this as part of a longer series that actually adds 10 bit support.
>
> The problem is that adding support for 10 bit doesn't mean you can just use LSIZE
> all the time since you still need support for 8 bit as well.
>
> E.g. CLIPLSIZE makes no sense, I would expect to see a CLIP256 and a CLIP1024.
>
> So it becomes a bit more complex than just adding an LSIZE define.
Yes, proper 10 bit support will be more complex (and I'm not sure if
I'll be able to do it, I might need help there). OTOH... table size is
used at 7 places in three different files, and 256 is _very_ common
constant.
So IMO this makes sense regardless of full 10-bit support.
Best regards,
Pavel
> >>> #define CLIP(color, min, max) (((color) > (max)) ? (max) : (((color) < (min)) ? (min) : (color)))
> >>>
> >>> static int whitebalance_active(struct v4lprocessing_data *data)
> >>> @@ -111,10 +111,10 @@ static int whitebalance_calculate_lookup_tables_generic(
> >>>
> >>> avg_avg = (data->green_avg + data->comp1_avg + data->comp2_avg) / 3;
> >>>
> >>> - for (i = 0; i < 256; i++) {
> >>> - data->comp1[i] = CLIP256(data->comp1[i] * avg_avg / data->comp1_avg);
> >>> - data->green[i] = CLIP256(data->green[i] * avg_avg / data->green_avg);
> >>> - data->comp2[i] = CLIP256(data->comp2[i] * avg_avg / data->comp2_avg);
> >>> + for (i = 0; i < LSIZE; i++) {
> >>> + data->comp1[i] = CLIPLSIZE(data->comp1[i] * avg_avg / data->comp1_avg);
> >>> + data->green[i] = CLIPLSIZE(data->green[i] * avg_avg / data->green_avg);
> >>> + data->comp2[i] = CLIPLSIZE(data->comp2[i] * avg_avg / data->comp2_avg);
> >>> }
> >>>
> >>> return 1;
> >>>
> >
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: Digital signature
