| Message ID | 20240206105209.315221-2-nicolas.gaullier@cji.paris |
|---|---|
| State | New |
| Headers | show |
| Series | avformat/demux: Add durationprobesize AVOption | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/commit_msg_loongarch64 | warning | Please wrap lines in the body of the commit message between 60 and 72 characters. |
| andriy/commit_msg_x86 | warning | Please wrap lines in the body of the commit message between 60 and 72 characters. |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On date Tuesday 2024-02-06 11:52:09 +0100, Nicolas Gaullier wrote: > Yet another probesize used to get the last pts (and thus the duration) > of mpegts/ps files. 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 exemple, especially if streamcopying above it. nit: exemple typo > The current code does not behave well with high bitrates and high video buffering > (physical gap between the last video packet and the last audio packet). > > 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. Here, even if audio > duration is found but not the video duration, there will be a retry, so at the > end the full user-overriden probesize will be used. > > Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> > --- > libavformat/avformat.h | 8 ++++++++ > libavformat/demux.c | 13 ++++++++----- > libavformat/options_table.h | 1 + > 3 files changed, 17 insertions(+), 5 deletions(-) please add missing user doc in doc/formats.texi (you might reuse the hightlight above to explain why this is needed), also the new field requires an entry in doc/APIChanges > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index 5d0fe82250..533f5a963d 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -1823,6 +1823,14 @@ typedef struct AVFormatContext { > * Freed by libavformat in avformat_free_context(). > */ > AVStreamGroup **stream_groups; > + > + /** > + * Maximum number of bytes read at the end of input in order to determine the > + * stream durations. Used by avformat_find_stream_info() for MPEG-TS/PS. let's clarify this: is there any reason why this should not be used with other formats? If this the case, probably a private option would be best. If not, probably we should amend the doxy as it suggests it is only useful with MPEG TS/PS. > + * > + * Demuxing only, set by the caller before avformat_open_input(). > + */ > + int64_t duration_probesize; > } AVFormatContext; > > /** > diff --git a/libavformat/demux.c b/libavformat/demux.c > index 6f640b92b1..798b44c979 100644 > --- a/libavformat/demux.c > +++ b/libavformat/demux.c > @@ -1740,8 +1740,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) > @@ -1749,6 +1750,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; > @@ -1784,7 +1787,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; > > @@ -1793,7 +1796,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 { > @@ -1847,7 +1850,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 91708de453..c2bdb484a7 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 stream durations", OFFSET(duration_probesize), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, INT64_MAX, D}, duration_probesize? ... to probe the stream duration (why the plural?)
>De : Stefano Sabatini <stefasab@gmail.com> >Envoyé : mercredi 7 février 2024 00:52 > >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h index >> + * stream durations. Used by avformat_find_stream_info() for MPEG-TS/PS. > >let's clarify this: is there any reason why this should not be used with other formats? If this the case, probably a private option would be best. If not, probably we should amend the doxy as it suggests it is only useful with MPEG TS/PS. There is already an AVOption in the same case: skip_estimate_duration_from_pts, but indeed, it is much more appropriate to mention estimate_timings_from_pts rather than referring to mpeg directly. The texi says "At present, applicable for MPEG-PS and MPEG-TS". So, I will just try to go in the same logic. >> diff --git a/libavformat/options_table.h b/libavformat/options_table.h >> index 91708de453..c2bdb484a7 100644 >> --- a/libavformat/options_table.h >> +++ b/libavformat/options_table.h > >> +{"durationprobesize", "maximum number of bytes to probe the stream >> +durations", OFFSET(duration_probesize), AV_OPT_TYPE_INT64, {.i64 = 0 >> +}, 0, INT64_MAX, D}, > >duration_probesize? ... to probe the stream duration (why the plural?) The option affects the probing of all the streams and then these are computed to get the overall file duration. I will update all the wording. The naming of the avoption itself is a big worry for me. I tried to mimic format_probesize, but plural or not, I don't know what is best? I will send a v2 with same code but all revised wordings and doc. Thank you very much for the review. Sorry for the delay, I was very busy with my other patch serie. Nicolas
diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 5d0fe82250..533f5a963d 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1823,6 +1823,14 @@ typedef struct AVFormatContext { * Freed by libavformat in avformat_free_context(). */ AVStreamGroup **stream_groups; + + /** + * Maximum number of bytes read at the end of input in order to determine the + * stream durations. Used by avformat_find_stream_info() for MPEG-TS/PS. + * + * Demuxing only, set by the caller before avformat_open_input(). + */ + int64_t duration_probesize; } AVFormatContext; /** diff --git a/libavformat/demux.c b/libavformat/demux.c index 6f640b92b1..798b44c979 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1740,8 +1740,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) @@ -1749,6 +1750,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; @@ -1784,7 +1787,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; @@ -1793,7 +1796,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 { @@ -1847,7 +1850,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 91708de453..c2bdb484a7 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 stream durations", OFFSET(duration_probesize), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, INT64_MAX, D}, {NULL}, };
Yet another probesize used to get the last pts (and thus the duration) of mpegts/ps files. 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 exemple, especially if streamcopying above it. The current code does not behave well with high bitrates and high video buffering (physical gap between the last video packet and the last audio packet). 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. Here, even if audio duration is found but not the video duration, there will be a retry, so at the end the full user-overriden probesize will be used. Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> --- libavformat/avformat.h | 8 ++++++++ libavformat/demux.c | 13 ++++++++----- libavformat/options_table.h | 1 + 3 files changed, 17 insertions(+), 5 deletions(-)