[FFmpeg-devel] [PATCH] Added integer 32 bits support to wavpack

[Kostya]

On Fri, May 01, 2009 at 01:49:37PM +0200, Laurent Aimar wrote:
>  The attached patch completes the support for files with INT32INFO chunk
> with non zero sent_bits.
> 
> -- 
> fenrir
 


> diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
> index f31402d..0aded35 100644
> --- a/libavcodec/wavpack.c
> +++ b/libavcodec/wavpack.c
> @@ -57,7 +57,7 @@ enum WP_ID{
>      WP_ID_INT32INFO,
>      WP_ID_DATA,
>      WP_ID_CORR,
> -    WP_ID_FLT,
> +    WP_ID_EXTRA,

Well, WavPack documentation says it's "special extended bitstream for
floating point data or integers > 24 bit". While old name was not
entirely correct, new one is even more ambiguous. Please try to give it
a better name than mere "extra".

>      WP_ID_CHANINFO
>  };
>  
> @@ -85,11 +85,15 @@ typedef struct WavpackContext {
>      int joint;
>      uint32_t CRC;
>      GetBitContext gb;
> +    int got_extra;
> +    uint32_t crc_extra;

Again, maybe got_wvx and crc_wvx instead? 

> +    GetBitContext gb_extra;
>      int data_size; // in bits
>      int samples;
>      int terms;
>      Decorr decorr[MAX_TERMS];
>      int zero, one, zeroes;
> +    int extra_bits;
>      int and, or, shift;
>      int post_shift;
>      int hybrid, hybrid_bitrate;
> @@ -344,6 +348,7 @@ static inline int wv_unpack_stereo(WavpackContext *s, GetBitContext *gb, void *d
>      int A, B, L, L2, R, R2, bit;
>      int pos = 0;
>      uint32_t crc = 0xFFFFFFFF;
> +    uint32_t crc_extra = 0xFFFFFFFF;
>      int16_t *dst16 = dst;
>      int32_t *dst32 = dst;
>  
> @@ -424,6 +429,18 @@ static inline int wv_unpack_stereo(WavpackContext *s, GetBitContext *gb, void *d
>          if(s->joint)
>              L += (R -= (L >> 1));
>          crc = (crc * 3 + L) * 3 + R;
> +        if(s->extra_bits){
> +            L <<= s->extra_bits;
> +            R <<= s->extra_bits;
> +
> +            if(s->got_extra){
> +                L |= get_bits(&s->gb_extra, s->extra_bits);
> +                crc_extra = crc_extra * 9 + (L&0xffff) * 3 + ((L>>16)&0xffff);
> +
> +                R |= get_bits(&s->gb_extra, s->extra_bits);
> +                crc_extra = crc_extra * 9 + (R&0xffff) * 3 + ((R>>16)&0xffff);

+ ((unsigned)R >> 16), I think (and Michael has pointed that out already).

> +            }
> +        }
>          bit = (L & s->and) | s->or;
>          if(hires)
>              *dst32++ = (((L + bit) << s->shift) - bit) << s->post_shift;
> @@ -441,6 +458,10 @@ static inline int wv_unpack_stereo(WavpackContext *s, GetBitContext *gb, void *d
>          av_log(s->avctx, AV_LOG_ERROR, "CRC error\n");
>          return -1;
>      }
> +    if(s->got_extra && crc_extra != s->crc_extra){
> +        av_log(s->avctx, AV_LOG_ERROR, "CRC extra error\n");
> +        return -1;
> +    }

I read it as not very good way to say "An additional CRC error" and
suspect it should mean something else.

[monaural decoding case skipped]
> @@ -533,7 +568,9 @@ static int wavpack_decode_frame(AVCodecContext *avctx,
>  
>      memset(s->decorr, 0, MAX_TERMS * sizeof(Decorr));
>      memset(s->ch, 0, sizeof(s->ch));
> +    s->extra_bits = 0;
>      s->and = s->or = s->shift = 0;
> +    s->got_extra = 0;
>  
>      s->samples = AV_RL32(buf); buf += 4;
>      if(!s->samples){
> @@ -701,7 +738,7 @@ static int wavpack_decode_frame(AVCodecContext *avctx,
>                  continue;
>              }
>              if(buf[0])
> -                s->post_shift = buf[0];
> +                s->extra_bits = buf[0];
>              else if(buf[1])
>                  s->shift = buf[1];
>              else if(buf[2]){
> @@ -719,6 +756,12 @@ static int wavpack_decode_frame(AVCodecContext *avctx,
>              buf += size;
>              got_bs = 1;
>              break;
> +        case WP_ID_EXTRA:
> +            init_get_bits(&s->gb_extra, buf, size * 8);
> +            s->crc_extra = get_bits_long( &s->gb_extra, 32);

I'd use s->crc_extra = AV_RL32(buf) and open bit buffer after that but this
looks fine too. Oh, and please add block size check.

> +            buf += size;
> +            s->got_extra = 1;
> +            break;
>          default:
>              buf += size;
>          }