Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 0/1] avformat/demux: Add durationprobesize AVOption
@ 2024-02-06 10:52 Nicolas Gaullier
  2024-02-06 10:52 ` [FFmpeg-devel] [PATCH 1/1] " Nicolas Gaullier
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Gaullier @ 2024-02-06 10:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Nicolas Gaullier

I posted "avformat/demux: Add more retries to get more stream durations"
last friday and it is a complementary patch.
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10654
Note that, if this "Add more retries" patch is accepted, I would update
this patch to set DURATION_MAX_RETRY_USER to the value of MORE_DURATIONS_MAX_RETRY (3),
which means that if the user ask for 8M, the first step will be to try 1M,
then 2M, then up to 8M until all A/V durations are found.
Currently, since there is only one extra retry to get all durations,
if the user ask for 8M, the first step is to try with 4M.

The default behaviour remains unchanged as exact&complete stream durations
is only required for somes use cases and/or some specific files.

Here is a sample file (mpegts @15Mb/s, with A/V pts cleanly cut at the end):
https://0x0.st/HkxN.ts

If it can help, I can add a trac issue or build a patchset with the two patches.

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

 libavformat/avformat.h      |  8 ++++++++
 libavformat/demux.c         | 13 ++++++++-----
 libavformat/options_table.h |  1 +
 3 files changed, 17 insertions(+), 5 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".

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [FFmpeg-devel] [PATCH 1/1] avformat/demux: Add durationprobesize AVOption
  2024-02-06 10:52 [FFmpeg-devel] [PATCH 0/1] avformat/demux: Add durationprobesize AVOption Nicolas Gaullier
@ 2024-02-06 10:52 ` Nicolas Gaullier
  2024-02-06 23:52   ` Stefano Sabatini
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Gaullier @ 2024-02-06 10:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Nicolas Gaullier

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(-)

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},
 };
 
-- 
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".

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/1] avformat/demux: Add durationprobesize AVOption
  2024-02-06 10:52 ` [FFmpeg-devel] [PATCH 1/1] " Nicolas Gaullier
@ 2024-02-06 23:52   ` Stefano Sabatini
  2024-03-05 11:07     ` Nicolas Gaullier
  0 siblings, 1 reply; 4+ messages in thread
From: Stefano Sabatini @ 2024-02-06 23:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Nicolas Gaullier

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?)


_______________________________________________
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".

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/1] avformat/demux: Add durationprobesize AVOption
  2024-02-06 23:52   ` Stefano Sabatini
@ 2024-03-05 11:07     ` Nicolas Gaullier
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Gaullier @ 2024-03-05 11:07 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

>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
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-03-05 11:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-06 10:52 [FFmpeg-devel] [PATCH 0/1] avformat/demux: Add durationprobesize AVOption Nicolas Gaullier
2024-02-06 10:52 ` [FFmpeg-devel] [PATCH 1/1] " Nicolas Gaullier
2024-02-06 23:52   ` Stefano Sabatini
2024-03-05 11:07     ` Nicolas Gaullier

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git