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

[Michael Niedermayer]

On Thu, Dec 10, 2009 at 10:59:23PM -0800, Michael Tison wrote:
[...]
> +static int cdg_tile_block(CDGraphicsContext *cc, uint8_t *data, int b)
> +{
> +    int ci, ri;
> +    int color;
> +    int x, y;
> +    int ai;
> +    int stride   = cc->frame.linesize[0];
> +    uint8_t *buf = cc->frame.data[0];
> +
> +    ri = (data[2] & 0x1F) * CDG_TILE_HEIGHT + cc->vscroll;
> +    ci = (data[3] & 0x3F) * CDG_TILE_WIDTH  + cc->hscroll;
> +
> +    if ((unsigned) ri > (CDG_FULL_HEIGHT - CDG_TILE_HEIGHT)) {

you could make ci/ri unsigned which would avoid this cast


> +        av_log(NULL, AV_LOG_ERROR, "tile is out of height range\n");

NULL is a bad context

[...]
> +static void cdg_copy_rect_buf(int out_tl_x, int out_tl_y, uint8_t *out,
> +                              int in_tl_x, int in_tl_y, uint8_t *in,
> +                              int w, int h, int stride)
> +{
> +    int y;
> +
> +    in  = in  + in_tl_x  + in_tl_y  * stride;

in +=


[...]
> +static void cdg_fill_wrapper(int out_tl_x, int out_tl_y, uint8_t *out,
> +                             int in_tl_x, int in_tl_y, uint8_t *in,
> +                             int color, int w, int h, int stride, int roll)
> +{

> +    if (roll)
> +        cdg_copy_rect_buf(out_tl_x, out_tl_y, out, in_tl_x, in_tl_y,
> +                          in, w, h, stride);
> +    else

missing {}
btw, you should try tools/patcheck


[...]
> +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;
> +    int ret, xor, cpy;
> +    uint8_t command, inst;
> +    uint8_t cdg_data[CDG_DATA_SIZE];
> +    AVFrame new_frame;
> +    CDGraphicsContext *cc = avctx->priv_data;
> +
> +    if (avctx->reget_buffer(avctx, &cc->frame)) {
> +        av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
> +        return -1;
> +    }
> +
> +    command = bytestream_get_byte(&buf);
> +    inst    = bytestream_get_byte(&buf);
> +    buf += 2;  /// skipping 2 unneeded bytes
> +    bytestream_get_buffer(&buf, cdg_data, CDG_DATA_SIZE);
> +
> +    if ((command & CDG_MASK) == CDG_COMMAND) {
> +        switch (inst & CDG_MASK) {
> +        case CDG_INST_MEMORY_PRESET:
> +            if (!(cdg_data[1] & 0x0F))
> +                memset(cc->frame.data[0], cdg_data[0] & 0x0F,
> +                       cc->frame.linesize[0] * CDG_FULL_HEIGHT);
> +            break;
> +        case CDG_INST_LOAD_PAL_LO:
> +            cdg_load_palette(cc, cdg_data, 1);
> +            break;
> +        case CDG_INST_LOAD_PAL_HIGH:
> +            cdg_load_palette(cc, cdg_data, 0);
> +            break;
> +        case CDG_INST_BORDER_PRESET:
> +            cdg_border_preset(cc, cdg_data);
> +            break;
> +        case CDG_INST_TILE_BLOCK_XOR:
> +        case CDG_INST_TILE_BLOCK:

> +            xor = (inst & CDG_MASK) == CDG_INST_TILE_BLOCK_XOR ? 1 : 0;

?1:0 is superflous

[..]
> +static int read_header(AVFormatContext *s, AVFormatParameters *ap)
> +{
> +    AVStream *vst;
> +
> +    vst = av_new_stream(s, 0);
> +    if (!vst)
> +        return AVERROR(ENOMEM);
> +
> +    vst->codec->codec_type = CODEC_TYPE_VIDEO;
> +    vst->codec->codec_id   = CODEC_ID_CDGRAPHICS;
> +
> +    /// 75 sectors/sec * 4 packets/sector = 300 packets/sec
> +    av_set_pts_info(vst, 32, 1, 300);
> +

> +    vst->duration = (url_fsize(s->pb) * vst->time_base.den) / (CDG_PACKET_SIZE * 300);

url_fsize() can fail, this is missing error checking

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/20091211/81b7fadb/attachment.pgp>