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

[Kostya]

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? 
 
> > +
> > +	do {
> > +		n--;
> > +		if (code >= C[n]) {
> > +			bits |= 1 << n;
> > +			code -= C[n];
> > +			C -= 32;
> > +			k--;
> > +		}
> > +	} while(k > 0);
> 
> interresting way to store bits

It's not my invention 
 
> [...]
> > +        bands[maxband - 1].res[0] = get_vlc2(gb, res_vlc[0].table, MPC8_RES_BITS, 2);
> > +        bands[maxband - 1].res[1] = get_vlc2(gb, res_vlc[0].table, MPC8_RES_BITS, 2);
> > +        if(bands[maxband - 1].res[0] > 15) bands[maxband - 1].res[0] -= 17;
> > +        if(bands[maxband - 1].res[1] > 15) bands[maxband - 1].res[1] -= 17;
> > +        for(i = maxband - 2; i >= 0; i--){
> > +            bands[i].res[0] = get_vlc2(gb, res_vlc[bands[i+1].res[0] > 2].table, MPC8_RES_BITS, 2) + bands[i + 1].res[0];
> > +            bands[i].res[1] = get_vlc2(gb, res_vlc[bands[i+1].res[1] > 2].table, MPC8_RES_BITS, 2) + bands[i + 1].res[1];
> > +            if(bands[i].res[0] > 15) bands[i].res[0] -= 17;
> > +            if(bands[i].res[1] > 15) bands[i].res[1] -= 17;
> > +        }
> 
> for(i = maxband - 1; i >= 0; i--){
>     for(j=0; j<2; j++){
>         last[j] = get_vlc2(gb, res_vlc[last[j] > 2].table, MPC8_RES_BITS, 2) + last[j];
>         if(last[j] > 15) last[j] -= 17;
>         bands[i].res[j]= last[j];
>     }
> }
> 
> 
> [...]
> > +const static uint8_t mpc8_cnk_len[16][32] =
> > +{
> > +	{0, 1, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5},
> [...]
> > +const static uint32_t mpc8_cnk_lost[16][32] =
> > +{
> > +	{0, 0, 1, 0, 3, 2, 1, 0, 7, 6, 5, 4, 3, 2, 1, 0, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0},
> [...]
> > +static const uint8_t mpc8_log2[32] =
> > +{ 0, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 5};
> > +
> > +static const uint8_t mpc8_log2_lost[32] =
> > +{ 0, 1, 0, 3, 2, 1, 0, 7, 6, 5, 4, 3, 2, 1, 0, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 31};
> 
> these are nearly duplicate maybe you could change the code a little so
> only 1 would be needed

it's related to Golomb (not Rice) codes
first are obtained as ceil(log2(n)), the second is 2^ceil(log2(n)) - n
I'm trying to find a way to completely remove them
 
> [...]
> > +static const uint16_t mpc8_q6_codes[2][MPC8_Q6_SIZE] = {
> > +{
> > + 0x0000, 0x0001, 0x0002, 0x0003, 0x0004, 0x0005, 0x0004, 0x0003,
> > + 0x0004, 0x0005, 0x0005, 0x0006, 0x0004, 0x0005, 0x0004, 0x0003,
> > + 0x0005, 0x0006, 0x0007, 0x0007, 0x0006, 0x0007, 0x0008, 0x0009,
> > + 0x0005, 0x0006, 0x0007, 0x0004, 0x0005, 0x0006, 0x0007,
> > +},
> > +{
> > + 0x0000, 0x0001, 0x0002, 0x0003, 0x0004, 0x0004, 0x0005, 0x0004,
> > + 0x0005, 0x0006, 0x0005, 0x0006, 0x0007, 0x0008, 0x0009, 0x000A,
> > + 0x000B, 0x000C, 0x000D, 0x000E, 0x000F, 0x0007, 0x0008, 0x0009,
> > + 0x0006, 0x0007, 0x0005, 0x0006, 0x0007, 0x0002, 0x0003,
> > +}
> > +};
> 
> this and several other tabels fit in 8bit

shrinked 
 
> [...]
> > +/* 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?
 
> [...]
> > +static void mpc8_parse_seektable(AVFormatContext *s, int64_t off)
> > +{
> > +    MPCContext *c = s->priv_data;
> > +    int tag;
> > +    int64_t size, pos, ppos[2];
> > +    uint8_t *buf;
> > +    int i, t, seekd;
> > +    GetBitContext gb;
> > +
> > +    url_fseek(&s->pb, off, SEEK_SET);
> > +    mpc8_get_chunk_header(&s->pb, &tag, &size);
> > +    if(tag != TAG_SEEKTABLE){
> > +        av_log(s, AV_LOG_ERROR, "No seek table at given position\n");
> > +        return;
> > +    }
> > +    if(!(buf = av_malloc(size)))
> > +        return;
> > +    get_buffer(&s->pb, buf, size);
> > +    init_get_bits(&gb, buf, size * 8);
> > +    size = gb_get_v(&gb);
> 
> check size please
> 
> [...]
> > +    s->start_time = 0;
> > +    s->duration = c->samples * AV_TIME_BASE / st->codec->sample_rate;
> 
>     /** decoding: position of the first frame of the component, in
>        AV_TIME_BASE fractional seconds. NEVER set this value directly:
>                                         ^^^^^ 
>        it is deduced from the AVStream values.  */
>     int64_t start_time;
>     /** decoding: duration of the stream, in AV_TIME_BASE fractional
>        seconds. NEVER set this value directly: it is deduced from the
>                 ^^^^^
>        AVStream values.  */
>     int64_t duration;

Oops, will set stream duration instead. 

I will send a patch when I resolve Golomb-like codes reading and get_v()
will be made common.
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB