[FFmpeg-devel] [PATCH] add MicroDVD muxer and demuxer

[Tomas Härdin]

Aurelien Jacobs skrev 2011-03-29 00:42:
> --- /dev/null
> +++ b/libavformat/microdvddec.c
> +static int microdvd_probe(AVProbeData *p)
> +{
> +    unsigned char c, *ptr = p->buf;
> +    int i;
> +
> +    if (AV_RB24(ptr) == 0xEFBBBF)
> +        ptr += 3;  /* skip UTF-8 BOM */
> +
> +    for (i=0; i<3; i++) {
> +        if (sscanf(ptr, "{%*d}{}%c",&c) != 1&&
> +            sscanf(ptr, "{%*d}{%*d}%c",&c) != 1&&
> +            sscanf(ptr, "{DEFAULT}{}%c",&c) != 1)

Using sscanf() looks unsafe - IIRC the probe buffer isn't null 
terminated. snscanf() doesn't seem to be standard though.

Also nit: space before &&

> +            return 0;
> +        ptr += strcspn(ptr, "\n") + 1;

Likewise.

> +    }
> +    return AVPROBE_SCORE_MAX;
> +}
> +
> +static int microdvd_read_header(AVFormatContext *s, AVFormatParameters *ap)
> +{
> +    AVRational pts_info = (AVRational){ 2997, 125 };  /* default: 23.976 fps */
> +    MicroDVDContext *microdvd = s->priv_data;
> +    AVStream *st = av_new_stream(s, 0);
> +    int i, frame;
> +    float fps;
> +    char c;
> +
> +    if (!st)
> +        return -1;
> +    for (i=0; i<FF_ARRAY_ELEMS(microdvd->lines); i++) {
> +        microdvd->pos[i] = avio_tell(s->pb);
> +        ff_get_line(s->pb, microdvd->lines[i], sizeof(microdvd->lines[i]));
> +        if ((sscanf(microdvd->lines[i], "{%d}{}%5f",&frame,&fps) == 2 ||
> +             sscanf(microdvd->lines[i], "{%d}{%*d}%5f",&frame,&fps) == 2)
> +&&  frame<= 1&&  fps>  3&&  fps<  100)

Uhm, really odd indent.. Also, why not frame > 1 or slightly strange 
frame rates?

> +            pts_info = av_d2q(fps, 100000);

Does this always do "the right thing", considering the limited precision 
of float? Maybe I'm nitpicking, but parsing %d.%d seems more stable.

> +        if (sscanf(microdvd->lines[i], "{DEFAULT}{}%c",&c) == 1) {
> +            st->codec->extradata = av_strdup(microdvd->lines[i] + 11);
> +            st->codec->extradata_size = strlen(st->codec->extradata);
> +            i--;
> +        }
> +    }
> +    av_set_pts_info(st, 64, pts_info.den, pts_info.num);
> +    st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> +    st->codec->codec_id   = CODEC_ID_MICRODVD;
> +    return 0;
> +}

The rest looks OK AFAICT, except perhaps for some weird formatting here 
and there.

/Tomas