[FFmpeg-devel] [PATCH] rl2 format demuxer

[Michael Niedermayer]

On Sat, Mar 15, 2008 at 10:08:53PM +0100, Sascha Sommer wrote:
> Hi,
> 
> attached patch adds support for the rl2 format as described on
> http://wiki.multimedia.cx/index.php?title=RL2
> It depends on the rl2 decoder patch that I sent earlier
> 
[...]
> +typedef struct {
> +    int stream_index; ///< audio or video stream index
> +    uint32_t offset;  ///< chunk offset relative to the file start 
> +    uint32_t size;    ///< size of the chunk
> +    int64_t pts;      ///< presentation timestamp 
> +} rl2_sample_t;

You should use  AVStream.index_entries / av_add_index_entry()


[...]
> +/**
> + * check if the file is in rl2 format
> + * @param p probe buffer
> + * @return 0 when the probe buffer does not contain rl2 data, > 0 otherwise
> + */
> +static int rl2_probe(AVProbeData *p)
> +{

> +    if(p->buf_size < 12)
> +        return 0;

unneeded see AVPROBE_PADDING_SIZE


[...]
> +    back_size = get_le32(pb); /** get size of the background frame */ 
> +    signature = get_be32(pb);
> +    data_size = get_be32(pb);
> +    frame_count = get_le32(pb);
> +

> +    if(back_size < 0 || frame_count < 0)
> +        return AVERROR_INVALIDDATA;

These checks depend on the size of int, is that intended?


> +
> +    encoding_method = get_le16(pb);
> +    sound_rate = get_le16(pb);
> +    rate = get_le16(pb);
> +    channels = get_le16(pb);
> +    def_sound_size = get_le16(pb);
> +
> +    /** setup audio stream if present */

> +    if(sound_rate != 0){

superflous != 0


> +        pts_num = def_sound_size;
> +        pts_den = rate;
> +
> +        st = av_new_stream(s, 0);
> +        if (!st)
> +            return AVERROR(ENOMEM);

> +        audio_stream_index = st->index;

Thats guranteed to be 0 here so theres no sense in storing it ...


> +        st->codec->codec_type = CODEC_TYPE_AUDIO;
> +        st->codec->codec_id = CODEC_ID_PCM_U8;
> +        st->codec->codec_tag = 1;
> +        st->codec->channels = channels;
> +        st->codec->bits_per_sample = 8;
> +        st->codec->sample_rate = rate;
> +        st->codec->bit_rate = st->codec->channels * st->codec->sample_rate *
> +            st->codec->bits_per_sample;
> +        st->codec->block_align = st->codec->channels *
> +            st->codec->bits_per_sample / 8;
> +    }
> +
> +    /** setup video stream */
> +    st = av_new_stream(s, 0);
> +    if(!st)
> +         return AVERROR(ENOMEM);
> +

> +    video_stream_index = st->index;

If you init vieo first then it would be guranteed to be 0 and audio would
be guranteed to be 1 if there is audio.


[...]
> +    st->codec->extradata_size = EXTRADATA1_SIZE + back_size;
> +    st->codec->extradata = av_mallocz(st->codec->extradata_size + FF_INPUT_BUFFER_PADDING_SIZE); 
> +    if(!st->codec->extradata)
> +        return AVERROR(ENOMEM);
> +
> +    if(get_buffer(pb,st->codec->extradata,EXTRADATA1_SIZE) != EXTRADATA1_SIZE)
> +        return AVERROR(EIO);
> +
> +    /** get background frame */
> +    if(signature == RLV3_TAG && back_size > 0){
> +        if(get_buffer(pb,st->codec->extradata + EXTRADATA1_SIZE,back_size) != back_size)
> +            return AVERROR(EIO);
> +    }
> +

> +    for(i=0; i<s->nb_streams; i++)
> +        av_set_pts_info(s->streams[i], 33, pts_num, pts_den);

33 is wrong



> +
> +    /** allocate sample_table: max 1 audio and 1 video sample per frame */
> +    rl2->sample_table = av_malloc(frame_count * 2 * sizeof(rl2_sample_t));
> +
> +    if(!rl2->sample_table)
> +        return AVERROR(ENOMEM);
> +
> +    chunk_size = av_malloc(frame_count * sizeof(uint32_t));
> +    audio_size = av_malloc(frame_count * sizeof(uint32_t));
> +    chunk_offset = av_malloc(frame_count * sizeof(uint32_t));
> +
> +    if(!chunk_size || !audio_size || !chunk_offset)
> +        return AVERROR(ENOMEM);
> +
> +    /** read offset and size tables */
> +    for(i=0; i < frame_count;i++)
> +        chunk_size[i] = get_le32(pb);
> +    for(i=0; i < frame_count;i++)
> +        chunk_offset[i] = get_le32(pb);
> +    for(i=0; i < frame_count;i++)
> +        audio_size[i] = get_le32(pb) & 0xFFFF;

Several heap overflows.


> +
> +    /** build the sample index */
> +    for(i=0;i<frame_count;i++){
> +        if(sound_rate != 0 && audio_size[i] > 0){
> +            rl2->sample_table[rl2->sample_count].stream_index = audio_stream_index;
> +            rl2->sample_table[rl2->sample_count].offset = chunk_offset[i];
> +            rl2->sample_table[rl2->sample_count].size = audio_size[i];
> +            rl2->sample_table[rl2->sample_count].pts = audio_frame_counter;
> +            rl2->sample_table[rl2->sample_count].pts *= def_sound_size;
> +            rl2->sample_table[rl2->sample_count].pts /= rate;

Is rl2->sample_table[rl2->sample_count].pts % rate == 0 here, that is is the
division exact? If not this calculation is wrong.

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

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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/20080316/54e32a73/attachment.pgp>