[FFmpeg-devel] [PATCH] wmapro decoder
Sascha Sommer
saschasommer
Mon Aug 31 21:55:25 CEST 2009
Hi,
> >
> > + uint32_t frame_num; ///< current frame
> > number
>
> unused
>
It is used for error output and some debugging messages. Does it waste too
many resources?
> > +/**
> > + *@brief Decode the subframe length.
> > + *@param s context
> > + *@param offset sample offset in the frame
> > + *@return decoded subframe length on success, 0 in case of an error
> > + */
> > +static int decode_subframe_length(WMA3DecodeContext *s, int offset)
> > +{
> > + int log2_subframe_len = 0;
> > + int subframe_len;
> > +
> > + /** no need to read from the bitstream when only one length is
> > possible */ + if (offset == s->samples_per_frame -
> > s->min_samples_per_subframe) + return s->min_samples_per_subframe;
> > +
> > + /** 1 bit indicates if the subframe is of maximum length */
> > + if (s->max_subframe_len_bit) {
> > + if (get_bits1(&s->gb))
> > + log2_subframe_len = 1 + get_bits(&s->gb,
> > s->subframe_len_bits-1); + } else
> > + log2_subframe_len = get_bits(&s->gb, s->subframe_len_bits);
> > +
> >
> > + subframe_len = s->samples_per_frame / (1 << log2_subframe_len);
>
> subframe_len = s->samples_per_frame >> log2_subframe_len;
>
> and i would guess the names are not good because one would expect
> 1<<log2_X = X
>
Fixed.
> > +
> > + /** sanity check the length */
> > + if (subframe_len < s->min_samples_per_subframe
> > + || subframe_len > s->samples_per_frame) {
> > + av_log(s->avctx, AV_LOG_ERROR, "broken frame: subframe_len
> > %i\n", + subframe_len);
> > + return 0;
>
> within ffmpeg errors are negative values
>
Changed.
> > +static int decode_tilehdr(WMA3DecodeContext *s)
> > +{
> >
> > + uint16_t num_samples[WMAPRO_MAX_CHANNELS];
> > + uint8_t contains_subframe[WMAPRO_MAX_CHANNELS];
> > + int channels_for_cur_subframe = s->num_channels;
> > + int fixed_channel_layout = 0;
> > + int min_channel_len = 0;
>
> these could benefit from some doxy
>
Added.
> >
> > + /** save transmitted scale factors so that they can be
> > reused for + the next subframe */
> > + memcpy(s->channel[c].saved_scale_factors,
> > + s->channel[c].scale_factors, s->num_bands *
> > + sizeof(*s->channel[c].saved_scale_factors));
>
> what happens with s->channel[c].scale_factors so that it cant just be used?
>
The saved scale factors might be resampled for several subframes before some
new values are transmitted and saved_scale_factors is updated.
If the resampled scale factors would be used for the resampling, the output
would change. Therefore two buffers are needed.
> > + memset(s->channel[c].coeffs, 0,
> > + sizeof(*s->channel[c].coeffs) * subframe_len);
>
> IMHO it would be better to do this in a else clause of the
> if(transmit_coefs) and before the rle decoding is started
>
Changed then.
> > + /** interleave samples and write them to the output buffer */
> > + for (i = 0; i < s->num_channels; i++) {
> > + float* ptr;
> > + int incr = s->num_channels;
> > + float* iptr = s->channel[i].out;
> > + int x;
> > +
> > + ptr = s->samples + i;
> > +
> > + for (x = 0; x < s->samples_per_frame; x++) {
> > + *ptr = av_clipf(*iptr++, -1.0, 32767.0 / 32768.0);
> > + ptr += incr;
> > + }
> > +
> > + /** reuse second half of the IMDCT output for the next frame */
> > + memcpy(&s->channel[i].out[0],
> > + &s->channel[i].out[s->samples_per_frame],
> > + s->samples_per_frame * sizeof(*s->channel[i].out) >> 1);
> > + }
>
> did you try to not do several passes over the output buffer?
> (maybe its faster ...)
>
This takes twice as long.
> > +/**
> > + *@brief WMA9 decoder
> > + */
> > +AVCodec wmapro_decoder = {
> > + "wmapro",
> > + CODEC_TYPE_AUDIO,
> > + CODEC_ID_WMAPRO,
> > + sizeof(WMA3DecodeContext),
>
> WMA9 decoder, WMAPRO, WMA3DecodeContext
> isnt that naming a little inconsistent?
>
Everything renamed to wmapro.
New patch attached...
Regards
Sascha
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wmapro_try7.patch
Type: text/x-diff
Size: 55209 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090831/f4d647c8/attachment.patch>
More information about the ffmpeg-devel
mailing list