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

Anssi Hannula anssi.hannula
Fri Apr 6 20:49:45 CEST 2007


Michael Niedermayer wrote:
> Hi

Hi!

> On Fri, Apr 06, 2007 at 01:36:22PM +0300, Anssi Hannula wrote:
> [...]
>> +static inline void c93_draw_4x4_colorblock(uint8_t *out, uint8_t **buf,
>> +                          int bpp, int colcnt, int stride, C93BlockType bt)
>> +{
>> +    unsigned int y_off, x_off, colmask, i, j, colbit;
>> +    uint8_t allcols[4];
>> +    uint8_t curcols[2];
>> +    uint8_t *cols = bt == C93_4X4_4COLOR_GRP ? curcols : allcols;
>> +    int bit = 1 + (bpp & 2);
>> +
>> +    for (y_off = 0; y_off < 8; y_off += 4) {
>> +        uint8_t *to = &out[y_off*stride];
>> +        for (x_off = 0; x_off < 8; x_off += 4) {
>> +            colbit = 0;
>> +            bytestream_get_buffer(buf, allcols, colcnt);
>> +            if (bt == C93_4X4_4COLOR)
>> +                colmask = bytestream_get_le32(buf);
>> +            else
>> +                colmask = bytestream_get_le16(buf);
>> +            if (bt == C93_4X4_4COLOR_GRP)
>> +                curcols[0] = allcols[0];
>> +
>> +            for (j = 0; j < 4; j++) {
>> +                if (bt == C93_4X4_4COLOR_GRP) {
>> +                    if (j == 2)
>> +                        curcols[0] = allcols[3];
>> +                    curcols[1] = allcols[1];
>> +                }
>> +                for (i = 0; i < 4; i++) {
>> +                    if (bt == C93_4X4_4COLOR_GRP && i == 2)
>> +                        curcols[1] = allcols[2];
>> +                    to[j*stride+i] = cols[(colmask >> colbit) & bit];
>> +                    colbit += bpp;
>> +                }
>> +            }
>> +            to += 4;
>> +        }
>> +    }
>> +}
>> +
>> +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 = &c93->pictures[c93->currentpic];
>> +    AVFrame * const oldpic = &c93->pictures[c93->currentpic^1];
>> +    AVFrame *picture = data;
>> +    uint8_t *out;
>> +    int stride, i, x, y;
>> +    C93BlockType bt = 0;
>> +
>> +    c93->currentpic ^= 1;
>> +
>> +    newpic->reference = 1;
>> +    newpic->buffer_hints = FF_BUFFER_HINTS_VALID | FF_BUFFER_HINTS_PRESERVE |
>> +                         FF_BUFFER_HINTS_REUSABLE | FF_BUFFER_HINTS_READABLE;
>> +    if (avctx->reget_buffer(avctx, newpic)) {
>> +        av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
>> +        return -1;
>> +    }
>> +
>> +    stride = newpic->linesize[0];
>> +
>> +    if (buf[0] & C93_FIRST_FRAME) {
>> +        newpic->pict_type = FF_I_TYPE;
>> +        newpic->key_frame = 1;
>> +    } else {
>> +        newpic->pict_type = FF_P_TYPE;
>> +        newpic->key_frame = 0;
>> +    }
>> +
>> +    if (*buf++ & C93_HAS_PALETTE) {
>> +        uint32_t *palette = (uint32_t *) newpic->data[1];
>> +        uint8_t *palbuf = buf + buf_size - 768 - 1;
>> +        for (i = 0; i < 256; i++) {
>> +            palette[i] = bytestream_get_be24(&palbuf);
>> +        }
>> +    } else {
>> +        if (oldpic->data[1])
>> +            memcpy(newpic->data[1], oldpic->data[1], 256 * 4);
>> +    }
>> +
>> +    for (y = 0; y < HEIGHT; y += 8) {
>> +        out = newpic->data[0] + y * stride;
>> +        for (x = 0; x < WIDTH; x += 8) {
>> +            uint8_t *copy_from = oldpic->data[0];
>> +            unsigned int offset, j;
>> +            uint8_t cols[2];
>> +
>> +            if (!bt)
>> +                bt = *buf++;
>> +
>> +            switch (bt & 0x0F) {
>> +            case C93_8X8_FROM_PREV:
>> +                offset = bytestream_get_le16(&buf);
>> +                if (c93_copy_block(avctx, out, copy_from, offset, 8, stride))
>> +                    return -1;
>> +                break;
>> +
>> +            case C93_4X4_FROM_CURR:
>> +                copy_from = newpic->data[0];
>> +            case C93_4X4_FROM_PREV:
>> +                for (j = 0; j < 8; j += 4) {
>> +                    for (i = 0; i < 8; i += 4) {
>> +                        offset = bytestream_get_le16(&buf);
>> +                        if (c93_copy_block(avctx, &out[j*stride+i],
>> +                                           copy_from, offset, 4, stride))
>> +                            return -1;
>> +                    }
>> +                }
>> +                break;
>> +
>> +            case C93_8X8_2COLOR:
>> +                cols[0] = *buf++;
>> +                cols[1] = *buf++;
>> +                for (j = 0; j < 8; j++) {
>> +                    for (i = 0; i < 8; i++) {
>> +                        out[j*stride+i] = cols[(buf[0] >> i) & 1];
>> +                    }
>> +                    buf++;
>> +                }
>> +                break;
>> +
>> +            case C93_4X4_2COLOR:
>> +                c93_draw_4x4_colorblock(out, &buf, 1, 2, stride, bt & 0x0F);
>> +                break;
>> +
>> +            case C93_4X4_4COLOR_GRP:
>> +                c93_draw_4x4_colorblock(out, &buf, 1, 4, stride, bt & 0x0F);
>> +                break;
>> +
>> +            case C93_4X4_4COLOR:
>> +                c93_draw_4x4_colorblock(out, &buf, 2, 4, stride, bt & 0x0F);
>> +                break;
> 
> i think the following is more readable and simpler

I agree.

> static inline void draw_n_color(uint8_t *out, int stride, int width, int height,
>                                 int bpp, uint8_t cols[2], uint8_t grps[4], uint32_t col){
>     int x,y;
>     for(y=0; y<height; y++){
>         if(grps)
>             cols[1]= grps[3*(y>>1)];
>         for(x=0; x<width; x++){
>             if(grps)
>                 cols[0]= grps[(x>>1)+1];
>             out[x + y*stride] = cols[col >> (32-bpp)];
>             col<<=bpp;

You're reading col data backwards, this won't work.
We need
             out[x + y*stride] = cols[col & ((bpp & 2) + 1)];
             col >>= bpp;
or more generic
             out[x + y*stride] = cols[col & ((1 << bpp) - 1)];
             col >>= bpp;
unless you come up with something simpler (again), of course ;)

I put the latter one into the attached patch which includes your
approach with minor fixes.

>         }
>     }
> }
> case C93_8X8_2COLOR:
>     bytestream_get_buffer(buf, cols, 2);
>     for(i=0; i<8; i++){
>         draw_n_color(out, stride, 8, 1, 1, cols, NULL, *buf++);
>     }
>     break;
> case C93_4x4_2COLOR:
> case C93_4X4_4COLOR:
> case C93_4X4_4COLOR_GRP:
>     for(i=0; i<8; i+=4){
>         for(j=0; j<8; j+=4){
>
>             if(C93_4x4_2COLOR){
>                 bytestream_get_buffer(buf, cols, 2);
>                 draw_n_color(out + i + j*stride, stride, 4, 4, 1, cols, NULL, bytestream_get_le16(buf));
>             }else if(C93_4X4_4COLOR){
>                 bytestream_get_buffer(buf, cols, 4);
>                 draw_n_color(out + i + j*stride, stride, 4, 4, 2, cols, NULL, bytestream_get_le32(buf));
>             }else{
>                 bytestream_get_buffer(buf, grps, 4);
>                 draw_n_color(out + i + j*stride, stride, 4, 4, 1, cols, grps, bytestream_get_le16(buf));
>             }
>         }
>     }
>     break;
> 
> 
[...]
> 
> [...]
>> +    if (ret < datasize) {
>> +        av_free_packet(pkt);
>> +        return AVERROR_IO;
>> +    }
>> +
>> +    datasize = get_le16(pb); /* palette size */
>> +    if (datasize) {
>> +        if (datasize != 768) {
>> +            av_log(s, AV_LOG_ERROR, "invalid palette size %u\n", datasize);
>> +            av_free_packet(pkt);
>> +            return AVERROR_INVALIDDATA;
>> +        }
>> +        pkt->data[0] |= C93_HAS_PALETTE;
>> +        ret = get_buffer(pb, pkt->data + pkt->size, datasize);
>> +        if (ret < datasize) {
>> +            av_free_packet(pkt);
>> +            return AVERROR_IO;
> 
> there are 3 av_free_packet(pkt); maybe a goto fail ... would be simpler

Ok.

-- 
Anssi Hannula

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-c93_6.diff
Type: text/x-patch
Size: 18785 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070406/191345fa/attachment.bin>



More information about the ffmpeg-devel mailing list