[FFmpeg-devel] [PATCH] avformat/fivdec: cached keyframes before video or audio stream was created

[XinZheng Zhang]

On Wed, Jul 27, 2016 at 1:12 AM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Tue, Jul 26, 2016 at 08:17:46PM +0800, Xinzheng Zhang wrote:
>> ---
>>  libavformat/flvdec.c | 84 ++++++++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 69 insertions(+), 15 deletions(-)
>>
>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>> index 2bf1e05..82b9f83 100644
>> --- a/libavformat/flvdec.c
>> +++ b/libavformat/flvdec.c
>> @@ -61,6 +61,12 @@ typedef struct FLVContext {
>>
>>      int broken_sizes;
>>      int sum_flv_tag_size;
>> +
>> +    int vhead_exists;
>> +    int ahead_exists;
>> +    int keyframe_count;
>> +    int64_t *keyframe_times;
>> +    int64_t *keyframe_filepositions;
>>  } FLVContext;
>>
>>  static int probe(AVProbeData *p, int live)
>> @@ -92,6 +98,42 @@ static int live_flv_probe(AVProbeData *p)
>>      return probe(p, 1);
>>  }
>>
>> +static void add_keyframes_index(AVFormatContext *s)
>> +{
>> +    FLVContext *flv           = s->priv_data;
>> +    AVStream *current_stream  = NULL;
>> +    AVStream *vstream         = NULL;
>> +    AVStream *astream         = NULL;
>> +    unsigned int i,j                ;
>> +
>> +    for (i = 0; i< s->nb_streams; i++) {
>> +        if (s->streams[i]->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
>> +            vstream = s->streams[i];
>> +        } else if (s->streams[i]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
>> +            astream = s->streams[i];
>> +        }
>> +    }
>
> this shouldt be needed
> just keep track of the stram index that the table belongs to
> that is add a flv->keyframe_stream_index
>
>
ok

>> +
>> +    AVStream *streams[2] = {vstream, astream};
>> +    for (i = 0; i < 2; i++) {
>> +        current_stream = streams[i];
>> +        if (current_stream && current_stream->nb_index_entries==0) {
>> +            for (j = 0; j < flv->keyframe_count; j++) {
>> +                av_add_index_entry(current_stream, flv->keyframe_filepositions[j], flv->keyframe_times[j] * 1000,
>> +                                   0, 0, AVINDEX_KEYFRAME);
>> +            }
>> +        }
>> +    }
>> +
>> +    // free keyframe index only if all expected streams have been created
>> +    if (((vstream && vstream->nb_index_entries>0) || !flv->vhead_exists) &&
>> +        ((astream && astream->nb_index_entries>0) || !flv->ahead_exists)) {
>> +        av_freep(&flv->keyframe_times);
>> +        av_freep(&flv->keyframe_filepositions);
>> +        flv->keyframe_count = 0;
>> +    }
>> +}
>
> spliting add_keyframes_index() out must be in a seperate patch
>
> also i would not trust the *head_exists flags, IIRC they can be
> wrong and they are not needed
> the function should just take the table load it with
> av_add_index_entry() and free the table.
> The rest should not be needed
> should be much simpler unless iam missing something
>
>

If I don't trust the head_exists flags, when should I free the index table?
Should I keep the index util both a\v stream have been loaded or keep
it util the flv_read_close().

> [...]
>> @@ -305,8 +348,7 @@ static int amf_get_string(AVIOContext *ioc, char *buffer, int buffsize)
>>      return length;
>>  }
>>
>> -static int parse_keyframes_index(AVFormatContext *s, AVIOContext *ioc,
>> -                                 AVStream *vstream, int64_t max_pos)
>> +static int parse_keyframes_index(AVFormatContext *s, AVIOContext *ioc, int64_t max_pos)
>>  {
>>      FLVContext *flv       = s->priv_data;
>>      unsigned int timeslen = 0, fileposlen = 0, i;
>> @@ -316,7 +358,7 @@ static int parse_keyframes_index(AVFormatContext *s, AVIOContext *ioc,
>>      int ret                = AVERROR(ENOSYS);
>>      int64_t initial_pos    = avio_tell(ioc);
>>
>> -    if (vstream->nb_index_entries>0) {
>> +    if (flv->keyframe_count > 0) {
>>          av_log(s, AV_LOG_WARNING, "Skipping duplicate index\n");
>>          return 0;
>>      }
>
> this looks wrong, a file can contain an index for each stream
> that shouldnt trigger a warning, only if there are 2 for the same
> stream its a duplicate
>
>
ok

> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> What does censorship reveal? It reveals fear. -- Julian Assange
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>