diff mbox

[FFmpeg-devel] avformat/concatdec: don't call open_file when seek position within a file

Message ID 1474345876-95480-1-git-send-email-raymondzheng1412@gmail.com
State Changes Requested
Headers show

Commit Message

raymond Sept. 20, 2016, 4:31 a.m. UTC
---
 libavformat/concatdec.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

Comments

raymond Sept. 20, 2016, 7:04 a.m. UTC | #1
Submit reasons:

I have enabled concat and async protocol,but ffmpeg will flush async buffer
when doing seek. I don't want flush async buffer when doing seek within the
same segment file.
I found, the concatdev will call open_file everytime I try to do seek,
which will cause call async_open,and then flush async buffer.


Solution:

I will judge if seek range in the same segment file, if so, I‘ll only call
try_seek.


2016-09-20 12:31 GMT+08:00 raymond <raymondzheng1412@gmail.com>:

> ---
>  libavformat/concatdec.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
> index b3a430e..1cd9ec1 100644
> --- a/libavformat/concatdec.c
> +++ b/libavformat/concatdec.c
> @@ -64,6 +64,8 @@ typedef struct {
>      ConcatMatchMode stream_match_mode;
>      unsigned auto_convert;
>      int segment_time_metadata;
> +    int cur_fileno;
> +    AVFormatContext *cur_avf_saved;
>  } ConcatContext;
>
>  static int concat_probe(AVProbeData *probe)
> @@ -333,6 +335,8 @@ static int open_file(AVFormatContext *avf, unsigned
> fileno)
>          avformat_close_input(&cat->avf);
>          return ret;
>      }
> +
> +    cat->cur_fileno = fileno;
>      cat->cur_file = file;
>      if (file->start_time == AV_NOPTS_VALUE)
>          file->start_time = !fileno ? 0 :
> @@ -711,13 +715,18 @@ static int real_seek(AVFormatContext *avf, int
> stream,
>              left  = mid;
>      }
>
> -    if ((ret = open_file(avf, left)) < 0)
> -        return ret;
> +    if (cat->cur_fileno != left) {
> +        if ((ret = open_file(avf, left)) < 0)
> +            return ret;
> +    } else {
> +        cat->avf = cat->cur_avf_saved;
> +    }
>
>      ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
>      if (ret < 0 &&
>          left < cat->nb_files - 1 &&
>          cat->files[left + 1].start_time < max_ts) {
> +        cat->avf = NULL;
>          if ((ret = open_file(avf, left + 1)) < 0)
>              return ret;
>          ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
> @@ -730,7 +739,7 @@ static int concat_seek(AVFormatContext *avf, int
> stream,
>  {
>      ConcatContext *cat = avf->priv_data;
>      ConcatFile *cur_file_saved = cat->cur_file;
> -    AVFormatContext *cur_avf_saved = cat->avf;
> +    cat->cur_avf_saved = cat->avf;
>      int ret;
>
>      if (!cat->seekable)
> @@ -739,12 +748,16 @@ static int concat_seek(AVFormatContext *avf, int
> stream,
>          return AVERROR(ENOSYS);
>      cat->avf = NULL;
>      if ((ret = real_seek(avf, stream, min_ts, ts, max_ts, flags)) < 0) {
> -        if (cat->avf)
> -            avformat_close_input(&cat->avf);
> -        cat->avf      = cur_avf_saved;
> +        if (cat->cur_file != cur_file_saved) {
> +            if (cat->avf)
> +                avformat_close_input(&cat->avf);
> +        }
> +        cat->avf      = cat->cur_avf_saved;
>          cat->cur_file = cur_file_saved;
>      } else {
> -        avformat_close_input(&cur_avf_saved);
> +        if (cat->cur_file != cur_file_saved) {
> +            avformat_close_input(&cat->cur_avf_saved);
> +        }
>          cat->eof = 0;
>      }
>      return ret;
> --
> 2.7.4
>
>
raymond Sept. 22, 2016, 2:02 a.m. UTC | #2
ping...

2016-09-20 15:04 GMT+08:00 raymond zheng <raymondzheng1412@gmail.com>:

> Submit reasons:
>
> I have enabled concat and async protocol,but ffmpeg will flush async
> buffer when doing seek. I don't want flush async buffer when doing seek
> within the same segment file.
> I found, the concatdev will call open_file everytime I try to do seek,
> which will cause call async_open,and then flush async buffer.
>
>
> Solution:
>
> I will judge if seek range in the same segment file, if so, I‘ll only call
> try_seek.
>
>
> 2016-09-20 12:31 GMT+08:00 raymond <raymondzheng1412@gmail.com>:
>
>> ---
>>  libavformat/concatdec.c | 27 ++++++++++++++++++++-------
>>  1 file changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
>> index b3a430e..1cd9ec1 100644
>> --- a/libavformat/concatdec.c
>> +++ b/libavformat/concatdec.c
>> @@ -64,6 +64,8 @@ typedef struct {
>>      ConcatMatchMode stream_match_mode;
>>      unsigned auto_convert;
>>      int segment_time_metadata;
>> +    int cur_fileno;
>> +    AVFormatContext *cur_avf_saved;
>>  } ConcatContext;
>>
>>  static int concat_probe(AVProbeData *probe)
>> @@ -333,6 +335,8 @@ static int open_file(AVFormatContext *avf, unsigned
>> fileno)
>>          avformat_close_input(&cat->avf);
>>          return ret;
>>      }
>> +
>> +    cat->cur_fileno = fileno;
>>      cat->cur_file = file;
>>      if (file->start_time == AV_NOPTS_VALUE)
>>          file->start_time = !fileno ? 0 :
>> @@ -711,13 +715,18 @@ static int real_seek(AVFormatContext *avf, int
>> stream,
>>              left  = mid;
>>      }
>>
>> -    if ((ret = open_file(avf, left)) < 0)
>> -        return ret;
>> +    if (cat->cur_fileno != left) {
>> +        if ((ret = open_file(avf, left)) < 0)
>> +            return ret;
>> +    } else {
>> +        cat->avf = cat->cur_avf_saved;
>> +    }
>>
>>      ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
>>      if (ret < 0 &&
>>          left < cat->nb_files - 1 &&
>>          cat->files[left + 1].start_time < max_ts) {
>> +        cat->avf = NULL;
>>          if ((ret = open_file(avf, left + 1)) < 0)
>>              return ret;
>>          ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
>> @@ -730,7 +739,7 @@ static int concat_seek(AVFormatContext *avf, int
>> stream,
>>  {
>>      ConcatContext *cat = avf->priv_data;
>>      ConcatFile *cur_file_saved = cat->cur_file;
>> -    AVFormatContext *cur_avf_saved = cat->avf;
>> +    cat->cur_avf_saved = cat->avf;
>>      int ret;
>>
>>      if (!cat->seekable)
>> @@ -739,12 +748,16 @@ static int concat_seek(AVFormatContext *avf, int
>> stream,
>>          return AVERROR(ENOSYS);
>>      cat->avf = NULL;
>>      if ((ret = real_seek(avf, stream, min_ts, ts, max_ts, flags)) < 0) {
>> -        if (cat->avf)
>> -            avformat_close_input(&cat->avf);
>> -        cat->avf      = cur_avf_saved;
>> +        if (cat->cur_file != cur_file_saved) {
>> +            if (cat->avf)
>> +                avformat_close_input(&cat->avf);
>> +        }
>> +        cat->avf      = cat->cur_avf_saved;
>>          cat->cur_file = cur_file_saved;
>>      } else {
>> -        avformat_close_input(&cur_avf_saved);
>> +        if (cat->cur_file != cur_file_saved) {
>> +            avformat_close_input(&cat->cur_avf_saved);
>> +        }
>>          cat->eof = 0;
>>      }
>>      return ret;
>> --
>> 2.7.4
>>
>>
>
Nicolas George Sept. 22, 2016, 7:52 a.m. UTC | #3
Le jour de la Récompense, an CCXXIV, raymond a écrit :
> ---
>  libavformat/concatdec.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)

Thanks for the patch. The object of the change is interesting, but there are
some implementation concerns, see below.

Also, remember that top-posting is not accepted on this list; if you do not
know what it means, look it up.

> 
> diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
> index b3a430e..1cd9ec1 100644
> --- a/libavformat/concatdec.c
> +++ b/libavformat/concatdec.c
> @@ -64,6 +64,8 @@ typedef struct {
>      ConcatMatchMode stream_match_mode;
>      unsigned auto_convert;
>      int segment_time_metadata;

> +    int cur_fileno;

This information is already available as "cur_file - files", see
open_next_file() for example. There is no need for an extra variable.

> +    AVFormatContext *cur_avf_saved;

This variable serves only locally for concat_seek() and real_seek(), it
would be better as just a parameter to real_seek().

>  } ConcatContext;
>  
>  static int concat_probe(AVProbeData *probe)
> @@ -333,6 +335,8 @@ static int open_file(AVFormatContext *avf, unsigned fileno)
>          avformat_close_input(&cat->avf);
>          return ret;
>      }
> +
> +    cat->cur_fileno = fileno;
>      cat->cur_file = file;
>      if (file->start_time == AV_NOPTS_VALUE)
>          file->start_time = !fileno ? 0 :
> @@ -711,13 +715,18 @@ static int real_seek(AVFormatContext *avf, int stream,
>              left  = mid;
>      }
>  
> -    if ((ret = open_file(avf, left)) < 0)
> -        return ret;
> +    if (cat->cur_fileno != left) {
> +        if ((ret = open_file(avf, left)) < 0)
> +            return ret;
> +    } else {
> +        cat->avf = cat->cur_avf_saved;
> +    }
>  
>      ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
>      if (ret < 0 &&
>          left < cat->nb_files - 1 &&
>          cat->files[left + 1].start_time < max_ts) {

> +        cat->avf = NULL;

This leaks if cat->avf is not cur_avf_saved, I think.

>          if ((ret = open_file(avf, left + 1)) < 0)
>              return ret;
>          ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
> @@ -730,7 +739,7 @@ static int concat_seek(AVFormatContext *avf, int stream,
>  {
>      ConcatContext *cat = avf->priv_data;
>      ConcatFile *cur_file_saved = cat->cur_file;
> -    AVFormatContext *cur_avf_saved = cat->avf;

> +    cat->cur_avf_saved = cat->avf;
>      int ret;

Interleaving statements and variables is not allowed in the code base.

>  
>      if (!cat->seekable)
> @@ -739,12 +748,16 @@ static int concat_seek(AVFormatContext *avf, int stream,
>          return AVERROR(ENOSYS);
>      cat->avf = NULL;
>      if ((ret = real_seek(avf, stream, min_ts, ts, max_ts, flags)) < 0) {
> -        if (cat->avf)
> -            avformat_close_input(&cat->avf);
> -        cat->avf      = cur_avf_saved;
> +        if (cat->cur_file != cur_file_saved) {
> +            if (cat->avf)
> +                avformat_close_input(&cat->avf);
> +        }
> +        cat->avf      = cat->cur_avf_saved;
>          cat->cur_file = cur_file_saved;
>      } else {
> -        avformat_close_input(&cur_avf_saved);
> +        if (cat->cur_file != cur_file_saved) {
> +            avformat_close_input(&cat->cur_avf_saved);
> +        }
>          cat->eof = 0;
>      }
>      return ret;

Regards,
diff mbox

Patch

diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index b3a430e..1cd9ec1 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -64,6 +64,8 @@  typedef struct {
     ConcatMatchMode stream_match_mode;
     unsigned auto_convert;
     int segment_time_metadata;
+    int cur_fileno;
+    AVFormatContext *cur_avf_saved;
 } ConcatContext;
 
 static int concat_probe(AVProbeData *probe)
@@ -333,6 +335,8 @@  static int open_file(AVFormatContext *avf, unsigned fileno)
         avformat_close_input(&cat->avf);
         return ret;
     }
+
+    cat->cur_fileno = fileno;
     cat->cur_file = file;
     if (file->start_time == AV_NOPTS_VALUE)
         file->start_time = !fileno ? 0 :
@@ -711,13 +715,18 @@  static int real_seek(AVFormatContext *avf, int stream,
             left  = mid;
     }
 
-    if ((ret = open_file(avf, left)) < 0)
-        return ret;
+    if (cat->cur_fileno != left) {
+        if ((ret = open_file(avf, left)) < 0)
+            return ret;
+    } else {
+        cat->avf = cat->cur_avf_saved;
+    }
 
     ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
     if (ret < 0 &&
         left < cat->nb_files - 1 &&
         cat->files[left + 1].start_time < max_ts) {
+        cat->avf = NULL;
         if ((ret = open_file(avf, left + 1)) < 0)
             return ret;
         ret = try_seek(avf, stream, min_ts, ts, max_ts, flags);
@@ -730,7 +739,7 @@  static int concat_seek(AVFormatContext *avf, int stream,
 {
     ConcatContext *cat = avf->priv_data;
     ConcatFile *cur_file_saved = cat->cur_file;
-    AVFormatContext *cur_avf_saved = cat->avf;
+    cat->cur_avf_saved = cat->avf;
     int ret;
 
     if (!cat->seekable)
@@ -739,12 +748,16 @@  static int concat_seek(AVFormatContext *avf, int stream,
         return AVERROR(ENOSYS);
     cat->avf = NULL;
     if ((ret = real_seek(avf, stream, min_ts, ts, max_ts, flags)) < 0) {
-        if (cat->avf)
-            avformat_close_input(&cat->avf);
-        cat->avf      = cur_avf_saved;
+        if (cat->cur_file != cur_file_saved) {
+            if (cat->avf)
+                avformat_close_input(&cat->avf);
+        }
+        cat->avf      = cat->cur_avf_saved;
         cat->cur_file = cur_file_saved;
     } else {
-        avformat_close_input(&cur_avf_saved);
+        if (cat->cur_file != cur_file_saved) {
+            avformat_close_input(&cat->cur_avf_saved);
+        }
         cat->eof = 0;
     }
     return ret;