flvdec: Only parse keyframe index when the underlying protocol allows seeking

Message ID 1302601403-98141-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö April 12, 2011, 9:43 a.m.
From: Michael Niedermayer <michaelni@gmx.at>

Reading the index currently requires seeking.
---
 libavformat/flvdec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Anton Khirnov April 12, 2011, 10:04 a.m. | #1
On Tue, Apr 12, 2011 at 12:43:23PM +0300, Martin Storsjö wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> Reading the index currently requires seeking.
> ---
>  libavformat/flvdec.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index 62d25c8..1a827fd 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -212,7 +212,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream, AVStream *vst
>          case AMF_DATA_TYPE_OBJECT: {
>              unsigned int keylen;
>  
> -            if (key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
> +            if (ioc->seekable && key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
>                  if (parse_keyframes_index(s, ioc, vstream, max_pos) < 0)
>                      return -1;
>  
> -- 
> 1.7.3.1
> 

Ok
Luca Barbato April 12, 2011, 10:18 a.m. | #2
On 04/12/2011 12:04 PM, Anton Khirnov wrote:
> On Tue, Apr 12, 2011 at 12:43:23PM +0300, Martin Storsjö wrote:
>> From: Michael Niedermayer <michaelni@gmx.at>
>>
>> Reading the index currently requires seeking.
>> ---
>>  libavformat/flvdec.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>> index 62d25c8..1a827fd 100644
>> --- a/libavformat/flvdec.c
>> +++ b/libavformat/flvdec.c
>> @@ -212,7 +212,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream, AVStream *vst
>>          case AMF_DATA_TYPE_OBJECT: {
>>              unsigned int keylen;
>>  
>> -            if (key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
>> +            if (ioc->seekable && key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
>>                  if (parse_keyframes_index(s, ioc, vstream, max_pos) < 0)
>>                      return -1;
>>  
>> -- 
>> 1.7.3.1
>>
> 
> Ok

s/reading/using. You should be able to read it w/out backward seeks.

lu
Martin Storsjö April 12, 2011, 10:57 a.m. | #3
On Tue, 12 Apr 2011, Luca Barbato wrote:

> On 04/12/2011 12:04 PM, Anton Khirnov wrote:
> > On Tue, Apr 12, 2011 at 12:43:23PM +0300, Martin Storsjö wrote:
> >> From: Michael Niedermayer <michaelni@gmx.at>
> >>
> >> Reading the index currently requires seeking.
> >> ---
> >>  libavformat/flvdec.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> >> index 62d25c8..1a827fd 100644
> >> --- a/libavformat/flvdec.c
> >> +++ b/libavformat/flvdec.c
> >> @@ -212,7 +212,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream, AVStream *vst
> >>          case AMF_DATA_TYPE_OBJECT: {
> >>              unsigned int keylen;
> >>  
> >> -            if (key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
> >> +            if (ioc->seekable && key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
> >>                  if (parse_keyframes_index(s, ioc, vstream, max_pos) < 0)
> >>                      return -1;
> >>  
> >> -- 
> >> 1.7.3.1
> >>
> > 
> > Ok
> 
> s/reading/using. You should be able to read it w/out backward seeks.

Yes, but you can't read the rest of the flv metadata reliably without some 
kind of seek at least (currently a backwards seek), see the other thread 
between me, Anton and Vladimir.

// Martin
Martin Storsjö April 12, 2011, 10:59 a.m. | #4
On Tue, 12 Apr 2011, Anton Khirnov wrote:

> On Tue, Apr 12, 2011 at 12:43:23PM +0300, Martin Storsjö wrote:
> > From: Michael Niedermayer <michaelni@gmx.at>
> > 
> > Reading the index currently requires seeking.
> > ---
> >  libavformat/flvdec.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> > index 62d25c8..1a827fd 100644
> > --- a/libavformat/flvdec.c
> > +++ b/libavformat/flvdec.c
> > @@ -212,7 +212,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream, AVStream *vst
> >          case AMF_DATA_TYPE_OBJECT: {
> >              unsigned int keylen;
> >  
> > -            if (key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
> > +            if (ioc->seekable && key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
> >                  if (parse_keyframes_index(s, ioc, vstream, max_pos) < 0)
> >                      return -1;
> >  
> > -- 
> > 1.7.3.1
> > 
> 
> Ok

Queued.

// Martin
Luca Barbato April 12, 2011, 11:24 a.m. | #5
On 4/12/11 12:57 PM, Martin Storsjö wrote:
> On Tue, 12 Apr 2011, Luca Barbato wrote:
>
>> On 04/12/2011 12:04 PM, Anton Khirnov wrote:
>>> On Tue, Apr 12, 2011 at 12:43:23PM +0300, Martin Storsjö wrote:
>>>> From: Michael Niedermayer<michaelni@gmx.at>
>>>>
>>>> Reading the index currently requires seeking.
>>>> ---
>>>>   libavformat/flvdec.c |    2 +-
>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>>>> index 62d25c8..1a827fd 100644
>>>> --- a/libavformat/flvdec.c
>>>> +++ b/libavformat/flvdec.c
>>>> @@ -212,7 +212,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream, AVStream *vst
>>>>           case AMF_DATA_TYPE_OBJECT: {
>>>>               unsigned int keylen;
>>>>
>>>> -            if (key&&  !strcmp(KEYFRAMES_TAG, key)&&  depth == 1)
>>>> +            if (ioc->seekable&&  key&&  !strcmp(KEYFRAMES_TAG, key)&&  depth == 1)
>>>>                   if (parse_keyframes_index(s, ioc, vstream, max_pos)<  0)
>>>>                       return -1;
>>>>
>>>> --
>>>> 1.7.3.1
>>>>
>>>
>>> Ok
>>
>> s/reading/using. You should be able to read it w/out backward seeks.
>
> Yes, but you can't read the rest of the flv metadata reliably without some
> kind of seek at least (currently a backwards seek), see the other thread
> between me, Anton and Vladimir.

I was about to reply but I had to run =\

Let's discuss there.

lu

Patch

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 62d25c8..1a827fd 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -212,7 +212,7 @@  static int amf_parse_object(AVFormatContext *s, AVStream *astream, AVStream *vst
         case AMF_DATA_TYPE_OBJECT: {
             unsigned int keylen;
 
-            if (key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
+            if (ioc->seekable && key && !strcmp(KEYFRAMES_TAG, key) && depth == 1)
                 if (parse_keyframes_index(s, ioc, vstream, max_pos) < 0)
                     return -1;