On Tue, 12 Apr 2011, Anton Khirnov wrote:
> On Tue, Apr 12, 2011 at 11:41:13AM +0300, Martin Storsjö wrote:
> > From: Kharkov Alexander <[email protected]>
> >
> > 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...
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.
// Martin_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
