[FFmpeg-devel] [PATCH] Check return value of avio_seek and avoid modifying

Michael Niedermayer michaelni at gmx.at
Wed Sep 14 16:33:52 CEST 2011


On Tue, Sep 13, 2011 at 12:25:23AM +0200, Joakim Plate wrote:
> Since i was working through the read_seek implementations i did a once over
> for checking return values for read_seek and avoiding modifying state.
> 
> I suppose it should be split into separate patches for each demuxer.
> Atleast for the ones moving code around abit.
> 
> fate still passes, even thou it probably would have been better if there
> was some test's for situations like these (files with index in front but
> cut short for example).

yes, please split before commit (i can do it too if you prefer)



[...]
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 3a5940b..7564c2d 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c

left to aurel


[...]
> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> index a9a992a..d01a106 100644
> --- a/libavformat/nutdec.c
> +++ b/libavformat/nutdec.c
> @@ -897,7 +897,8 @@ static int read_seek(AVFormatContext *s, int stream_index, int64_t pts, int flag
>      }
>      av_log(NULL, AV_LOG_DEBUG, "SEEKTO: %"PRId64"\n", pos2);
>      pos= find_startcode(s->pb, SYNCPOINT_STARTCODE, pos2);
> -    avio_seek(s->pb, pos, SEEK_SET);
> +    if (avio_seek(s->pb, pos, SEEK_SET) < 0)
> +        return -1;
>      av_log(NULL, AV_LOG_DEBUG, "SP: %"PRId64"\n", pos);
>      if(pos2 > pos || pos2 + 15 < pos){
>          av_log(NULL, AV_LOG_ERROR, "no syncpoint at backptr pos\n");

this will not work, the file position will likely have changed prior
to this failing seek (though failure of this seek without prior failure
is probably a bit unlikely)


[...]
> diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
> index e27fa14..214d7b1 100644
> --- a/libavformat/wtvdec.c
> +++ b/libavformat/wtvdec.c
> @@ -998,18 +998,21 @@ static int read_seek(AVFormatContext *s, int stream_index,
>      i = ff_index_search_timestamp(wtv->index_entries, wtv->nb_index_entries, ts_relative, flags);
>      if (i < 0) {
>          if (wtv->last_valid_pts == AV_NOPTS_VALUE || ts < wtv->last_valid_pts)
> -            avio_seek(pb, 0, SEEK_SET);
> +            if (avio_seek(pb, 0, SEEK_SET) < 0)
> +                return -1;
>          else if (st->duration != AV_NOPTS_VALUE && ts_relative > st->duration && wtv->nb_index_entries)
> -            avio_seek(pb, wtv->index_entries[wtv->nb_index_entries - 1].pos, SEEK_SET);
> +            if (avio_seek(pb, wtv->index_entries[wtv->nb_index_entries - 1].pos, SEEK_SET) < 0)
> +                return -1;

this needs some {}

[...]


The rest LGTM, please push them & thanks

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110914/2357ded1/attachment.asc>


More information about the ffmpeg-devel mailing list