[FFmpeg-devel] [PATCH] MVI demuxer / Motion Pixels decoder

Michael Niedermayer michaelni
Thu Jul 3 03:00:10 CEST 2008


On Wed, Jul 02, 2008 at 07:04:50PM +0200, Gregory Montoir wrote:
> Gregory Montoir wrote:
>> Michael Niedermayer wrote:
>>> [...]
>>>> Index: libavcodec/bitstream.h
>>>> ===================================================================
>>>> --- libavcodec/bitstream.h    (r?vision 14030)
>>>> +++ libavcodec/bitstream.h    (copie de travail)
>>>
>>> maybe it would be better to bswap32 the input into a temporary buffer
>>> iam not sure, i need to think about this more.
>>> Is this faster than a bswap32 and a temp buffer?
>> good idea.
>> using read_time() in mp_decode_frame gives (on a pentium m 1.7) :
>> [...]
> >
>> so, as it's faster and less ugly code wise, I changed the code to bswap 
>> the bitstream.
>
> Looks like I overlooked dsputil.c... attached a new patch using the 
> bswap_buf dsp func.

[...]
> +static void mp_read_changes_map(MotionPixelsContext *mp, GetBitContext *gb, int count, int bits_len, int read_color)
> +{
> +    uint16_t *pixels;
> +    int offset, w, h, color = 0, x, y, i;
> +
> +    while (count--) {
> +        offset = get_bits_long(gb, mp->offset_bits_len);
> +        w      = get_bits(gb, bits_len) + 1;
> +        h      = get_bits(gb, bits_len) + 1;
> +        if (read_color)
> +            color = get_bits(gb, 15);
> +        x = offset % mp->avctx->width;
> +        y = offset / mp->avctx->width;
> +        if (x + w > mp->avctx->width) {
> +            w = mp->avctx->width - x;
> +            if (w <= 0)
> +                continue;
> +        }
> +        if (y + h > mp->avctx->height) {
> +            h = mp->avctx->height - y;
> +            if (h <= 0)
> +                continue;
> +        }

While the tests might be enough, there are a little bit too many things
that could go wrong for my taste, following is clearer:

unsigned int offset, w, h, color = 0, x, y, i;
    while (count--) {
        offset = get_bits_long(gb, mp->offset_bits_len);
        w      = get_bits(gb, bits_len) + 1;
        h      = get_bits(gb, bits_len) + 1;
        if (read_color)
            color = get_bits(gb, 15);
        x = offset % mp->avctx->width;
        y = offset / mp->avctx->width;
        if(y>=mp->avctx->height)
            continue;
        w= FFMIN(w, mp->avctx->width  - x);
        h= FFMIN(h, mp->avctx->height - y);



[...]
> +static void mp_read_codes_table(MotionPixelsContext *mp, GetBitContext *gb)
> +{
> +    int i;
> +
> +    if (mp->codes_count == 1)
> +        mp->codes[0].delta = get_bits(gb, 4);
> +    else {
> +        mp->max_codes_bits = get_bits(gb, 4);
> +        for (i = 0; i < mp->codes_count; ++i)
> +            mp->codes[i].delta = get_bits(gb, 4);
> +        mp->current_codes_count = 0;
> +        mp_get_code(mp, gb, 0, 0);
> +   }
> +}

nitpicks: the int i could be movedinto the else, and the if could have {}


> +
> +static int mp_gradient(MotionPixelsContext *mp, int component, int v)
> +{
> +    int delta;
> +
> +    delta = (v - 7) * mp->gradient_scale[component];
> +    mp->gradient_scale[component] = (v == 0 || v == 14) ? 2 : 1;
> +    return delta;
> +}
> +

> +static DeltaAcc mp_quant(MotionPixelsContext *mp, int offset)
> +{
> +    int color;
> +
> +    color = *(uint16_t *)&mp->frame.data[0][(offset / mp->avctx->width) * mp->frame.linesize[0] + (offset % mp->avctx->width) * 2];
> +    return mp_quant_table[color];
> +}

This is really loads RGB15 and converting from RGB15->YUV


> +
> +static void mp_invquant(MotionPixelsContext *mp, int offset, const DeltaAcc *d)
> +{
> +    int r, g, b, color;
> +
> +    mp_iquant(d->q2, d->q1, d->q0, &r, &g, &b);
> +    r = av_clip(r, 0, 31);
> +    g = av_clip(g, 0, 31);
> +    b = av_clip(b, 0, 31);
> +    color = (r << 10) | (g << 5) | b;
> +    *(uint16_t *)&mp->frame.data[0][(offset / mp->avctx->width) * mp->frame.linesize[0] + (offset % mp->avctx->width) * 2] = color;
> +}

and that converts back
I wonder if it would be possible to skip this back and forth convertion?
Or if that would just lead to pics with artifacts ...
Either way the function names need to be improved.


[...]

> +                d.q2 += mp_gradient(mp, 0, mp_get_vlc(mp, gb));
> +                if ((x & 3) == 0) {
> +                    d.q1 += mp_gradient(mp, 1, mp_get_vlc(mp, gb));
> +                    d.q0 += mp_gradient(mp, 2, mp_get_vlc(mp, gb));
> +                    mp->hdt[(y >> 2) * mp->avctx->width + x] = d;
> +                }
> +                mp_invquant(mp, x0 + x, &d);
> +                ++x;
> +            }
[...]
> +static void mp_decode_frame_helper(MotionPixelsContext *mp, GetBitContext *gb)
> +{
> +    int y, y0, x0;
> +    DeltaAcc d;
> +
> +    for (y = 0; y < mp->avctx->height; ++y) {
> +        x0 = y * mp->avctx->width;
> +        if (mp->changes_map[x0] != 0) {
> +            d = mp_quant(mp, x0);
> +            memset(mp->gradient_scale, 1, sizeof(mp->gradient_scale));
> +        } else {

> +            d.q2 += mp_gradient(mp, 0, mp_get_vlc(mp, gb));
> +            if ((y & 3) == 0) {
> +                d.q1 += mp_gradient(mp, 1, mp_get_vlc(mp, gb));
> +                d.q0 += mp_gradient(mp, 2, mp_get_vlc(mp, gb));
> +            }

this looks a duplicated ...


> +            mp->vdt[y] = d;
> +            mp_invquant(mp, x0, &d);
> +        }
> +    }
> +    for (y0 = 0; y0 < 2; ++y0)
> +        for (y = y0; y < mp->avctx->height; y += 2)
> +            mp_decode_line(mp, gb, y);
> +}
> +
> +static av_cold int mp_decode_frame(AVCodecContext *avctx,
> +                                 void *data, int *data_size,
> +                                 const uint8_t *buf, int buf_size)
> +{
> +    MotionPixelsContext *mp = avctx->priv_data;
> +    GetBitContext gb;
> +    int count1, count2, sz;
> +
> +    mp->frame.reference = 1;
> +    mp->frame.buffer_hints = FF_BUFFER_HINTS_VALID | FF_BUFFER_HINTS_PRESERVE | FF_BUFFER_HINTS_REUSABLE;
> +    if (avctx->reget_buffer(avctx, &mp->frame)) {
> +        av_log(avctx, AV_LOG_ERROR, "motionpixels: reget_buffer() failed\n");
> +        return -1;
> +    }
> +
> +    /* le32 bitstream msb first */
> +    mp->bswapbuf = av_fast_realloc(mp->bswapbuf, &mp->bswapbuf_size, buf_size + FF_INPUT_BUFFER_PADDING_SIZE);
> +    mp->dsp.bswap_buf((uint32_t *)mp->bswapbuf, (const uint32_t *)buf, buf_size / 4);
> +    if (buf_size & 3)
> +        memcpy(mp->bswapbuf + (buf_size & ~3), buf + (buf_size & ~3), buf_size & 3);
> +    init_get_bits(&gb, mp->bswapbuf, buf_size * 8);
> +
> +    memset(mp->changes_map, 0, avctx->width * avctx->height);

> +    count1 = get_bits(&gb, 12);
> +    count2 = get_bits(&gb, 12);
> +
> +    if ((avctx->extradata[1] & 2) != 0) {
> +        mp_read_changes_map(mp, &gb, count1, 8, 0);
> +        mp_read_changes_map(mp, &gb, count2, 4, 0);
> +        count1 = get_bits(&gb, 12);
> +        count2 = get_bits(&gb, 12);
> +    }
> +    mp_read_changes_map(mp, &gb, count1, 8, 1);
> +    mp_read_changes_map(mp, &gb, count2, 4, 1);


    for(i=...; i<2; i++){
        int count1 = get_bits(&gb, 12);
        int count2 = get_bits(&gb, 12);
        mp_read_changes_map(mp, &gb, count1, 8, i);
        mp_read_changes_map(mp, &gb, count2, 4, i);
    }



[...]
> +static int read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    int ret, count;
> +    MviDemuxContext *mvi = s->priv_data;
> +    ByteIOContext *pb = s->pb;
> +
> +    if (mvi->video_frame_size == 0) {
> +        mvi->video_frame_size = (mvi->get_int)(pb);

> +        if (mvi->current_frame_counter == 0) {
> +            count = (mvi->audio_size_counter + 512) >> MVI_FRAC_BITS;
> +        } else {
> +            if (mvi->audio_size_left == 0)
> +                return AVERROR(EIO);
> +            count = (mvi->audio_size_counter + mvi->audio_frame_size + 512) >> MVI_FRAC_BITS;
> +            if (count > mvi->audio_size_left)
> +                count = mvi->audio_size_left;
> +            mvi->audio_size_counter += mvi->audio_frame_size;
> +        }

if the first audio_size_counter would be audio_frame_size bytes smaller
then the if() should become unneeded


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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/20080703/08c2754e/attachment.pgp>



More information about the ffmpeg-devel mailing list