| Message ID | 20240311155719.1874201-2-nicolas.gaullier@cji.paris |
|---|---|
| State | New |
| Headers | show |
| Series | avformat/demux: Add durationprobesize AVOption | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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
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, \
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(-)