[FFmpeg-devel] [PATCH] Musepack SV8 demuxer and decoder

[Michael Niedermayer]

Hi

On Sat, Nov 03, 2007 at 09:10:50PM +0200, Kostya wrote:
> On Sat, Nov 03, 2007 at 03:51:15PM +0100, Michael Niedermayer wrote:
> > Hi
> > 
> > On Sat, Nov 03, 2007 at 12:42:22PM +0200, Kostya wrote:
> > > On Sat, Nov 03, 2007 at 04:19:01AM +0100, Michael Niedermayer wrote:
> > > > Hi
> > > > 
> > > > On Fri, Nov 02, 2007 at 09:58:59AM +0200, Kostya wrote:
> > > > > Here is $subj. I will try to provide samples soon.
> > > 
> > > samples are at ftp://upload.mplayerhq.hu/incoming/mpc8_samples.tar (partially uploaded)
> > > 
> > >  
> > > > [...]
> > > > 
> > > > > +	code = get_bits(gb, mpc8_cnk_len[k-1][n-1] - 1);
> > > > > +
> > > > > +	if (code >= mpc8_cnk_lost[k-1][n-1])
> > > > > +		code = ((code << 1) | get_bits1(gb)) - mpc8_cnk_lost[k-1][n-1];
> > > > 
> > > > duplicate
> > > 
> > > what is duplicated? 
> > 
> > +    v = get_bits(gb, mpc8_log2[m]);
> > +    if(v >= mpc8_log2_lost[m])
> > +        v = (v << 1) + get_bits1(gb) - mpc8_log2_lost[m];
> > [...]
> > +       code = get_bits(gb, mpc8_cnk_len[k-1][n-1] - 1);
> > +
> > +       if (code >= mpc8_cnk_lost[k-1][n-1])
> > +               code = ((code << 1) | get_bits1(gb)) - mpc8_cnk_lost[k-1][n-1];
> 
> Factored out and dropped mpc8_log2* tables 
>  
> > [...]
> > > > [...]
> > > > > +/* shamelessly stolen from nutdec.c */
> > > > > +static uint64_t get_v(ByteIOContext *bc){
> > > > 
> > > > code duplication
> > > 
> > > I will prepare a patch to move it to avio.[ch]. Should it be
> > > left as inline or prototype + function body?
> > 
> > id say prototype + function body?
> 
> It uses ff_get_v 
[...]

> +    int code;
> +    const uint32_t * C = mpc8_cnk[k-1];
> +
> +    code = mpc8_dec_base(gb, k, n);

int code = mpc8_dec_base(gb, k, n);


[...]
> +    if(keyframe){
> +        c->buf_size = buf_size;
> +        c->bits = av_realloc(c->bits, buf_size + FF_INPUT_BUFFER_PADDING_SIZE);
> +        memcpy(c->bits, buf, buf_size);
> +        init_get_bits(gb, c->bits, buf_size * 8);
> +        memset(c->Q, 0, sizeof(c->Q));
> +    c->last_bits_used = 0;
> +    }

please remove this realloc() and memcpy() mess and decode the individual
frames which your decoder receives from the parser


[...]
> +                    if(cnt && cnt < SAMPLES_PER_BAND/2)
> +                        t = mpc8_dec_enum(gb, FFMIN(cnt, 18-cnt), 18);
> +                    if(cnt > 9) t = ~t;

the code surrounding mpc8_dec_enum() can be factorized, it occurs several
times


[...]
> +                    if(bands[i].res[ch] != 9){

res != 9


[...]
> +static const int mpc8_rate[4] = { 44100, 48000, 37800, 32000 };

too small, index can be up to 7


[...]
> +    int64_t silence;

unused


[...]
> +    av_set_pts_info(st, 32, 1152 * (1 << (st->codec->extradata[1]&3)*2), st->codec->sample_rate);
> +    st->duration = c->samples / (1152 * (1 << (st->codec->extradata[1]&3)*2));

(1152 << (st->codec->extradata[1]&3)*2)


> +    size -= url_ftell(pb) - pos;
> +    
> +    return 0;

useless statement befoe return


[...]
> +AVInputFormat mpc8_demuxer = {
> +    "mpc8",
> +    "musepack8",
> +    sizeof(MPCContext),
> +    mpc8_probe,
> +    mpc8_read_header,
> +    mpc8_read_packet,
> +    NULL,
> +    mpc8_read_seek,
> +    .extensions = "mpc",
> +};

if you have a probe function. then you dont really need extensions


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071104/11f9826e/attachment.pgp>