[Ffmpeg-devel] Re: [PATCH] GXF muxer

[Michael Niedermayer]

Hi

On Mon, Jul 17, 2006 at 04:23:32PM +0200, Baptiste Coudurier wrote:
[...]

> 
> > [...]
> >> +            if (sc->codec->codec_id == CODEC_ID_MPEG2VIDEO) {
> >> +                int p = 0, b = 0;
> >> +                
> >> +                assert(sc->i_per_gop);
> > 
> > can this assert fail with some input, if so its unaccpetable
> 
> It could if the mpeg-2 stream would contain no I-Frame, is that possible ?

why not? people can feed anything to ffmpeg, /dev/random, a damaged or
truncated stream, one which might be intentionally designed to trigger a
specific behavior (denial of service in that case ...)


[...]
> +static int gxf_write_umf_track_description(ByteIOContext *pb, GXFContext *ctx)
> +{
> +    char mpeg1_tracks, mpeg2_tracks, dv_tracks, audio_tracks;
> +    offset_t pos;
> +    int i;
> +
> +    pos = url_ftell(pb);
> +    ctx->umf_track_offset = pos - ctx->umf_start_offset;
> +    audio_tracks = '0';
> +    mpeg1_tracks = '0';
> +    mpeg2_tracks = '0';
> +    dv_tracks = '0';
> +    for (i = 0; i < ctx->fc->nb_streams; ++i) {
> +        AVStream *st = ctx->fc->streams[i];
> +        GXFStreamContext *sc = &ctx->streams[i];
> +
> +        switch (st->codec->codec_id) {
> +        case CODEC_ID_MPEG1VIDEO:
> +            sc->media_info = 'L' << 8;
> +            sc->media_info |= mpeg1_tracks++;
> +            break;
> +        case CODEC_ID_MPEG2VIDEO:
> +            sc->media_info = 'M' << 8;
> +            sc->media_info |= mpeg2_tracks++;
> +            break;
> +        case CODEC_ID_PCM_S16LE:
> +            sc->media_info = 'A' << 8;
> +            sc->media_info |= audio_tracks++;
> +            if (audio_tracks == ':')
> +                audio_tracks = 'A'; /* first 10 audio tracks are 0 to 9 next 22 are A to V */
> +            break;
> +        case CODEC_ID_DVVIDEO:
> +            sc->media_info = 'D' << 8;
> +            sc->media_info |= dv_tracks++;
> +            break;
> +        case CODEC_ID_MJPEG:
> +            sc->media_info = 'V' << 8;
> +            sc->media_info |= '0';
> +            break;
> +        default:
> +            sc->media_info = 'U' << 8;
> +            sc->media_info |= '0';
> +        }

one simplification idea:

    int tracks[255]={0};
    for (i = 0; i < ctx->fc->nb_streams; ++i) {
        AVStream *st = ctx->fc->streams[i];
        GXFStreamContext *sc = &ctx->streams[i];
        int id;

        switch (st->codec->codec_id) {
        case CODEC_ID_MPEG1VIDEO: id= 'L'; break;
        case CODEC_ID_MPEG2VIDEO: id= 'M'; break;
        case CODEC_ID_PCM_S16LE : id= 'A'; break;
        ...
        }
        sc->media_info= id << 8;
        sc->media_info |= '0' + (tracks[id]++);
    }

except these ive no objections, feel free to commit it ...

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

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is