[FFmpeg-devel] [PATCH 2/3] Bitmap Brothers JV demuxer

[Kostya]

On Wed, Mar 09, 2011 at 02:10:50AM +1100, Peter Ross wrote:
> ---
> diff --git a/libavformat/jvdec.c b/libavformat/jvdec.c
> new file mode 100644
> index 0000000..7acaabf
> --- /dev/null
> +++ b/libavformat/jvdec.c
[...]
> +} JVDemuxContext;
> +
> +#define MAGIC " Compression by John M Phillips Copyright (C) 1995 The Bitmap Brothers Ltd."
> +
> +static int read_probe(AVProbeData *pd)
> +{
> +    if (pd->buf[0] == 'J' && pd->buf[1] == 'V' &&
> +        !memcmp(pd->buf + 4, MAGIC, FFMIN(strlen(MAGIC), pd->buf_size - 4)))
> +        return AVPROBE_SCORE_MAX;
> +    return 0;
> +}

I think it's wrong to return score_max if you have too small probe buffer not
to fit all magic string.

[...]

BTW, do you initialise state explicitly somewhere?

> +static int read_packet(AVFormatContext *s,
> +                       AVPacket *pkt)
> +{
> +    JVDemuxContext *jv = s->priv_data;
> +    AVIOContext *pb = s->pb;
> +    AVStream *ast = s->streams[0];
> +
> +    while (!url_feof(s->pb) && jv->pts < ast->nb_index_entries) {
> +        const AVIndexEntry *e   = ast->index_entries + jv->pts;
> +        const JVFrame      *jvf = jv->frames + jv->pts;
> +
> +        switch(jv->state) {
> +        case JV_AUDIO:
> +            jv->state++;
> +            if (jvf->audio_size ) {
> +                if (av_get_packet(s->pb, pkt, jvf->audio_size) < 0)
> +                    return AVERROR(ENOMEM);
> +                pkt->stream_index = 0;
> +                pkt->pts          = e->timestamp;
> +                pkt->flags       |= PKT_FLAG_KEY;
> +                return 0;
> +            }
> +        case JV_VIDEO:
> +            jv->state++;
> +            if (jvf->video_size || jvf->palette) {
> +                int size = jvf->video_size + (jvf->palette ? 768 : 0);
> +                if (av_new_packet(pkt, size + 5))
> +                    return AVERROR(ENOMEM);
> +
> +                AV_WL32(pkt->data, jvf->video_size);
> +                pkt->data[4]      = jvf->video_type;
> +                if (avio_read(pb, pkt->data + 5, size) < 0)
> +                    return AVERROR(EIO);
> +
> +                pkt->size         = size + 5;
> +                pkt->stream_index = 1;
> +                pkt->pts          = jv->pts;
> +                if (jvf->video_type != 1)
> +                    pkt->flags |= PKT_FLAG_KEY;
> +                return 0;
> +            }
> +        case JV_PADDING:
> +            avio_skip(pb, e->size - jvf->audio_size - jvf->video_size - (jvf->palette ? 768 : 0));

Is that guaranteed to be always non-negative?

> +            jv->state = JV_AUDIO;
> +            jv->pts++;
> +        }
> +    }

Nice state machine you have here though.

[the rest rises no comments from me]