Re: [FFmpeg-devel] [PATCH] MULTI VLC decoding boost

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2864 bytes --]

On Mon, Aug 28, 2023 at 07:36:17PM +0200, Paul B Mahol wrote:
> Patches attached.
> 
> Thanks for kurosu for pointing unmerged branches.
> 


[...]

> +static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
> +		      const int num, const int numbits,
> +                      const VLCcode *buf,
> +                      uint32_t curcode, int curlen,
> +                      int curlimit, int curlevel,
> +                      const int minlen, const int max,
> +                      unsigned* levelcnt, VLC_MULTI_ELEM *info)
> +{

> +    if (nb_elems > 256 && curlevel > 2)
> +        return; // No room

this and


> +    for (int i = num-1; i > max; i--) {
> +        for (int j = 0; j < 2; j++) {
> +            int newlimit, sym;
> +            int t = j ? i-1 : i;
> +            int l = buf[t].bits;
> +            uint32_t code;
> +
> +            sym = buf[t].symbol;
> +            if (l > curlimit)
> +                return;
> +            code = curcode + (buf[t].code >> curlen);
> +            newlimit = curlimit - l;
> +            l  += curlen;
> +            if (nb_elems>256) AV_WN16(info->val+2*curlevel, sym);
> +            else info->val[curlevel] = sym&0xFF;
> +
> +            if (curlevel) { // let's not add single entries
> +                uint32_t val = code >> (32 - numbits);
> +                uint32_t  nb = val + (1U << (numbits - l));
> +                info->len = l;
> +                info->num = curlevel+1;
> +                for (; val < nb; val++)
> +                    AV_COPY64(table+val, info);
> +                levelcnt[curlevel-1]++;
> +            }
> +

> +            if (curlevel+1 < VLC_MULTI_MAX_SYMBOLS && newlimit >= minlen) {

this are 2 checks doing the same thing for 8 and 16 bit
what mess is this ?
for 8bit we have VLC_MULTI_MAX_SYMBOLS space (6) in the array so we skip beyond that
for 16bit we have VLC_MULTI_MAX_SYMBOLS/2 space which is 3 and the skip instead
is inside add_level() above with hardcoded litteral number
(nb_elems > 256 is a check for if its 8 or 16bit)

why is such totally hacked up code pushed with standing objections and no
review ?

yes, ill fix this one but i have the feeling this code has more surprises


> +                add_level(table, nb_elems, num, numbits, buf,
> +                          code, l, newlimit, curlevel+1,
> +                          minlen, max, levelcnt, info);
> +            }
> +        }
> +    }
> +}

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".