[FFmpeg-devel] [PATCH] CD+G Demuxer & Decoder

[Michael Niedermayer]

On Tue, Dec 01, 2009 at 12:13:24AM -0800, Michael Tison wrote:
[...]

> +    int color;
> +    int repeat;
> +
> +    color = cp->data[0] & 0x0F;
> +    repeat = cp->data[1] & 0x0F;
> +    if (!repeat)
> +       memset(cc->frame.data[0], color,
> +              cc->frame.linesize[0] * CDG_FULL_HEIGHT);
> +}
> +
> +static void cdg_border_preset(CDGraphicsContext *cc, CdgPacket *cp)
> +{
> +    int color;
> +    int repeat;
> +    int y;
> +    int lsize    = cc->frame.linesize[0];
> +    uint8_t *buf = cc->frame.data[0];
> +
> +    color  = cp->data[0] & 0x0F;
> +    repeat = cp->data[1] & 0x0F;

looks duplicated


[...]
> +static void cdg_tile_block(CDGraphicsContext *cc, CdgPacket *cp, int b)
> +{
> +    int c0, c1;
> +    int ci, ri;
> +    int byte, pix, color;
> +    int x, y;
> +    int ai;
> +    int lsize    = cc->frame.linesize[0];
> +    uint8_t *buf = cc->frame.data[0];
> +
> +    c0 =  cp->data[0] & 0x0F;
> +    c1 =  cp->data[1] & 0x0F;
> +    ri = (cp->data[2] & 0x1F) * CDG_TILE_HEIGHT;
> +    ci = (cp->data[3] & 0x3F) * CDG_TILE_WIDTH;
> +
> +    if (ri > (CDG_FULL_HEIGHT - CDG_TILE_HEIGHT))
> +       return;
> +    if (ci > (CDG_FULL_WIDTH - CDG_TILE_WIDTH))
> +       return;
> +
> +    for (y = 0; y < CDG_TILE_HEIGHT; y++) {
> +       byte = cp->data[4 + y] & 0x3F;
> +       for (x = 0; x < CDG_TILE_WIDTH; x++) {
> +           pix = (byte >> (5 - x)) & 0x01;
> +           ai = ci + x + cc->hscroll + (lsize * (ri + y + cc->vscroll));

> +           assert(ai >> 0 && ai < CDG_FULL_HEIGHT * lsize);

is it certain that no random input can make this fail?



[..]
> +static void cdg_fill_rect_buf(int out_tl_x, int out_tl_y,

copy not fill


> +                              uint8_t *out,
> +                              int in_tl_x, int in_tl_y,
> +                              uint8_t *in, int w, int h, int lsize)
> +{
> +    int x, y;
> +    in = in + in_tl_x + in_tl_y * lsize;
> +    out = out + out_tl_x + out_tl_y * lsize;
> +    for (y = 0; y < h; y++)
> +       for (x = 0; x < w; x++)
> +           out[x + y * lsize] = in[x + y * lsize];
> +}

[...]
> +static int cdg_decode_frame(AVCodecContext *avctx,
> +                            void *data, int *data_size, AVPacket *avpkt)
> +{
> +    const uint8_t *buf = avpkt->data;
> +    int buf_size = avpkt->size;
> +    AVFrame new_frame;
> +    CDGraphicsContext *cc = avctx->priv_data;
> +    CdgPacket cp;
> +
> +    if (avctx->reget_buffer(avctx, &cc->frame)) {
> +       av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
> +       return -1;
> +    }
> +
> +    cp.command     = bytestream_get_byte(&buf);
> +    cp.instruction = bytestream_get_byte(&buf);

> +    bytestream_get_buffer(&buf, (uint8_t *) & cp.parityQ, 2);
> +    bytestream_get_buffer(&buf, (uint8_t *) & cp.data,   16);
> +    bytestream_get_buffer(&buf, (uint8_t *) & cp.parityP, 2);

parity is unused, use it or dont read it.


[...]
> +       *data_size = sizeof(AVFrame);
> +       *(AVFrame *) data = cc->frame;
> +       return buf_size;
> +    }
> +
> +    *data_size = 0;
> +    *(AVFrame *) data = cc->frame;
> +    return 0;

this looks like it can be done simpler


[...]
> +static int read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    int ret;
> +    ByteIOContext *pb = s->pb;
> +

> +    ret = av_get_packet(pb, pkt, CDG_PACKET_SIZE);
> +    if (pb->eof_reached == 1)
> +       return -1;

possible memleak, and wrong return code


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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20091201/6f0a0ce7/attachment.pgp>