[FFmpeg-devel] [PATCH v4 0/1] avformat/mpegts: Add duration_probesize AVOption

[FFmpeg-devel] [PATCH v4 0/1] avformat/mpegts: Add duration_probesize AVOption

[Nicolas Gaullier]

Thanks to Stefano for the precise inspection, I addressed all the points.

The question about what is specific to mpeg remains, so I will try to elaborate on this.
I don't see how duration_probesize could be needed in any way beyond estimate_timings_from_pts().
And it seems there is no other headerless format like mpeg's, byte seekable but especially dts-muxed, cbr-stuffinged with, potentially, a high I/O physical audio/video delay - or not.
It seems the probing of mpeg streams is very specific currently and I don't think this situation will change in the future.
Take mp4 kind formats: strictly index-based, and even for a truncated file (with moov at the head), no I/O is required to get the actual truncated stream durations.
Take mxf kind formats that can be streamed, or simply truncated/broken: the editunits are quite consistent, carrying both audio&video, so recovering the duration would not be very tricky.

This v4 is "an experimental try" to lift the AVOption in the demuxer (which mean no API change). The problem is that it makes the AVOption belong to mpegts.c (I will focus on mpegts, I think it is the only real use case) but unused by it, so I don't feel it is acceptable?
Any input welcome.

Nicolas Gaullier (1):
  avformat/mpegts: Add duration_probesize AVOption

 doc/demuxers.texi     |  13 +++++
 doc/formats.texi      |   2 +-
 libavformat/demux.c   |  23 ++++++---
 libavformat/mpegts.c  | 104 +---------------------------------------
 libavformat/mpegts.h  | 108 +++++++++++++++++++++++++++++++++++++++++-
 libavformat/version.h |   2 +-
 6 files changed, 140 insertions(+), 112 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 v4 1/1] avformat/mpegts: Add duration_probesize AVOption

[Nicolas Gaullier]

Yet another probesize option 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/demuxers.texi     |  13 +++++
 doc/formats.texi      |   2 +-
 libavformat/demux.c   |  23 ++++++---
 libavformat/mpegts.c  | 104 +---------------------------------------
 libavformat/mpegts.h  | 108 +++++++++++++++++++++++++++++++++++++++++-
 libavformat/version.h |   2 +-
 6 files changed, 140 insertions(+), 112 deletions(-)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index b70f3a38d7..f88ae18a6e 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -992,6 +992,19 @@ streams move to different PIDs. Default value is 0.
 @item max_packet_size
 Set maximum size, in bytes, of packet emitted by the demuxer. Payloads above this size
 are split across multiple packets. Range is 1 to INT_MAX/2. Default is 204800 bytes.
+
+@item duration_probesize
+Set probing size, in bytes, for input duration estimation which requires a specific probing
+for PTS at end of file.
+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
+250000 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.
 @end table
 
 @section mpjpeg
diff --git a/doc/formats.texi b/doc/formats.texi
index 69fc1457a4..e16cd88b2c 100644
--- a/doc/formats.texi
+++ b/doc/formats.texi
@@ -225,7 +225,7 @@ 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 strict, f_strict @var{integer} (@emph{input/output})
diff --git a/libavformat/demux.c b/libavformat/demux.c
index 147f3b93ac..bc23786410 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -47,6 +47,7 @@
 #include "id3v2.h"
 #include "internal.h"
 #include "url.h"
+#include "mpegts.h"
 
 static int64_t wrap_timestamp(const AVStream *st, int64_t timestamp)
 {
@@ -1803,20 +1804,30 @@ 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_DEFAULT_MAX_READ_SIZE 250000LL
+#define DURATION_DEFAULT_MAX_RETRY 6
 
-/* only usable for MPEG-PS streams */
+/* only usable for MPEG-PS/TS streams */
 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 = DURATION_DEFAULT_MAX_READ_SIZE;
+    int duration_max_retry = DURATION_DEFAULT_MAX_RETRY;
     int found_duration = 0;
     int is_end;
     int64_t filesize, offset, duration;
     int retry = 0;
 
+    if (!strcmp(ic->iformat->name, "mpegts")) {
+        MpegTSContext *ts = ic->priv_data;
+        if (ts->duration_probesize) {
+            duration_max_retry = 1;
+            duration_max_read_size = ts->duration_probesize >> duration_max_retry;
+        }
+    }
+
     /* flush packet queue */
     ff_flush_packet_queue(ic);
 
@@ -1847,7 +1858,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;
 
@@ -1856,7 +1867,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 {
@@ -1910,7 +1921,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/mpegts.c b/libavformat/mpegts.c
index de7a3c8b45..dbe58cb57c 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -64,52 +64,6 @@
 #define PROBE_PACKET_MAX_BUF 8192
 #define PROBE_PACKET_MARGIN 5
 
-enum MpegTSFilterType {
-    MPEGTS_PES,
-    MPEGTS_SECTION,
-    MPEGTS_PCR,
-};
-
-typedef struct MpegTSFilter MpegTSFilter;
-
-typedef int PESCallback (MpegTSFilter *f, const uint8_t *buf, int len,
-                         int is_start, int64_t pos);
-
-typedef struct MpegTSPESFilter {
-    PESCallback *pes_cb;
-    void *opaque;
-} MpegTSPESFilter;
-
-typedef void SectionCallback (MpegTSFilter *f, const uint8_t *buf, int len);
-
-typedef void SetServiceCallback (void *opaque, int ret);
-
-typedef struct MpegTSSectionFilter {
-    int section_index;
-    int section_h_size;
-    int last_ver;
-    unsigned crc;
-    unsigned last_crc;
-    uint8_t *section_buf;
-    unsigned int check_crc : 1;
-    unsigned int end_of_section_reached : 1;
-    SectionCallback *section_cb;
-    void *opaque;
-} MpegTSSectionFilter;
-
-struct MpegTSFilter {
-    int pid;
-    int es_id;
-    int last_cc; /* last cc code (-1 if first packet) */
-    int64_t last_pcr;
-    int discard;
-    enum MpegTSFilterType type;
-    union {
-        MpegTSPESFilter pes_filter;
-        MpegTSSectionFilter section_filter;
-    } u;
-};
-
 struct Stream {
     int idx;
     int stream_identifier;
@@ -128,63 +82,6 @@ struct Program {
     int pmt_found;
 };
 
-struct MpegTSContext {
-    const AVClass *class;
-    /* user data */
-    AVFormatContext *stream;
-    /** raw packet size, including FEC if present */
-    int raw_packet_size;
-
-    int64_t pos47_full;
-
-    /** if true, all pids are analyzed to find streams */
-    int auto_guess;
-
-    /** compute exact PCR for each transport stream packet */
-    int mpeg2ts_compute_pcr;
-
-    /** fix dvb teletext pts                                 */
-    int fix_teletext_pts;
-
-    int64_t cur_pcr;    /**< used to estimate the exact PCR */
-    int64_t pcr_incr;   /**< used to estimate the exact PCR */
-
-    /* data needed to handle file based ts */
-    /** stop parsing loop */
-    int stop_parse;
-    /** packet containing Audio/Video data */
-    AVPacket *pkt;
-    /** to detect seek */
-    int64_t last_pos;
-
-    int skip_changes;
-    int skip_clear;
-    int skip_unknown_pmt;
-
-    int scan_all_pmts;
-
-    int resync_size;
-    int merge_pmt_versions;
-    int max_packet_size;
-
-    int id;
-
-    /******************************************/
-    /* private mpegts data */
-    /* scan context */
-    /** structure to keep track of Program->pids mapping */
-    unsigned int nb_prg;
-    struct Program *prg;
-
-    int8_t crc_validity[NB_PID_MAX];
-    /** filters for various streams specified by PMT + for the PAT and PMT */
-    MpegTSFilter *pids[NB_PID_MAX];
-    int current_pid;
-
-    AVStream *epg_stream;
-    AVBufferPool* pools[32];
-};
-
 #define MPEGTS_OPTIONS \
     { "resync_size",   "set size limit for looking up a new synchronization",  \
         offsetof(MpegTSContext, resync_size), AV_OPT_TYPE_INT,                 \
@@ -212,6 +109,7 @@ static const AVOption options[] = {
      {.i64 = 0}, 0, 1, 0 },
     {"max_packet_size", "maximum size of emitted packet", offsetof(MpegTSContext, max_packet_size), AV_OPT_TYPE_INT,
      {.i64 = 204800}, 1, INT_MAX/2, AV_OPT_FLAG_DECODING_PARAM },
+    {"duration_probesize", "maximum number of bytes to probe the durations of the streams", offsetof(MpegTSContext,duration_probesize), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, INT64_MAX, AV_OPT_FLAG_DECODING_PARAM},
     { NULL },
 };
 
diff --git a/libavformat/mpegts.h b/libavformat/mpegts.h
index 14ae312c50..8b31276bf1 100644
--- a/libavformat/mpegts.h
+++ b/libavformat/mpegts.h
@@ -165,7 +165,113 @@
 #define METADATA_DESCRIPTOR          0x26
 #define METADATA_STD_DESCRIPTOR      0x27
 
-typedef struct MpegTSContext MpegTSContext;
+enum MpegTSFilterType {
+    MPEGTS_PES,
+    MPEGTS_SECTION,
+    MPEGTS_PCR,
+};
+
+typedef struct MpegTSFilter MpegTSFilter;
+
+typedef int PESCallback (MpegTSFilter *f, const uint8_t *buf, int len,
+                         int is_start, int64_t pos);
+
+typedef struct MpegTSPESFilter {
+    PESCallback *pes_cb;
+    void *opaque;
+} MpegTSPESFilter;
+
+typedef void SectionCallback (MpegTSFilter *f, const uint8_t *buf, int len);
+
+typedef void SetServiceCallback (void *opaque, int ret);
+
+typedef struct MpegTSSectionFilter {
+    int section_index;
+    int section_h_size;
+    int last_ver;
+    unsigned crc;
+    unsigned last_crc;
+    uint8_t *section_buf;
+    unsigned int check_crc : 1;
+    unsigned int end_of_section_reached : 1;
+    SectionCallback *section_cb;
+    void *opaque;
+} MpegTSSectionFilter;
+
+struct MpegTSFilter {
+    int pid;
+    int es_id;
+    int last_cc; /* last cc code (-1 if first packet) */
+    int64_t last_pcr;
+    int discard;
+    enum MpegTSFilterType type;
+    union {
+        MpegTSPESFilter pes_filter;
+        MpegTSSectionFilter section_filter;
+    } u;
+};
+
+typedef struct MpegTSContext {
+    const AVClass *class;
+    /* user data */
+    AVFormatContext *stream;
+    /** raw packet size, including FEC if present */
+    int raw_packet_size;
+
+    int64_t pos47_full;
+
+    /** if true, all pids are analyzed to find streams */
+    int auto_guess;
+
+    /** compute exact PCR for each transport stream packet */
+    int mpeg2ts_compute_pcr;
+
+    /** fix dvb teletext pts                                 */
+    int fix_teletext_pts;
+
+    int64_t cur_pcr;    /**< used to estimate the exact PCR */
+    int64_t pcr_incr;   /**< used to estimate the exact PCR */
+
+    /* data needed to handle file based ts */
+    /** stop parsing loop */
+    int stop_parse;
+    /** packet containing Audio/Video data */
+    AVPacket *pkt;
+    /** to detect seek */
+    int64_t last_pos;
+
+    int skip_changes;
+    int skip_clear;
+    int skip_unknown_pmt;
+
+    int scan_all_pmts;
+
+    int resync_size;
+    int merge_pmt_versions;
+    int max_packet_size;
+
+    int id;
+
+    /* for external use by avformat_find_stream_info() */
+    /** Maximum number of bytes read from input in order to determine stream durations.
+        Can be set to 0 to let avformat choose using a heuristic. */
+    int64_t duration_probesize;
+
+    /******************************************/
+    /* private mpegts data */
+    /* scan context */
+    /** structure to keep track of Program->pids mapping */
+    unsigned int nb_prg;
+    struct Program *prg;
+
+    int8_t crc_validity[NB_PID_MAX];
+    /** filters for various streams specified by PMT + for the PAT and PMT */
+    MpegTSFilter *pids[NB_PID_MAX];
+    int current_pid;
+
+    AVStream *epg_stream;
+    AVBufferPool* pools[32];
+} MpegTSContext;
 
 MpegTSContext *avpriv_mpegts_parse_open(AVFormatContext *s);
 int avpriv_mpegts_parse_packet(MpegTSContext *ts, AVPacket *pkt,
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 v4 1/1] avformat/mpegts: Add duration_probesize AVOption

[Stefano Sabatini]

On date Tuesday 2024-03-26 14:18:31 +0100, Nicolas Gaullier wrote:
> Yet another probesize option 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/demuxers.texi     |  13 +++++
>  doc/formats.texi      |   2 +-
>  libavformat/demux.c   |  23 ++++++---
>  libavformat/mpegts.c  | 104 +---------------------------------------
>  libavformat/mpegts.h  | 108 +++++++++++++++++++++++++++++++++++++++++-
>  libavformat/version.h |   2 +-
>  6 files changed, 140 insertions(+), 112 deletions(-)
> 
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index b70f3a38d7..f88ae18a6e 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -992,6 +992,19 @@ streams move to different PIDs. Default value is 0.
>  @item max_packet_size
>  Set maximum size, in bytes, of packet emitted by the demuxer. Payloads above this size
>  are split across multiple packets. Range is 1 to INT_MAX/2. Default is 204800 bytes.
> +
> +@item duration_probesize
> +Set probing size, in bytes, for input duration estimation which requires a specific probing
> +for PTS at end of file.
> +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
> +250000 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.
>  @end table
>  
>  @section mpjpeg
> diff --git a/doc/formats.texi b/doc/formats.texi
> index 69fc1457a4..e16cd88b2c 100644
> --- a/doc/formats.texi
> +++ b/doc/formats.texi
> @@ -225,7 +225,7 @@ 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.

nit++: PTS

>  At present, applicable for MPEG-PS and MPEG-TS.
>  
>  @item strict, f_strict @var{integer} (@emph{input/output})
> diff --git a/libavformat/demux.c b/libavformat/demux.c
> index 147f3b93ac..bc23786410 100644
> --- a/libavformat/demux.c
> +++ b/libavformat/demux.c
> @@ -47,6 +47,7 @@
>  #include "id3v2.h"
>  #include "internal.h"
>  #include "url.h"

> +#include "mpegts.h"

this is entangling generic and specific code, and it is something
which should be avoided (ideally the generic code should know nothing
about the specific demuxer)

Rather than this, I think the previous approach is more valid: even if
we have an option that it is only useful for a specific demuxer, its
implementation was still abstract and avoiding any reference to
internals of the mpegts demuxer - and maybe it might turn useful for
other formats - who knows.

What we need in that case is only clarify the use case for which it
was conceived (which might well only apply to a specific demuxer) at
the documentation level. To clarify, I was mostly fine with the
previous patch, but for the nits and for the documentation issue - but
if this turns out that the only use case is about the MPEGTS, probably
we should just mention that (as you was already doing).

[...]

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