On Tue, Apr 12, 2011 at 11:58:44AM +0300, Martin Storsjö wrote:
> On Tue, 12 Apr 2011, Anton Khirnov wrote:
>
> > On Tue, Apr 12, 2011 at 11:41:13AM +0300, Martin Storsjö wrote:
> > > From: Kharkov Alexander <kharkovalexan...@gmail.com>
> > >
> > > Current keyframes data parser unconditionally rewind metadata to
> > > the end at the end of function. As result ALL metadata located
> > > after keyframes index not parsed, and as metadata object can have
> > > ANY placement inside metadata it can lead to unpredictable result
> > > (bitrate can not be found, etc.). As result FLV movie will not
> > > play at all in such situation.
> > > ---
> > > libavformat/flvdec.c | 3 ++-
> > > 1 files changed, 2 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> > > index f27b70c..5438ab0 100644
> > > --- a/libavformat/flvdec.c
> > > +++ b/libavformat/flvdec.c
> > > @@ -136,6 +136,7 @@ static int parse_keyframes_index(AVFormatContext *s,
> > > AVIOContext *ioc, AVStream
> > > int64_t *times = NULL;
> > > int64_t *filepositions = NULL;
> > > int ret = 0;
> > > + int64_t initial_pos = url_ftell(ioc);
> >
> > avio_tell()
>
> Fixed locally
>
> > >
> > > while (avio_tell(ioc) < max_pos - 2 && amf_get_string(ioc, str_val,
> > > sizeof(str_val)) > 0) {
> > > int64_t* current_array;
> > > @@ -183,7 +184,7 @@ static int parse_keyframes_index(AVFormatContext *s,
> > > AVIOContext *ioc, AVStream
> > > finish:
> > > av_freep(×);
> > > av_freep(&filepositions);
> > > - avio_seek(ioc, max_pos, SEEK_SET);
> > > + avio_seek(ioc, initial_pos, SEEK_SET);
> > > return ret;
> > > }
> > >
> >
> > This looks shady. Why should it rewind to the original position?
>
> Well, ideally it should probably seek to the end of the keyframes index
> (but at least not to the end of all the metadata), but since the end of
> the keyframes index isn't trivial to find (since this also is called when
> exiting this function, if any of the allocations failed halfway through),
> this at least seeks back to a good position, where the generic flv amr
> parser can continue to iterate through it all. Or is it trivial to
> calculate the end position of the keyframes index array? My amf knowledge
> is a bit rusty...
> Hmm, seems you're right. And here I was thinking that's a nice and not at all braindead format. > Another variant would be not to do any seek in either direction if > everything went well, at least, but it still requires seeking somewhere if > something failed. > That sounds like a better idea. -- Anton Khirnov
signature.asc
Description: Digital signature
_______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
