[FFmpeg-devel] rmdec.c: add SIPR codec try #2

[Kostya]

On Wed, Mar 18, 2009 at 04:25:20PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> I'm gonna keep trying, just so I learn to not be an idiot. If you want
> me to stop, just say so and I'll commit that first version.
> 
> On Tue, Mar 17, 2009 at 11:57 PM, Kostya <kostya.shishkov at gmail.com> wrote:
> > On Tue, Mar 17, 2009 at 05:34:01PM -0400, Ronald S. Bultje wrote:
> >> + ? ? ? ?/* swap 4bit-nibbles of block 'i' with 'o' */
> >> + ? ? ? ?if (!((i ^ o) & 1)) LOOP_ITER {
> >> + ? ? ? ? ? ?buf[i >> 1] = (buf[i >> 1] & mask2) | (buf[o >> 1] & mask1);
> >> + ? ? ? ? ? ?buf[o >> 1] = (buf[o >> 1] & mask2) | tmp;
> >> + ? ? ? ?} LOOP_END else LOOP_ITER {
> >> + ? ? ? ? ? ?int shift1 = (i & 1) ? 4 : 0, shift2 = (i & 1) ? 0 : 4;
> >> + ? ? ? ? ? ?buf[i >> 1] = (buf[i >> 1] & mask2) |
> >> + ? ? ? ? ? ? ? ?(((buf[o >> 1] & mask2) << shift1) >> shift2);
> >> + ? ? ? ? ? ?buf[o >> 1] = (buf[o >> 1] & mask1) | ((tmp << shift2) >> shift1);
> >> + ? ? ? ?} LOOP_END;
> >
> > No, that still looks like some suspicious condition.
> > You'd better write it without macroses.
> 
> OK, I rewrote it without the (slow) loops. I tried to unroll the loops
> in such a way that as little work as possible is done within the
> loops. Please let me know if I'm showing improvements. :-).
> 
> Ronald

[...]
> @@ -637,6 +661,65 @@
>      }
>  }
>  
> +/** perform 4-bit block reordering for SIPR data */
> +static void
> +rm_reorder_sipr_data (RMStream *ast)
> +{
> +    int n, bs = ast->sub_packet_h * ast->audio_framesize * 2 / 96; // nibbles per subpacket
> +
> +    for (n = 0; n < 38; n++) {
> +        int j;
> +        int i = bs * sipr_swaps[n][0];
> +        int o = bs * sipr_swaps[n][1];
> +        uint8_t *buf = ast->pkt.data;
> +
> +        /* swap 4bit-nibbles of block 'i' with 'o' */
> +        if (!((i ^ o) & 1)) {
> +            int bytes = (bs - (i & 1)) >> 1;
> +            char t[bytes];
> +            if ((i ^ bs) & 1) { /* last 4bits, if unaligned */
> +                int tmp = buf[(i + bs - 1) >> 1] & 0x0F;
> +                buf[(i + bs - 1) >> 1] = (buf[i >> 1] & 0xF0) |
> +                    (buf[(o + bs - 1) >> 1] & 0x0F);
> +                buf[o >> 1] = (buf[(o + bs - 1) >> 1] & 0xF0) | tmp;
> +            }
> +            if (i & 1) { /* first 4 bits, if unaligned */
> +                int tmp = buf[i >> 1] & 0xF0;
> +                buf[i >> 1] = (buf[i >> 1] & 0x0F) | (buf[o >> 1] & 0xF0);
> +                buf[o >> 1] = (buf[o >> 1] & 0x0F) | tmp;
> +                i++; o++;
> +            }
> +            memcpy(&t,           &buf[i >> 1], bytes);
> +            memcpy(&buf[i >> 1], &buf[o >> 1], bytes);
> +            memcpy(&buf[o >> 1], &t,           bytes);
> +        } else {
> +            int mask1, mask2, shift1, shift2, tmp;
> +
> +            mask1  = (i & 1) ? 0xF0 : 0x0F;
> +            mask2  = (i & 1) ? 0x0F : 0xF0;
> +            shift1 = (i & 1) ?    4 :    0;
> +            shift2 = (i & 1) ?    0 :    4;
> +            for (j = 0; j < bs; j+=2, i+=2, o+=2) {
> +                tmp = buf[i >> 1] & mask1;
> +                buf[i >> 1] = (buf[i >> 1] & mask2) |
> +                    (((buf[o >> 1] & mask2) << shift1) >> shift2);
> +                buf[o >> 1] = (buf[o >> 1] & mask1) |
> +                    ((tmp << shift2) >> shift1);
> +            }
> +            i -= j - 1; o-= j - 1;
> +            tmp = mask1;  mask1 = mask2;   mask2 = tmp;
> +            tmp = shift1; shift1 = shift2; shift2 = tmp;
> +            for (j = 1; j < bs; j+=2, i+=2, o+=2) {
> +                tmp = buf[i >> 1] & mask1;
> +                buf[i >> 1] = (buf[i >> 1] & mask2) |
> +                    (((buf[o >> 1] & mask2) << shift1) >> shift2);
> +                buf[o >> 1] = (buf[o >> 1] & mask1) |
> +                    ((tmp << shift2) >> shift1);
> +            }
> +        }
> +    }
> +}
> +
>  int
>  ff_rm_parse_packet (AVFormatContext *s, ByteIOContext *pb,
>                      AVStream *st, RMStream *ast, int len, AVPacket *pkt,

Well, you are definitely showing progress. I can hardly understand what
this code does.

Please replace it with the first version (one loop) and commit.
I fear it would take long to improve it while the rest is fine.