[FFmpeg-devel] [PATCH] rl2 format demuxer

[Sascha Sommer]

Hi,

> > +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()
>

Done and rest of the code changed to match your proposal.

> > +/**
> > + * 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
>

Removed

>
> [...]
>
> > +    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?
>

Hm I think these values might be unsigned after all. I fixed that and
changed this check to check for sizes that might cause heap overflows later.

> > +
> > +    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
>

Removed ;)


> > +        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 ...
>

Removed and stream init reordered so that the always present video comes 
first.

>
> > +    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
>

changed to 32

> > +
> > +    /** 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.
>

See above.

> > +
> > +    /** 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.
>

Should be fixed now.

Regards

Sascha



-------------- next part --------------
A non-text attachment was scrubbed...
Name: rl2_demuxer_try2.patch
Type: text/x-diff
Size: 9888 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080316/168389b6/attachment.patch>