[FFmpeg-devel] [PATCH v3 0/1] avformat/demux: Add durationprobesize AVOption

[FFmpeg-devel] [PATCH v3 0/1] avformat/demux: Add durationprobesize AVOption

[Nicolas Gaullier]

Ping ?

v3 "bis" rebased and wrapped lines in commit msg

Nicolas Gaullier (1):
  avformat/demux: Add durationprobesize AVOption

 doc/APIchanges              |  3 +++
 doc/formats.texi            | 16 +++++++++++++++-
 libavformat/avformat.h      | 12 ++++++++++++
 libavformat/demux.c         | 13 ++++++++-----
 libavformat/options_table.h |  1 +
 libavformat/version.h       |  2 +-
 6 files changed, 40 insertions(+), 7 deletions(-)

-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 1/1] avformat/demux: Add durationprobesize AVOption

[Nicolas Gaullier]

Yet another probesize used to get the durations when
estimate_timings_from_pts is required. It is aimed at users interested
in better durations probing for itself, or because using
avformat_find_stream_info indirectly and requiring exact values: for
concatdec for example, especially if streamcopying above it.
The current code is a performance trade-off that can fail to get video
stream durations in a scenario with high bitrates and buffering for
files ending cleanly (as opposed to live captures): the physical gap
between the last video packet and the last audio packet is very high in
such a case.

Default behaviour is unchanged: 250k up to 250k << 6 (step by step).
Setting this new option has two effects:
- override the maximum probesize (currently 250k << 6)
- reduce the number of steps to 1 instead of 6, this is to avoid
detecting the audio "too early" and failing to reach a video packet.
Even if a single audio stream duration is found but not the other
audio/video stream durations, there will be a retry, so at the end the
full user-overriden probesize will be used as expected by the user.

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 doc/APIchanges              |  3 +++
 doc/formats.texi            | 16 +++++++++++++++-
 libavformat/avformat.h      | 12 ++++++++++++
 libavformat/demux.c         | 13 ++++++++-----
 libavformat/options_table.h |  1 +
 libavformat/version.h       |  2 +-
 6 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index cf58c8c5f0..c97acf60d1 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
 
 API changes, most recent first:
 
+2024-03-11 - xxxxxxxxxx - lavf 61.1.100 - avformat.h
+  Add AVFormatContext.duration_probesize
+
 2024-03-08 - xxxxxxxxxx - lavc 61.1.100 - avcodec.h
   Add AVCodecContext.[nb_]side_data_prefer_packet.
 
diff --git a/doc/formats.texi b/doc/formats.texi
index 69fc1457a4..9cada03a6e 100644
--- a/doc/formats.texi
+++ b/doc/formats.texi
@@ -225,9 +225,23 @@ Specifies the maximum number of streams. This can be used to reject files that
 would require too many resources due to a large number of streams.
 
 @item skip_estimate_duration_from_pts @var{bool} (@emph{input})
-Skip estimation of input duration when calculated using PTS.
+Skip estimation of input duration if it requires an additional probing for pts at end of file.
 At present, applicable for MPEG-PS and MPEG-TS.
 
+@item durationprobesize @var{integer} (@emph{input})
+Set probing size for input duration estimation when it actually requires an additional probing
+for pts at end of file.
+At present, applicable for MPEG-PS and MPEG-TS.
+It is aimed at users interested in better durations probing for itself, or indirectly
+for specific use cases like using the concat demuxer.
+Files with high bitrates and ending cleanly (as opposed to live captures), can lead
+to a large physical gap between the last video packet and the last audio packet,
+so many bytes have to be read in order to get a video stream duration.
+Setting this value has a performance impact even for small files because the probing size is fixed.
+Default behaviour is a trade-off, largely adaptive: the probing size may range from
+250K up to 16M, but it is not extended to get streams durations at all costs.
+Must be an integer not lesser than 1, or 0 for default behaviour.
+
 @item strict, f_strict @var{integer} (@emph{input/output})
 Specify how strictly to follow the standards. @code{f_strict} is deprecated and
 should be used only via the @command{ffmpeg} tool.
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index de40397676..9042a62b70 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1667,6 +1667,8 @@ typedef struct AVFormatContext {
      * Skip duration calcuation in estimate_timings_from_pts.
      * - encoding: unused
      * - decoding: set by user
+     *
+     * @sa duration_probesize
      */
     int skip_estimate_duration_from_pts;
 
@@ -1870,6 +1872,16 @@ typedef struct AVFormatContext {
      * @return 0 on success, a negative AVERROR code on failure
      */
     int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
+
+    /**
+     * Maximum number of bytes read from input in order to determine stream durations
+     * when using estimate_timings_from_pts in avformat_find_stream_info().
+     * Demuxing only, set by the caller before avformat_find_stream_info().
+     * Can be set to 0 to let avformat choose using a heuristic.
+     *
+     * @sa skip_estimate_duration_from_pts
+     */
+    int64_t duration_probesize;
 } AVFormatContext;
 
 /**
diff --git a/libavformat/demux.c b/libavformat/demux.c
index 4c50eb5568..68b0c51720 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1817,8 +1817,9 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic)
                "Estimating duration from bitrate, this may be inaccurate\n");
 }
 
-#define DURATION_MAX_READ_SIZE 250000LL
-#define DURATION_MAX_RETRY 6
+#define DURATION_MAX_READ_SIZE_DEFAULT 250000LL
+#define DURATION_MAX_RETRY_DEFAULT 6
+#define DURATION_MAX_RETRY_USER 1
 
 /* only usable for MPEG-PS streams */
 static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
@@ -1826,6 +1827,8 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
     FFFormatContext *const si = ffformatcontext(ic);
     AVPacket *const pkt = si->pkt;
     int num, den, read_size, ret;
+    int64_t duration_max_read_size = ic->duration_probesize ? ic->duration_probesize >> DURATION_MAX_RETRY_USER : DURATION_MAX_READ_SIZE_DEFAULT;
+    int duration_max_retry = ic->duration_probesize ? DURATION_MAX_RETRY_USER : DURATION_MAX_RETRY_DEFAULT;
     int found_duration = 0;
     int is_end;
     int64_t filesize, offset, duration;
@@ -1861,7 +1864,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
     filesize = ic->pb ? avio_size(ic->pb) : 0;
     do {
         is_end = found_duration;
-        offset = filesize - (DURATION_MAX_READ_SIZE << retry);
+        offset = filesize - (duration_max_read_size << retry);
         if (offset < 0)
             offset = 0;
 
@@ -1870,7 +1873,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
         for (;;) {
             AVStream *st;
             FFStream *sti;
-            if (read_size >= DURATION_MAX_READ_SIZE << (FFMAX(retry - 1, 0)))
+            if (read_size >= duration_max_read_size << (FFMAX(retry - 1, 0)))
                 break;
 
             do {
@@ -1924,7 +1927,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
         }
     } while (!is_end &&
              offset &&
-             ++retry <= DURATION_MAX_RETRY);
+             ++retry <= duration_max_retry);
 
     av_opt_set_int(ic, "skip_changes", 0, AV_OPT_SEARCH_CHILDREN);
 
diff --git a/libavformat/options_table.h b/libavformat/options_table.h
index b9dca147f9..0cc921a28d 100644
--- a/libavformat/options_table.h
+++ b/libavformat/options_table.h
@@ -108,6 +108,7 @@ static const AVOption avformat_options[] = {
 {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
 {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
 {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D },
+{"durationprobesize", "maximum number of bytes to probe the durations of the streams in estimate_timings_from_pts", OFFSET(duration_probesize), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, INT64_MAX, D},
 {NULL},
 };
 
diff --git a/libavformat/version.h b/libavformat/version.h
index 752aac16f7..a7c80dc564 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -31,7 +31,7 @@
 
 #include "version_major.h"
 
-#define LIBAVFORMAT_VERSION_MINOR   0
+#define LIBAVFORMAT_VERSION_MINOR   1
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 1/1] avformat/demux: Add durationprobesize AVOption

[Stefano Sabatini]

Sorry for the slow replay.

On date Monday 2024-03-11 16:57:19 +0100, Nicolas Gaullier wrote:
> Yet another probesize used to get the durations when
> estimate_timings_from_pts is required. It is aimed at users interested
> in better durations probing for itself, or because using
> avformat_find_stream_info indirectly and requiring exact values: for
> concatdec for example, especially if streamcopying above it.
> The current code is a performance trade-off that can fail to get video
> stream durations in a scenario with high bitrates and buffering for
> files ending cleanly (as opposed to live captures): the physical gap
> between the last video packet and the last audio packet is very high in
> such a case.
> 
> Default behaviour is unchanged: 250k up to 250k << 6 (step by step).
> Setting this new option has two effects:
> - override the maximum probesize (currently 250k << 6)
> - reduce the number of steps to 1 instead of 6, this is to avoid
> detecting the audio "too early" and failing to reach a video packet.
> Even if a single audio stream duration is found but not the other
> audio/video stream durations, there will be a retry, so at the end the
> full user-overriden probesize will be used as expected by the user.
> 
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
>  doc/APIchanges              |  3 +++
>  doc/formats.texi            | 16 +++++++++++++++-
>  libavformat/avformat.h      | 12 ++++++++++++
>  libavformat/demux.c         | 13 ++++++++-----
>  libavformat/options_table.h |  1 +
>  libavformat/version.h       |  2 +-
>  6 files changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index cf58c8c5f0..c97acf60d1 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
>  
>  API changes, most recent first:
>  

> +2024-03-11 - xxxxxxxxxx - lavf 61.1.100 - avformat.h
> +  Add AVFormatContext.duration_probesize

nit++: missing dot at end of sentence

> +
>  2024-03-08 - xxxxxxxxxx - lavc 61.1.100 - avcodec.h
>    Add AVCodecContext.[nb_]side_data_prefer_packet.
>  
> diff --git a/doc/formats.texi b/doc/formats.texi
> index 69fc1457a4..9cada03a6e 100644
> --- a/doc/formats.texi
> +++ b/doc/formats.texi
> @@ -225,9 +225,23 @@ Specifies the maximum number of streams. This can be used to reject files that
>  would require too many resources due to a large number of streams.
>  
>  @item skip_estimate_duration_from_pts @var{bool} (@emph{input})
> -Skip estimation of input duration when calculated using PTS.


> +Skip estimation of input duration if it requires an additional probing for pts at end of file.

consistency: PTS

>  At present, applicable for MPEG-PS and MPEG-TS.
>  

> +@item durationprobesize @var{integer} (@emph{input})

I'd go with duration_probesize, as it looks more readable (although we
also have fpsprobesize), but most option names are splitting the name
and this looks way more readable.

> +Set probing size for input duration estimation when it actually requires an additional probing
> +for pts at end of file.

consistency: PTS

also specify the measure unit of the value

> +At present, applicable for MPEG-PS and MPEG-TS.

I'm still confused why this should not be applicated to other formats
as well.

> +It is aimed at users interested in better durations probing for itself, or indirectly
> +for specific use cases like using the concat demuxer.

> +Files with high bitrates and ending cleanly (as opposed to live captures), can lead
> +to a large physical gap between the last video packet and the last audio packet,
> +so many bytes have to be read in order to get a video stream duration.
> +Setting this value has a performance impact even for small files because the probing size is fixed.
> +Default behaviour is a trade-off, largely adaptive: the probing size may range from

> +250K up to 16M, but it is not extended to get streams durations at all costs.

If this is bytes, then it might be confusing using K and M, maybe
I'd use Ki and Mi to prevent the possible confusion.

> +Must be an integer not lesser than 1, or 0 for default behaviour.
> +
>  @item strict, f_strict @var{integer} (@emph{input/output})
>  Specify how strictly to follow the standards. @code{f_strict} is deprecated and
>  should be used only via the @command{ffmpeg} tool.
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index de40397676..9042a62b70 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1667,6 +1667,8 @@ typedef struct AVFormatContext {
>       * Skip duration calcuation in estimate_timings_from_pts.
>       * - encoding: unused
>       * - decoding: set by user
> +     *

> +     * @sa duration_probesize

nit: let's favor long forms to help readability (seealso or equivalent)

>       */
>      int skip_estimate_duration_from_pts;
>  
> @@ -1870,6 +1872,16 @@ typedef struct AVFormatContext {
>       * @return 0 on success, a negative AVERROR code on failure
>       */
>      int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> +
> +    /**
> +     * Maximum number of bytes read from input in order to determine stream durations
> +     * when using estimate_timings_from_pts in avformat_find_stream_info().
> +     * Demuxing only, set by the caller before avformat_find_stream_info().
> +     * Can be set to 0 to let avformat choose using a heuristic.
> +     *

> +     * @sa skip_estimate_duration_from_pts

ditto

> +     */
> +    int64_t duration_probesize;
>  } AVFormatContext;
>  
>  /**
> diff --git a/libavformat/demux.c b/libavformat/demux.c
> index 4c50eb5568..68b0c51720 100644
> --- a/libavformat/demux.c
> +++ b/libavformat/demux.c
> @@ -1817,8 +1817,9 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic)
>                 "Estimating duration from bitrate, this may be inaccurate\n");
>  }
>  
> -#define DURATION_MAX_READ_SIZE 250000LL
> -#define DURATION_MAX_RETRY 6

> +#define DURATION_MAX_READ_SIZE_DEFAULT 250000LL
> +#define DURATION_MAX_RETRY_DEFAULT 6
> +#define DURATION_MAX_RETRY_USER 1

nit:

DURATION_DEFAULT_MAX_READ_SIZE
DURATION_DEFAULT_MAX_RETRY_DEFAULT
DURATION_MAX_RETRY 1

>  
>  /* only usable for MPEG-PS streams */
>  static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
> @@ -1826,6 +1827,8 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
>      FFFormatContext *const si = ffformatcontext(ic);
>      AVPacket *const pkt = si->pkt;
>      int num, den, read_size, ret;

> +    int64_t duration_max_read_size = ic->duration_probesize ? ic->duration_probesize >> DURATION_MAX_RETRY_USER : DURATION_MAX_READ_SIZE_DEFAULT;

nit: I find the right shift followed by the leftshift a bit confusing,
but probably there is no simple way to prevent it

> +    int duration_max_retry = ic->duration_probesize ? DURATION_MAX_RETRY_USER : DURATION_MAX_RETRY_DEFAULT;
>      int found_duration = 0;
>      int is_end;
>      int64_t filesize, offset, duration;
> @@ -1861,7 +1864,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
>      filesize = ic->pb ? avio_size(ic->pb) : 0;
>      do {
>          is_end = found_duration;
> -        offset = filesize - (DURATION_MAX_READ_SIZE << retry);
> +        offset = filesize - (duration_max_read_size << retry);
>          if (offset < 0)
>              offset = 0;
>  
> @@ -1870,7 +1873,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
>          for (;;) {
>              AVStream *st;
>              FFStream *sti;
> -            if (read_size >= DURATION_MAX_READ_SIZE << (FFMAX(retry - 1, 0)))
> +            if (read_size >= duration_max_read_size << (FFMAX(retry - 1, 0)))
>                  break;
>  
>              do {
> @@ -1924,7 +1927,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
>          }
>      } while (!is_end &&
>               offset &&
> -             ++retry <= DURATION_MAX_RETRY);
> +             ++retry <= duration_max_retry);
>  
>      av_opt_set_int(ic, "skip_changes", 0, AV_OPT_SEARCH_CHILDREN);
>  
> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> index b9dca147f9..0cc921a28d 100644
> --- a/libavformat/options_table.h
> +++ b/libavformat/options_table.h
> @@ -108,6 +108,7 @@ static const AVOption avformat_options[] = {
>  {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
>  {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
>  {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D },

> +{"durationprobesize", "maximum number of bytes to probe the durations of the streams in estimate_timings_from_pts", OFFSET(duration_probesize), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, INT64_MAX, D},

duration_probesize ?

[...]

Thanks
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 0/1] avformat/demux: Add durationprobesize AVOption

[Nicolas Gaullier]

Thanks to Anton:
- fixed APIchanges
- reworded the nebulous "calculated using PTS" both for duration_probesize and older skip_estimate_duration_from_pts
- documented avformat.h for the special 0 value meaning default behaviour

Plus:
Added "see also" cross links in apidoc between skip_estimate_duration_from_pts and duration_probesize
Added version.h lost previously

Nicolas Gaullier (1):
  avformat/demux: Add durationprobesize AVOption

 doc/APIchanges              |  3 +++
 doc/formats.texi            | 16 +++++++++++++++-
 libavformat/avformat.h      | 12 ++++++++++++
 libavformat/demux.c         | 13 ++++++++-----
 libavformat/options_table.h |  1 +
 libavformat/version.h       |  2 +-
 6 files changed, 40 insertions(+), 7 deletions(-)

-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".