[Ffmpeg-devel] [PATCH] C93 demuxer and decoder (GSoC qualification task)

Måns Rullgård mans
Sun Apr 1 23:16:22 CEST 2007


Anssi Hannula <anssi.hannula at gmail.com> writes:

> Hi!
>
> As my Google Summer of Code qualification task, attached is a patch
> which adds C93 [1] demuxer and video decoder to FFmpeg. The samples from
> samples.mplayerhq.hu can be properly played out with it.
>
> I'm not too familiar with FFmpeg codebase yet, though, so there could be
> some silly mistakes in the code ;). Please review it.
>
> [1] http://wiki.multimedia.cx/index.php?title=C93
>
> Index: libavcodec/c93.c
> ===================================================================
> --- libavcodec/c93.c	(revision 0)
> +++ libavcodec/c93.c	(revision 0)

[license header]

> +static inline int c93_conv_offset(int offset, int x, int y, int size,
> +                                                    int stride)
> +{
> +    int prev_x = offset % 320 + x;
> +    int prev_y = offset / 320 + y;
> +    if (prev_x >= 320) {
> +        prev_x -= 320;
> +        prev_y += size;
> +    }
> +    return prev_y*stride+prev_x;
> +}
> +
> +static int c93_decode_frame(AVCodecContext * avctx,
> +                 void *data, int *data_size, uint8_t * buf, int buf_size)
> +{
> +    C93DecoderContext * const c93 = avctx->priv_data;
> +    AVFrame * const newpic = (AVFrame*) &(c93->pictures[c93->currentpic]);
> +    AVFrame * const oldpic = (AVFrame*) &(c93->pictures[1 - c93->currentpic]);

Needless casts and parens.

> +    AVFrame *picture = data;
> +    uint8_t *out;
> +    int stride, i, x, y;
> +    C93BlockType bt1, bt2;
> +
> +    c93->currentpic = c93->currentpic ? 0 : 1;

c93->currentpic ^= 1;

> +    if (avctx->reget_buffer(avctx, newpic)) {
> +        av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
> +        return -1;
> +    }
> +    if (avctx->reget_buffer(avctx, oldpic)) {
> +        av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
> +        return -1;
> +    }
> +
> +    stride = newpic->linesize[0];
> +    out = newpic->data[0];
> +
> +    newpic->reference = 1;
> +    if (buf[0] & 0x02) { /* first frame */

#define C93_FIRST_FRAME 2 or something and drop the comment.

> +        newpic->pict_type = FF_I_TYPE;
> +        newpic->key_frame = 1;
> +    } else {
> +        newpic->pict_type = FF_P_TYPE;
> +        newpic->key_frame = 0;
> +    }
> +
> +    if ((buf++)[0] & 0x01) { /* contains palette */

*buf++, and make that 1 a #define too.

> +        uint32_t *pal1 = (uint32_t *) newpic->data[1];
> +        uint32_t *pal2 = (uint32_t *) oldpic->data[1];
> +        for (i = 0; i < 256; i++, buf += 3) {
> +            pal1[i] = pal2[i] = (buf[2] << 16) | (buf[1] << 8) | buf[0];

AV_RL24(buf)

> +        }
> +    }
> +
> +    bt1 = bt2 = 0;
> +    for (x = y = 0; x < 320 || y < 184; x += 8) {
> +        int offset, y_off, x_off, j, k, col0, col1;
> +        int colbit = 0;
> +        int cols[4];
> +
> +        if (x >= 320) {
> +            y += 8;
> +            x = 0;
> +        }
> +        if (!bt1) {
> +            bt1 = buf[0] & 0x0F;
> +            bt2 = (buf[0] >> 4) & 0x0F;
> +            buf++;
> +        }
> +
> +        switch (bt1) {
> +        case C93_8X8_FROM_PREV:
> +            offset = AV_RL16(buf);
> +            buf += 2;
> +            for (j = 0; j < 8; j++)
> +                for (i = 0; i < 8; i++) {
> +                    int loc = c93_conv_offset(offset, i, j, 8, stride);

I'm sure this could be calculated more efficiently than by calling
that function for each iteration.

> +                    if (loc >= 192 * stride + 320) {
> +                        av_log(avctx, AV_LOG_ERROR, "invalid offset %d for"
> +                                " mode 2 at %dx%d\n", offset, x, y);
> +                        return -1;
> +                    }
> +                    out[(y+j)*stride+x+i] = oldpic->data[0][loc];
> +                }

Isn't this just another way of saying memcpy()?  If it can't be
removed, the outer for loop should also have braces.  There's enough
stuff inside it that braces would make it easier to read.

> +            break;
> +
> +        case C93_4X4_FROM_PREV:
> +            for (k = 0; k < 4; k++) {
> +                y_off = k > 1 ? 4 : 0;
> +                x_off = (k % 2) ? 4 : 0;

k & 1 or make k unsigned (or both).

> +                offset = AV_RL16(buf);
> +                if (offset >= 320 * 192) {
> +                    return -1;

No reason?

> +                }
> +                buf += 2;
> +                for (j = 0; j < 4; j++)
> +                    for (i = 0; i < 4; i++) {
> +                        int loc = c93_conv_offset(offset, i, j, 4, stride);
> +                        if (loc >= 192 * stride + 320) {
> +                            av_log(avctx, AV_LOG_ERROR, "invalid offset %d for"
> +                                    " mode 6 at %dx%d\n", offset, x, y);
> +                            return -1;
> +                        }
> +                        out[(y+j+y_off)*stride+x+i+x_off] =
> +                                                       oldpic->data[0][loc];
> +                    }
> +            }
> +            break;

[...]

The other prediction modes are very similar.  Try to merge them into
an inline function.  Constant arguments to an inline function allow
the same optimizations as constants directly in the code.

> +        case C93_NOOP:
> +            break;
> +
> +        case C93_8X8_INTRA:
> +            for (j = 0; j < 8; j++)
> +                for (i = 0; i < 8; i++)
> +                    out[(y+j)*stride+x+i] = (buf++)[0];

Replace the inner loop with a memcpy().

> +            break;
> +
> +        default:
> +            av_log(avctx, AV_LOG_ERROR, "unexpected type %x at %dx%d\n",
> +                                                                bt1, x, y);
> +            return -1;
> +        }
> +        bt1 = bt2;
> +        bt2 = 0;
> +    }
> +
> +    *picture = *newpic;
> +    *data_size = sizeof(AVFrame);
> +
> +    return buf_size;
> +}
> +
> +AVCodec c93_decoder = {
> +    "c93",
> +    CODEC_TYPE_VIDEO,
> +    CODEC_ID_C93,
> +    sizeof(C93DecoderContext),
> +    c93_decode_init,
> +    NULL,
> +    NULL,
> +    c93_decode_frame,
> +    CODEC_CAP_DR1,
> +};

[...]

> Index: libavformat/c93.c
> ===================================================================
> --- libavformat/c93.c	(revision 0)
> +++ libavformat/c93.c	(revision 0)
> @@ -0,0 +1,206 @@
> +/*
> + * Interplay C93 demuxer
> + * Copyright (c) 2007 Anssi Hannula <anssi.hannula at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include "avformat.h"
> +#include "voc.h"
> +
> +typedef struct {
> +    uint16_t index;
> +    uint8_t length;
> +    uint8_t frames;
> +} C93BlockRecord;
> +
> +typedef struct {
> +    voc_dec_context_t voc;
> +
> +    C93BlockRecord block_records[512];
> +    int current_block;
> +
> +    uint32_t frame_offsets[32];
> +    int current_frame;
> +    int decode_audio;
> +
> +    AVStream *audio;
> +} C93DemuxContext;
> +
> +static int c93_probe(AVProbeData *p)
> +{
> +    if (p->buf_size < 13)
> +        return 0;
> +
> +    if (p->buf[0] == 0x01 && p->buf[1] == 0x00 &&
> +        p->buf[4] == 0x01 + p->buf[2] &&
> +        p->buf[8] == p->buf[4] + p->buf[6] &&
> +        p->buf[12] == p->buf[8] + p->buf[10])
> +        return AVPROBE_SCORE_MAX / 2;
> +
> +    return 0;
> +}
> +
> +static int c93_read_header(AVFormatContext *s,
> +                           AVFormatParameters *ap)
> +{
> +    AVStream *video;
> +    ByteIOContext *pb = &s->pb;
> +    C93DemuxContext *c93 = s->priv_data;
> +    int i;
> +    int framecount = 0;
> +
> +    for (i = 0; i < 512; i++) {
> +        c93->block_records[i].index = get_le16(pb);
> +        c93->block_records[i].length = get_byte(pb);
> +        c93->block_records[i].frames = get_byte(pb);
> +        if (c93->block_records[i].frames > 32) {
> +            av_log(s, AV_LOG_ERROR, "too many frames in block\n");
> +            return AVERROR_INVALIDDATA;
> +        }
> +        framecount += c93->block_records[i].frames;
> +    }
> +
> +    video = av_new_stream(s, 0);
> +    if (!video)
> +        return AVERROR_NOMEM;
> +
> +    video->codec->codec_type = CODEC_TYPE_VIDEO;
> +    video->codec->codec_id = CODEC_ID_C93;
> +    video->codec->width = 320;
> +    video->codec->height = 192;
> +    video->codec->time_base = (AVRational) { 2, 25 };
> +    /* 4:3 320x200 with 8 empty lines */
> +    video->codec->sample_aspect_ratio = (AVRational) { 5, 6 };
> +    video->nb_frames = framecount;
> +
> +    c93->audio = av_new_stream(s, 1);
> +    if (!c93->audio) {
> +        av_freep(video);
> +        return AVERROR_NOMEM;
> +    }
> +
> +    c93->audio->codec->codec_type = CODEC_TYPE_AUDIO;
> +    c93->current_block = 0;
> +    c93->current_frame = 0;
> +    c93->decode_audio = -1;
> +
> +    return 0;
> +}
> +
> +static int c93_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    ByteIOContext *pb = &s->pb;
> +    C93DemuxContext *c93 = s->priv_data;
> +    C93BlockRecord *br = &c93->block_records[c93->current_block];
> +    uint16_t videosize;
> +    uint16_t palettesize;
> +    uint16_t soundsize;
> +    int ret, i;
> +    offset_t location;
> +
> +    location = url_ftell(pb);
> +    if (br->index * 2048 + c93->current_frame * 4 > location) {
> +        c93->current_frame = 0;
> +        c93->decode_audio = -1;
> +        while (c93->current_block > 0 && br->index * 2048 > location) {
> +            c93->current_block--;
> +            br--;
> +        }
> +    }
> +
> +    if (c93->decode_audio ==
> +        c93->current_block * 32 + c93->current_frame) {
> +        c93->current_frame++;
> +
> +        soundsize = get_le16(pb);
> +        if (soundsize > 42) {
> +            url_fskip(pb, 26); /* VOC header */
> +            ret = voc_get_packet(s, pkt, c93->audio, soundsize - 26);
> +            if (ret > 0) {
> +                pkt->stream_index = 1;
> +                pkt->flags |= PKT_FLAG_KEY;
> +                return ret;
> +            }
> +        }
> +    }
> +    if (c93->current_frame >= br->frames) {
> +        if (c93->current_block >= 511 || !(br+1)->length)

I prefer br[1].length.

> +            return AVERROR_IO;
> +        br++;
> +        c93->current_block++;
> +        c93->current_frame = 0;
> +    }
> +
> +    if (c93->current_frame == 0) {
> +        url_fseek(pb, br->index * 2048, SEEK_SET);
> +        for (i = 0; i < 32; i++) {
> +            c93->frame_offsets[i] = get_le32(pb);
> +        }
> +    }
> +
> +    url_fseek(pb,br->index * 2048 +
> +            c93->frame_offsets[c93->current_frame], SEEK_SET);
> +    videosize = get_le16(pb);
> +    url_fskip(pb, videosize);
> +    palettesize = get_le16(pb);
> +
> +    ret = av_new_packet(pkt, videosize + palettesize + 1);
> +    if (ret < 0)
> +        return ret;
> +
> +    pkt->data[0] = palettesize ? 0x01 : 0x00;

pkt->data[0] = !!palettesize;

> +    if (palettesize) {
> +        if (palettesize != 768) {
> +            av_log(s, AV_LOG_ERROR, "invalid palette size %u\n", palettesize);
> +            av_free_packet(pkt);
> +            return AVERROR_INVALIDDATA;
> +        }
> +        ret = get_buffer(pb, pkt->data + 1, palettesize);
> +        if (ret < palettesize) {
> +            av_free_packet(pkt);
> +            return AVERROR_IO;
> +        }
> +    }
> +    url_fskip(pb, -(palettesize + videosize + 2));

Somehow I don't like the look of that.

> +    ret = get_buffer(pb, pkt->data + palettesize + 1, videosize);
> +    if (ret < videosize) {
> +        av_free_packet(pkt);
> +        return AVERROR_IO;
> +    }
> +    url_fskip(pb, palettesize + 2);
> +    pkt->size = palettesize + videosize + 1;
> +    pkt->stream_index = 0;
> +    c93->decode_audio = c93->current_block * 32 + c93->current_frame;
> +    /* only the first frame is guaranteed to not reference previous frames */
> +    if (c93->decode_audio == 0) {
> +        pkt->flags |= PKT_FLAG_KEY;
> +        pkt->data[0] |= 0x02;
> +    }
> +    return 0;
> +}
> +
> +AVInputFormat c93_demuxer = {
> +    "c93",
> +    "Interplay C93",
> +    sizeof(C93DemuxContext),
> +    c93_probe,
> +    c93_read_header,
> +    c93_read_packet,
> +};

-- 
M?ns Rullg?rd
mans at mansr.com




More information about the ffmpeg-devel mailing list