Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements

["Tomas Härdin" <git@haerdin.se>]

>             if (c) {
>                 e[0] = 1 << 14;
>                 e[1] = 0 << 14;
>                 e[2] = v[1];
>                 e[3] = v[0];
>             } else {
>                 e[0] = v[0];
>                 e[1] = v[1];
>                 e[2] = 0 << 14;
>                 e[3] = 1 << 14;
>             }
> 
>             if (invert2x2(e, d)) {
>                 sum = UINT64_MAX;
>                 goto next;
>             }
> 

You can make use of the properties of e to simplify calculating the
inverse. The determinant is always v[0]<<14, so you can just do if
(!v[0]) continue; and skip the determinant check altogether.

>                 if (d[i] != av_clip_intp2(d[i], 15)) {

d[i] < INT16_MIN || d[i] > INT16_MAX is more clear and probably faster

> +                    lt = ((lm * e[0]) >> 14) + ((rm * e[1]) >> 14);
> +                    rt = ((lm * e[2]) >> 14) + ((rm * e[3]) >> 14);

Result is implementation-defined. Use division by (1<<14). Also add
then divide. The intermediate result is 49 bits so fits easily in 64
bits.

You could also simplify this calculation by again making use of the
properties of e.

>                     if (c)
>                         v += FFABS(rt);
>                     else
>                         v += FFABS(lt);
>                     sum += v;
>                     if (sum > best_sum)
>                         goto next;

Seems like this reduces to solving a linear program.

>                     if ((((lt * d[0]) >> 14) + ((rt * d[1]) >> 14))
> != lm) {
>                         sum = UINT64_MAX;
>                         goto next;
>                     }
> 
>                     if ((((lt * d[2]) >> 14) + ((rt * d[3]) >> 14))
> != rm) {
>                         sum = UINT64_MAX;
>                         goto next;
>                     }

Looks like a massive hack. I'd prefer to formally verify that the
arithmetic works out. Also again you can make use of the properties of
e, or inv(e) as it were.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".