On Fri, Dec 16, 2011 at 1:45 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Fri, 2011-12-16 at 10:26 +0530, Mahapatra, Chandrabhanu wrote:
>> On Thu, Dec 15, 2011 at 2:25 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
>> > On Wed, 2011-12-14 at 10:21 +0530, Chandrabhanu Mahapatra wrote:
>> >
>> >> +const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps)
>> >> +{
>> >> + int i;
>> >> + static const struct {
>> >> + int Mmin;
>> >> + int Mmax;
>> >> + const struct dispc_coef *coef_3;
>> >> + const struct dispc_coef *coef_5;
>> >> + } coefs[] = {
>> >> + { 26, 32, coef3_M32, coef5_M32 },
>> >> + { 22, 26, coef3_M26, coef5_M26 },
>> >> + { 19, 22, coef3_M22, coef5_M22 },
>> >> + { 16, 19, coef3_M19, coef5_M19 },
>> >> + { 14, 16, coef3_M16, coef5_M16 },
>> >> + { 13, 14, coef3_M14, coef5_M14 },
>> >> + { 12, 13, coef3_M13, coef5_M13 },
>> >> + { 11, 12, coef3_M12, coef5_M12 },
>> >> + { 10, 11, coef3_M11, coef5_M11 },
>> >> + { 9, 10, coef3_M10, coef5_M10 },
>> >> + { 8, 9, coef3_M9, coef5_M9 },
>> >> + { 3, 8, coef3_M8, coef5_M8 },
>> >> + /*
>> >> + * When upscaling more than two times, blockiness and outlines
>> >> + * around the image are observed when M8 tables are used. M11,
>> >> + * M16 and M19 tables are used to prevent this.
>> >> + */
>> >> + { 2, 3, coef3_M11, coef5_M11 },
>> >> + { 1, 2, coef3_M16, coef5_M16 },
>> >> + };
>> >> +
>> >> + inc /= 128;
>> >> + for (i = 0; i < ARRAY_LEN(coefs); ++i)
>> >> + if (inc > coefs[i].Mmin && inc <= coefs[i].Mmax)
>> >> + return five_taps ? coefs[i].coef_5 : coefs[i].coef_3;
>> >> + if (inc == 1)
>> >> + return five_taps ? coef3_M19 : coef5_M19;
>> >> + return NULL;
>> >> +}
>> >
>> > Why don't you handle the inc == 1 case the same as others? Just have an
>> > entry in the table for Mmin=0, Mmax = 1.
>> >
>> For inc=1 i.e. M=1 , scaling ratio is maximum as L/M=8. DISPC scaler
>> doesnot support upscaling more than 8 itmes. Having an (Mmin,Mmax] of
>> (0,1] will allow such cases.
>
> I don't think I understand. A table entry for 0,1 would match exactly
> one inc value, which is 1. Which is the same as you do with the separate
> if statement now.
>
yes, you are right.
>> > Also, I think it's a bit confusing that Mmin is exclusive and Mmax is
>> > inclusive in the comparison. It makes the table a bit hard to read, when
>> > looking at which entry is used for which inc. I'd recommend using
>> > inclusive comparison for both.
>> >
>> > Tomi
>> >
>> Having both inclusive will allow us to delete the extra comparison for
>> inc==1 but in my opinion having Mmin exclusive and Mmax inclusive
>> actually gives an clear idea of comparison. The tables mostly go by
>> the Mmax value.
>> For example, for inc=26 coef3/5_M26 table is selected, for inc=22
>> coef3/5_M22 is selected etc.
>> If we have both Mmin and Mmax as inclusive above case becomes slightly
>> incoherent. Say for M=26 instead of coef3/5_M26 which seems more
>> obvious choice coef3/5_M32 is selected.
>
> I don't understand this either... If you now have:
>
> { 26, 32, coef3_M32, coef5_M32 },
> { 22, 26, coef3_M26, coef5_M26 },
>
> It would be changed to
>
> { 27, 32, coef3_M32, coef5_M32 },
> { 23, 26, coef3_M26, coef5_M26 },
>
> and it would match the same inc values as before after changing the Mmin
> comparison to >=.
>
yes, I had mistakenly overlooked it.
>> For both inclusive cases to work and avoid confusion and delete extra
>> comparison for inc==1 , I have to reverse the order of table entries
>> in "coef" table. But for that I will have to put the "When upscaling
>> more than two times, blockiness and outlines" comment at the beginning
>> of the table and then start with { 1, 2, coef3_M16, coef5_M16 }.
>> This will create even more confusion.
>
> The ranges for the table elements are exclusive. The order doesn't
> matter because one inc value can only match one table entry. So I have
> to say I don't understand this comment either =). Am I missing
> something?
>
> Tomi
>
I mistakenly thought inc to be float which created all this confusion.
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
