[FFmpeg-devel] [PATCH] wmapro decoder

Vitor Sessak vitor1001
Sat Jun 20 20:09:41 CEST 2009


Sascha Sommer wrote:
> Hi Michael,
> 
> thanks for the review. See my answers below.

[... review omitted ...]

> 
> Fixed.
> Besides that I also fixed following points from Vitors mails in the same 
> thread:
> - set avctx->sample_fmt
> - use const matrix offset table
> - get rid of wma3.h
> - removed encoding X in general.texi
> - removed prefix from static functions and tables
> 
> The code still outputs shorts. Float produced clicks all around the place. I'm 
> not sure yet if the problem is in my code or in ffmpeg.

It looks like it works for me, see attached.

> Float output can also be added after the code has been added to svn imho.
> 
> New patch attached.


> +/**
> + * @brief main decoder context
> + */

The @brief here makes the plain C code harder to read without improving 
a lot doxy output, so it is better to remove it (happens a lot elsewhere).

> +    /** generic decoder variables */
> +    AVCodecContext*  avctx;                         ///< codec context for av_log
> +    DSPContext       dsp;                           ///< accelerated dsp functions
> +    uint8_t          frame_data[MAX_FRAMESIZE +
> +                      FF_INPUT_BUFFER_PADDING_SIZE];///< compressed frame data
> +    MDCTContext      mdct_ctx[WMAPRO_BLOCK_SIZES];  ///< MDCT context per block size
> +    DECLARE_ALIGNED_16(float, tmp[WMAPRO_BLOCK_MAX_SIZE]); ///< imdct output buffer
> +    float*           windows[WMAPRO_BLOCK_SIZES];   ///< window per block size
> +    int              coef_max[2];                   ///< max length of vlc codes

The "/** generic decoder variables */" will be misinterpreted by doxygen 
as been only referent to "AVCodecContext*  avctx;". There is a doxygen 
syntax for grouping.

> +/**
> + *@brief Initialize the decoder.
> + *@param avctx codec context
> + *@return 0 on success, -1 otherwise
> + */
> +static av_cold int decode_init(AVCodecContext *avctx)
> +{
> +    WMA3DecodeContext *s = avctx->priv_data;
> +    uint8_t *edata_ptr = avctx->extradata;
> +    int16_t* sfb_offsets;
> +    unsigned int channel_mask;
> +    int i;
> +    int log2_num_subframes;
> +
> +    s->avctx = avctx;
> +    dsputil_init(&s->dsp, avctx);
> +
> +    avctx->sample_fmt = SAMPLE_FMT_S16;
> +
> +    /** FIXME: is this really the right thing to do for 24 bits? */
> +    s->sample_bit_depth = 16; // avctx->bits_per_sample;
> +    if (avctx->extradata_size >= 18) {
> +        s->decode_flags     = AV_RL16(edata_ptr+14);
> +        channel_mask    = AV_RL32(edata_ptr+2);
> +//        s->sample_bit_depth = AV_RL16(edata_ptr);
> +#ifdef DEBUG
> +        /** dump the extradata */
> +        for (i=0 ; i<avctx->extradata_size ; i++)
> +            av_log(avctx, AV_LOG_DEBUG, "[%x] ",avctx->extradata[i]);
> +        av_log(avctx, AV_LOG_DEBUG, "\n");
> +#endif
> +
> +    } else {
> +        ff_log_ask_for_sample(avctx, "Unknown extradata size\n");
> +        return -1;
> +    }
> +
> +    /** generic init */
> +    s->log2_frame_size = av_log2(avctx->block_align*8)+1;
> +
> +    /** frame info */
> +    s->skip_frame = 1; /** skip first frame */
> +    s->packet_loss = 1;
> +    s->len_prefix = (s->decode_flags & 0x40) >> 6;
> +
> +    if (!s->len_prefix) {
> +         ff_log_ask_for_sample(avctx, "no length prefix\n");
> +         return -1;
> +    }
> +
> +    /** get frame len */
> +    s->samples_per_frame = 1 << ff_wma_get_frame_len_bits(avctx->sample_rate,
> +                                                          3, s->decode_flags);
> +
> +    /** init previous block len */
> +    for (i=0;i<avctx->channels;i++)
> +        s->channel[i].prev_block_len = s->samples_per_frame;
> +
> +    /** subframe info */
> +    log2_num_subframes = ((s->decode_flags & 0x38) >> 3);
> +    s->max_num_subframes = 1 << log2_num_subframes;
> +    s->num_possible_block_sizes = log2_num_subframes + 1;
> +    s->min_samples_per_subframe = s->samples_per_frame / s->max_num_subframes;
> +    s->dynamic_range_compression = (s->decode_flags & 0x80) >> 7;
> +
> +    if (s->max_num_subframes > MAX_SUBFRAMES) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid number of subframes %i\n",
> +                      s->max_num_subframes);
> +        return -1;
> +    }
> +
> +    s->num_channels = avctx->channels;
> +
> +    /** extract lfe channel position */
> +    s->lfe_channel = -1;
> +
> +    if (channel_mask & 8) {
> +        unsigned int mask;
> +        for (mask=1;mask < 16;mask <<= 1) {
> +            if (channel_mask & mask)
> +                ++s->lfe_channel;
> +        }
> +    }
> +
> +    if (s->num_channels < 0 || s->num_channels > WMAPRO_MAX_CHANNELS) {
> +        ff_log_ask_for_sample(avctx, "invalid number of channels\n");
> +        return -1;
> +    }
> +
> +    INIT_VLC_STATIC(&sf_vlc, SCALEVLCBITS, HUFF_SCALE_SIZE,
> +                 scale_huffbits, 1, 1,
> +                 scale_huffcodes, 2, 2, 616);
> +
> +    INIT_VLC_STATIC(&sf_rl_vlc, VLCBITS, HUFF_SCALE_RL_SIZE,
> +                 scale_rl_huffbits, 1, 1,
> +                 scale_rl_huffcodes, 4, 4, 1406);
> +
> +    INIT_VLC_STATIC(&coef_vlc[0], VLCBITS, HUFF_COEF0_SIZE,
> +                 coef0_huffbits, 1, 1,
> +                 coef0_huffcodes, 4, 4, 2108);
> +
> +    s->coef_max[0] = ((HUFF_COEF0_MAXBITS+VLCBITS-1)/VLCBITS);
> +
> +    INIT_VLC_STATIC(&coef_vlc[1], VLCBITS, HUFF_COEF1_SIZE,
> +                 coef1_huffbits, 1, 1,
> +                 coef1_huffcodes, 4, 4, 3912);
> +
> +    s->coef_max[1] = ((HUFF_COEF1_MAXBITS+VLCBITS-1)/VLCBITS);
> +
> +    INIT_VLC_STATIC(&vec4_vlc, VLCBITS, HUFF_VEC4_SIZE,
> +                 vec4_huffbits, 1, 1,
> +                 vec4_huffcodes, 2, 2, 604);
> +
> +    INIT_VLC_STATIC(&vec2_vlc, VLCBITS, HUFF_VEC2_SIZE,
> +                 vec2_huffbits, 1, 1,
> +                 vec2_huffcodes, 2, 2, 562);
> +
> +    INIT_VLC_STATIC(&vec1_vlc, VLCBITS, HUFF_VEC1_SIZE,
> +                 vec1_huffbits, 1, 1,
> +                 vec1_huffcodes, 2, 2, 562);
> +
> +    s->num_sfb = av_mallocz(sizeof(int8_t)*s->num_possible_block_sizes);
> +    s->sfb_offsets = av_mallocz(MAX_BANDS *
> +                                sizeof(int16_t) * s->num_possible_block_sizes);
> +    s->subwoofer_cutoffs = av_mallocz(sizeof(int16_t) *
> +                                      s->num_possible_block_sizes);
> +    s->sf_offsets = av_mallocz(MAX_BANDS * s->num_possible_block_sizes *
> +                               s->num_possible_block_sizes * sizeof(int16_t));

There is no necessity of clearing s->num_sfb, it will be filled up in 
the next few lines (is clearing the others needed?), so you can use 
plain av_malloc().

> +/**
> + *@brief Calculate a decorrelation matrix from the bitstream parameters.
> + *@param s codec context
> + *@param chgroup channel group for which the matrix needs to be calculated
> + */
> +static void decode_decorrelation_matrix(WMA3DecodeContext* s,
> +                                            WMA3ChannelGroup* chgroup)
> +{
> +    int i;
> +    int offset = 0;
> +    char rotation_offset[WMAPRO_MAX_CHANNELS * WMAPRO_MAX_CHANNELS];
> +    memset(chgroup->decorrelation_matrix,0,
> +           sizeof(float) *s->num_channels * s->num_channels);
> +
> +    for (i=0;i<chgroup->num_channels  * (chgroup->num_channels - 1) >> 1;i++)
> +        rotation_offset[i] = get_bits(&s->gb,6);
> +
> +    for (i=0;i<chgroup->num_channels;i++) {
> +        if (get_bits1(&s->gb)) {
> +            chgroup->decorrelation_matrix[chgroup->num_channels * i + i]=  1.0;
> +        } else
> +            chgroup->decorrelation_matrix[chgroup->num_channels * i + i]= -1.0;

chgroup->decorrelation_matrix[chgroup->num_channels * i + i] =
                                       -1 + get_bits1(&s->gb)<<1;

or

chgroup->decorrelation_matrix[chgroup->num_channels * i + i] = 

                                    get_bits1(&s->gb) ? 1.0, -1.0;


> +                    if (s->num_channels == 2) {
> +                        chgroup->transform = 1;
> +                    } else {
> +                        chgroup->transform = 2;
> +                        /** cos(pi/4) */
> +                        chgroup->decorrelation_matrix[0] = 0.70703125;
> +                        chgroup->decorrelation_matrix[1] = -0.70703125;
> +                        chgroup->decorrelation_matrix[2] = 0.70703125;
> +                        chgroup->decorrelation_matrix[3] = 0.70703125;

Already mentioned...

> +
> +    if (len <= 0 || buflen > MAX_FRAMESIZE) {
> +         ff_log_ask_for_sample(s->avctx, "input buffer to small\n");

s/to small/too small/

Note also that the sample at 
http://samples.mplayerhq.hu/A-codecs/WMA9/wmapro/New%20Stories%20(Highway%20Blues)-2.wma 
trigger this error.

> +    for (i=0;i<s->num_channels;i++) {
> +        s->channel[i].decoded_samples = 0;
> +        s->channel[i].cur_subframe = 0;
> +        s->channel[i].reuse_sf = 0;
> +    }

memset(...)

-Vitor

-------------- next part --------------
A non-text attachment was scrubbed...
Name: wma_use_float.diff
Type: text/x-diff
Size: 2012 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090620/0fac5e56/attachment.diff>



More information about the ffmpeg-devel mailing list