[FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0)

[FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0)

[Anton Khirnov]

This is the standard way to mark unreachable cases in a switch
---
 fftools/ffmpeg_demux.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 95af31e9ef..5d07b7153d 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1223,8 +1223,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     case AVMEDIA_TYPE_ATTACHMENT:
     case AVMEDIA_TYPE_UNKNOWN:
         break;
-    default:
-        abort();
+    default: av_assert0(0);
     }
 
     ist->par = avcodec_parameters_alloc();
-- 
2.42.0

_______________________________________________
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 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf

[Anton Khirnov]

---
 doc/ffmpeg.texi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 7246a46d2f..d75517b443 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1643,8 +1643,6 @@ option to disable streams individually.
 As an output option, disables subtitle recording i.e. automatic selection or
 mapping of any subtitle stream. For full manual control see the @code{-map}
 option.
-@item -sbsf @var{bitstream_filter}
-Deprecated, see -bsf
 @end table
 
 @section Advanced Subtitle options
-- 
2.42.0

_______________________________________________
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 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder

[Anton Khirnov]

Add it to decoder options instead, to be processed when opening the
decoder. This way it won't be overridden by flags the user might be
setting otherwise.
---
 fftools/ffmpeg_demux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 5d07b7153d..cacdc76a71 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1156,7 +1156,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     }
 
     if (o->bitexact)
-        ist->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
+        av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY);
 
     switch (par->codec_type) {
     case AVMEDIA_TYPE_VIDEO:
-- 
2.42.0

_______________________________________________
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 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder

[Anton Khirnov]

This avoids the requirement to always have a decoder context.
---
 fftools/ffmpeg_demux.c | 43 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index cacdc76a71..892094c512 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -899,19 +899,18 @@ static int choose_decoder(const OptionsContext *o, AVFormatContext *s, AVStream
     }
 }
 
-static int guess_input_channel_layout(InputStream *ist, int guess_layout_max)
+static int guess_input_channel_layout(InputStream *ist, AVCodecParameters *par,
+                                      int guess_layout_max)
 {
-    AVCodecContext *dec = ist->dec_ctx;
-
-    if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
+    if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
         char layout_name[256];
 
-        if (dec->ch_layout.nb_channels > guess_layout_max)
+        if (par->ch_layout.nb_channels > guess_layout_max)
             return 0;
-        av_channel_layout_default(&dec->ch_layout, dec->ch_layout.nb_channels);
-        if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC)
+        av_channel_layout_default(&par->ch_layout, par->ch_layout.nb_channels);
+        if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC)
             return 0;
-        av_channel_layout_describe(&dec->ch_layout, layout_name, sizeof(layout_name));
+        av_channel_layout_describe(&par->ch_layout, layout_name, sizeof(layout_name));
         av_log(ist, AV_LOG_WARNING, "Guessed Channel Layout: %s\n", layout_name);
     }
     return 1;
@@ -1145,16 +1144,6 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
         ist->user_set_discard = ist->st->discard;
     }
 
-    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
-    if (!ist->dec_ctx)
-        return AVERROR(ENOMEM);
-
-    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
-    if (ret < 0) {
-        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
-        return ret;
-    }
-
     if (o->bitexact)
         av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY);
 
@@ -1181,7 +1170,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     case AVMEDIA_TYPE_AUDIO: {
         int guess_layout_max = INT_MAX;
         MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st);
-        guess_input_channel_layout(ist, guess_layout_max);
+        guess_input_channel_layout(ist, par, guess_layout_max);
         break;
     }
     case AVMEDIA_TYPE_DATA:
@@ -1190,7 +1179,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
         MATCH_PER_STREAM_OPT(fix_sub_duration, i, ist->fix_sub_duration, ic, st);
         MATCH_PER_STREAM_OPT(canvas_sizes, str, canvas_size, ic, st);
         if (canvas_size) {
-            ret = av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height,
+            ret = av_parse_video_size(&par->width, &par->height,
                                       canvas_size);
             if (ret < 0) {
                 av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size);
@@ -1201,8 +1190,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
         /* Compute the size of the canvas for the subtitles stream.
            If the subtitles codecpar has set a size, use it. Otherwise use the
            maximum dimensions of the video streams in the same file. */
-        ist->sub2video.w = ist->dec_ctx->width;
-        ist->sub2video.h = ist->dec_ctx->height;
+        ist->sub2video.w = par->width;
+        ist->sub2video.h = par->height;
         if (!(ist->sub2video.w && ist->sub2video.h)) {
             for (int j = 0; j < ic->nb_streams; j++) {
                 AVCodecParameters *par1 = ic->streams[j]->codecpar;
@@ -1226,6 +1215,16 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     default: av_assert0(0);
     }
 
+    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
+    if (!ist->dec_ctx)
+        return AVERROR(ENOMEM);
+
+    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
+    if (ret < 0) {
+        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
+        return ret;
+    }
+
     ist->par = avcodec_parameters_alloc();
     if (!ist->par)
         return AVERROR(ENOMEM);
-- 
2.42.0

_______________________________________________
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 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding

[Anton Khirnov]

It is not needed otherwise.
---
 fftools/ffmpeg_demux.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 892094c512..c51140b1c5 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed)
     if (decoding_needed && ds->sch_idx_dec < 0) {
         int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
 
+        ist->dec_ctx = avcodec_alloc_context3(ist->dec);
+        if (!ist->dec_ctx)
+            return AVERROR(ENOMEM);
+
+        ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par);
+        if (ret < 0) {
+            av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
+            return ret;
+        }
+
         ret = sch_add_dec(d->sch, decoder_thread, ist, d->loop && is_audio);
         if (ret < 0)
             return ret;
@@ -1215,23 +1225,13 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     default: av_assert0(0);
     }
 
-    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
-    if (!ist->dec_ctx)
-        return AVERROR(ENOMEM);
-
-    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
-    if (ret < 0) {
-        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
-        return ret;
-    }
-
     ist->par = avcodec_parameters_alloc();
     if (!ist->par)
         return AVERROR(ENOMEM);
 
-    ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx);
+    ret = avcodec_parameters_copy(ist->par, par);
     if (ret < 0) {
-        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
+        av_log(ist, AV_LOG_ERROR, "Error exporting stream parameters.\n");
         return ret;
     }
 
-- 
2.42.0

_______________________________________________
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 6/8] fftools/ffmpeg_demux: add demuxing thread private data

[Anton Khirnov]

To be used for data that never needs to be visible outside of the
demuxer thread, similarly as was previously done for other components.
---
 fftools/ffmpeg_demux.c | 67 ++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index c51140b1c5..eae1f0bde5 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -115,6 +115,11 @@ typedef struct Demuxer {
     int                   nb_streams_finished;
 } Demuxer;
 
+typedef struct DemuxThreadContext {
+    // packet used for reading from the demuxer
+    AVPacket *pkt_demux;
+} DemuxThreadContext;
+
 static DemuxStream *ds_from_ist(InputStream *ist)
 {
     return (DemuxStream*)ist;
@@ -565,18 +570,36 @@ static void thread_set_name(InputFile *f)
     ff_thread_setname(name);
 }
 
+static void demux_thread_uninit(DemuxThreadContext *dt)
+{
+    av_packet_free(&dt->pkt_demux);
+
+    memset(dt, 0, sizeof(*dt));
+}
+
+static int demux_thread_init(DemuxThreadContext *dt)
+{
+    memset(dt, 0, sizeof(*dt));
+
+    dt->pkt_demux = av_packet_alloc();
+    if (!dt->pkt_demux)
+        return AVERROR(ENOMEM);
+
+    return 0;
+}
+
 static void *input_thread(void *arg)
 {
     Demuxer   *d = arg;
     InputFile *f = &d->f;
-    AVPacket *pkt;
+
+    DemuxThreadContext dt;
+
     int ret = 0;
 
-    pkt = av_packet_alloc();
-    if (!pkt) {
-        ret = AVERROR(ENOMEM);
+    ret = demux_thread_init(&dt);
+    if (ret < 0)
         goto finish;
-    }
 
     thread_set_name(f);
 
@@ -589,7 +612,7 @@ static void *input_thread(void *arg)
         DemuxStream *ds;
         unsigned send_flags = 0;
 
-        ret = av_read_frame(f->ctx, pkt);
+        ret = av_read_frame(f->ctx, dt.pkt_demux);
 
         if (ret == AVERROR(EAGAIN)) {
             av_usleep(10000);
@@ -598,12 +621,12 @@ static void *input_thread(void *arg)
         if (ret < 0) {
             if (d->loop) {
                 /* signal looping to our consumers */
-                pkt->stream_index = -1;
+                dt.pkt_demux->stream_index = -1;
 
-                ret = sch_demux_send(d->sch, f->index, pkt, 0);
+                ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0);
                 if (ret >= 0)
-                    ret = seek_to_start(d, (Timestamp){ .ts = pkt->pts,
-                                                        .tb = pkt->time_base });
+                    ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts,
+                                                        .tb = dt.pkt_demux->time_base });
                 if (ret >= 0)
                     continue;
 
@@ -622,39 +645,39 @@ static void *input_thread(void *arg)
         }
 
         if (do_pkt_dump) {
-            av_pkt_dump_log2(NULL, AV_LOG_INFO, pkt, do_hex_dump,
-                             f->ctx->streams[pkt->stream_index]);
+            av_pkt_dump_log2(NULL, AV_LOG_INFO, dt.pkt_demux, do_hex_dump,
+                             f->ctx->streams[dt.pkt_demux->stream_index]);
         }
 
         /* the following test is needed in case new streams appear
            dynamically in stream : we ignore them */
-        ds = pkt->stream_index < f->nb_streams ?
-             ds_from_ist(f->streams[pkt->stream_index]) : NULL;
+        ds = dt.pkt_demux->stream_index < f->nb_streams ?
+             ds_from_ist(f->streams[dt.pkt_demux->stream_index]) : NULL;
         if (!ds || ds->discard || ds->finished) {
-            report_new_stream(d, pkt);
-            av_packet_unref(pkt);
+            report_new_stream(d, dt.pkt_demux);
+            av_packet_unref(dt.pkt_demux);
             continue;
         }
 
-        if (pkt->flags & AV_PKT_FLAG_CORRUPT) {
+        if (dt.pkt_demux->flags & AV_PKT_FLAG_CORRUPT) {
             av_log(d, exit_on_error ? AV_LOG_FATAL : AV_LOG_WARNING,
                    "corrupt input packet in stream %d\n",
-                   pkt->stream_index);
+                   dt.pkt_demux->stream_index);
             if (exit_on_error) {
-                av_packet_unref(pkt);
+                av_packet_unref(dt.pkt_demux);
                 ret = AVERROR_INVALIDDATA;
                 break;
             }
         }
 
-        ret = input_packet_process(d, pkt, &send_flags);
+        ret = input_packet_process(d, dt.pkt_demux, &send_flags);
         if (ret < 0)
             break;
 
         if (d->readrate)
             readrate_sleep(d);
 
-        ret = demux_send(d, ds, pkt, send_flags);
+        ret = demux_send(d, ds, dt.pkt_demux, send_flags);
         if (ret < 0)
             break;
     }
@@ -664,7 +687,7 @@ static void *input_thread(void *arg)
         ret = 0;
 
 finish:
-    av_packet_free(&pkt);
+    demux_thread_uninit(&dt);
 
     return (void*)(intptr_t)ret;
 }
-- 
2.42.0

_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Anton Khirnov]

Previously bitstream filters could only be applied right before muxing,
this allows to apply them right after demuxing.
---
 Changelog                       |   1 +
 doc/ffmpeg.texi                 |  22 +++--
 fftools/ffmpeg_demux.c          | 139 ++++++++++++++++++++++++++++----
 fftools/ffmpeg_opt.c            |   2 +-
 tests/fate/ffmpeg.mak           |   5 ++
 tests/ref/fate/ffmpeg-bsf-input |  18 +++++
 6 files changed, 164 insertions(+), 23 deletions(-)
 create mode 100644 tests/ref/fate/ffmpeg-bsf-input

diff --git a/Changelog b/Changelog
index 5b2899d05b..f8191d88c7 100644
--- a/Changelog
+++ b/Changelog
@@ -18,6 +18,7 @@ version <next>:
 - lavu/eval: introduce randomi() function in expressions
 - VVC decoder
 - fsync filter
+- ffmpeg CLI -bsf option may now be used for input as well as output
 
 version 6.1:
 - libaribcaption decoder
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index d75517b443..59f7badcb6 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -2093,26 +2093,34 @@ an output mpegts file:
 ffmpeg -i inurl -streamid 0:33 -streamid 1:36 out.ts
 @end example
 
-@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{output,per-stream})
-Apply bitstream filters to matching streams.
+@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{input/output,per-stream})
+Apply bitstream filters to matching streams. The filters are applied to each
+packet as it is received from the demuxer (when used as an input option) or
+before it is sent to the muxer (when used as an output option).
 
 @var{bitstream_filters} is a comma-separated list of bitstream filter
-specifications. The specified bitstream filters are applied to coded packets in
-the order they are written in. Each bitstream filter specification is of the
-form
+specifications, each of the form
 @example
 @var{filter}[=@var{optname0}=@var{optval0}:@var{optname1}=@var{optval1}:...]
 @end example
 Any of the ',=:' characters that are to be a part of an option value need to be
 escaped with a backslash.
 
-Use the @code{-bsfs} option to get the list of bitstream filters.
+Use the @code{-bsfs} option to get the list of bitstream filters. E.g.
 @example
-ffmpeg -i h264.mp4 -c:v copy -bsf:v h264_mp4toannexb -an out.h264
+ffmpeg -bsf:v h264_mp4toannexb -i h264.mp4 -c:v copy -an out.h264
 @end example
+applies the @code{h264_mp4toannexb} bitstream filter (which converts
+MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream.
+
+On the other hand,
 @example
 ffmpeg -i file.mov -an -vn -bsf:s mov2textsub -c:s copy -f rawvideo sub.txt
 @end example
+applies the @code{mov2textsub} bitstream filter (which extracts text from MOV
+subtitles) to the @emph{output} subtitle stream. Note, however, that since both
+examples use @code{-c copy}, it matters little whether the filters are applied
+on input or output - that would change if transcoding was hapenning.
 
 @item -tag[:@var{stream_specifier}] @var{codec_tag} (@emph{input/output,per-stream})
 Force a tag/fourcc for matching streams.
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index eae1f0bde5..16d4f67e59 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -34,6 +34,7 @@
 #include "libavutil/time.h"
 #include "libavutil/timestamp.h"
 
+#include "libavcodec/bsf.h"
 #include "libavcodec/packet.h"
 
 #include "libavformat/avformat.h"
@@ -71,6 +72,8 @@ typedef struct DemuxStream {
 
     const AVCodecDescriptor *codec_desc;
 
+    AVBSFContext *bsf;
+
     /* number of packets successfully read for this stream */
     uint64_t nb_packets;
     // combined size of all the packets read
@@ -118,6 +121,8 @@ typedef struct Demuxer {
 typedef struct DemuxThreadContext {
     // packet used for reading from the demuxer
     AVPacket *pkt_demux;
+    // packet for reading from BSFs
+    AVPacket *pkt_bsf;
 } DemuxThreadContext;
 
 static DemuxStream *ds_from_ist(InputStream *ist)
@@ -513,13 +518,17 @@ static int do_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags,
     return 0;
 }
 
-static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags)
+static int demux_send(Demuxer *d, DemuxThreadContext *dt, DemuxStream *ds,
+                      AVPacket *pkt, unsigned flags)
 {
     InputFile  *f = &d->f;
     int ret;
 
+    // pkt can be NULL only when flushing BSFs
+    av_assert0(ds->bsf || pkt);
+
     // send heartbeat for sub2video streams
-    if (d->pkt_heartbeat && pkt->pts != AV_NOPTS_VALUE) {
+    if (d->pkt_heartbeat && pkt && pkt->pts != AV_NOPTS_VALUE) {
         for (int i = 0; i < f->nb_streams; i++) {
             DemuxStream *ds1 = ds_from_ist(f->streams[i]);
 
@@ -537,10 +546,69 @@ static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags
         }
     }
 
-    ret = do_send(d, ds, pkt, flags, "demuxed");
-    if (ret < 0)
-        return ret;
+    if (ds->bsf) {
+        if (pkt)
+            av_packet_rescale_ts(pkt, pkt->time_base, ds->bsf->time_base_in);
 
+        ret = av_bsf_send_packet(ds->bsf, pkt);
+        if (ret < 0) {
+            if (pkt)
+                av_packet_unref(pkt);
+            av_log(ds, AV_LOG_ERROR, "Error submitting a packet for filtering: %s\n",
+                   av_err2str(ret));
+            return ret;
+        }
+
+        while (1) {
+            ret = av_bsf_receive_packet(ds->bsf, dt->pkt_bsf);
+            if (ret == AVERROR(EAGAIN))
+                return 0;
+            else if (ret < 0) {
+                if (ret != AVERROR_EOF)
+                    av_log(ds, AV_LOG_ERROR,
+                           "Error applying bitstream filters to a packet: %s\n",
+                           av_err2str(ret));
+                return ret;
+            }
+
+            dt->pkt_bsf->time_base = ds->bsf->time_base_out;
+
+            ret = do_send(d, ds, dt->pkt_bsf, 0, "filtered");
+            if (ret < 0) {
+                av_packet_unref(dt->pkt_bsf);
+                return ret;
+            }
+        }
+    } else {
+        ret = do_send(d, ds, pkt, flags, "demuxed");
+        if (ret < 0)
+            return ret;
+    }
+
+    return 0;
+}
+
+static int demux_bsf_flush(Demuxer *d, DemuxThreadContext *dt)
+{
+    InputFile *f = &d->f;
+    int ret;
+
+    for (unsigned i = 0; i < f->nb_streams; i++) {
+        DemuxStream *ds = ds_from_ist(f->streams[i]);
+
+        if (!ds->bsf)
+            continue;
+
+        ret = demux_send(d, dt, ds, NULL, 0);
+        ret = (ret == AVERROR_EOF) ? 0 : (ret < 0) ? ret : AVERROR_BUG;
+        if (ret < 0) {
+            av_log(ds, AV_LOG_ERROR, "Error flushing BSFs: %s\n",
+                   av_err2str(ret));
+            return ret;
+        }
+
+        av_bsf_flush(ds->bsf);
+    }
 
     return 0;
 }
@@ -573,6 +641,7 @@ static void thread_set_name(InputFile *f)
 static void demux_thread_uninit(DemuxThreadContext *dt)
 {
     av_packet_free(&dt->pkt_demux);
+    av_packet_free(&dt->pkt_bsf);
 
     memset(dt, 0, sizeof(*dt));
 }
@@ -585,6 +654,10 @@ static int demux_thread_init(DemuxThreadContext *dt)
     if (!dt->pkt_demux)
         return AVERROR(ENOMEM);
 
+    dt->pkt_bsf = av_packet_alloc();
+    if (!dt->pkt_bsf)
+        return AVERROR(ENOMEM);
+
     return 0;
 }
 
@@ -619,10 +692,22 @@ static void *input_thread(void *arg)
             continue;
         }
         if (ret < 0) {
+            int ret_bsf;
+
+            if (ret == AVERROR_EOF)
+                av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n");
+            else {
+                av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n",
+                       av_err2str(ret));
+                ret = exit_on_error ? ret : 0;
+            }
+
+            ret_bsf = demux_bsf_flush(d, &dt);
+            ret = err_merge(ret == AVERROR_EOF ? 0 : ret, ret_bsf);
+
             if (d->loop) {
                 /* signal looping to our consumers */
                 dt.pkt_demux->stream_index = -1;
-
                 ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0);
                 if (ret >= 0)
                     ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts,
@@ -633,14 +718,6 @@ static void *input_thread(void *arg)
                 /* fallthrough to the error path */
             }
 
-            if (ret == AVERROR_EOF)
-                av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n");
-            else {
-                av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n",
-                       av_err2str(ret));
-                ret = exit_on_error ? ret : 0;
-            }
-
             break;
         }
 
@@ -677,7 +754,7 @@ static void *input_thread(void *arg)
         if (d->readrate)
             readrate_sleep(d);
 
-        ret = demux_send(d, ds, dt.pkt_demux, send_flags);
+        ret = demux_send(d, &dt, ds, dt.pkt_demux, send_flags);
         if (ret < 0)
             break;
     }
@@ -735,9 +812,11 @@ static void demux_final_stats(Demuxer *d)
 static void ist_free(InputStream **pist)
 {
     InputStream *ist = *pist;
+    DemuxStream *ds;
 
     if (!ist)
         return;
+    ds = ds_from_ist(ist);
 
     dec_free(&ist->decoder);
 
@@ -749,6 +828,8 @@ static void ist_free(InputStream **pist)
     avcodec_free_context(&ist->dec_ctx);
     avcodec_parameters_free(&ist->par);
 
+    av_bsf_free(&ds->bsf);
+
     av_freep(pist);
 }
 
@@ -1039,6 +1120,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     const char *hwaccel = NULL;
     char *hwaccel_output_format = NULL;
     char *codec_tag = NULL;
+    char *bsfs = NULL;
     char *next;
     char *discard_str = NULL;
     int ret;
@@ -1258,6 +1340,33 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
         return ret;
     }
 
+    MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, ic, st);
+    if (bsfs) {
+        ret = av_bsf_list_parse_str(bsfs, &ds->bsf);
+        if (ret < 0) {
+            av_log(ist, AV_LOG_ERROR,
+                   "Error parsing bitstream filter sequence '%s': %s\n",
+                   bsfs, av_err2str(ret));
+            return ret;
+        }
+
+        ret = avcodec_parameters_copy(ds->bsf->par_in, ist->par);
+        if (ret < 0)
+            return ret;
+        ds->bsf->time_base_in = ist->st->time_base;
+
+        ret = av_bsf_init(ds->bsf);
+        if (ret < 0) {
+            av_log(ist, AV_LOG_ERROR, "Error initializing bitstream filters: %s\n",
+                   av_err2str(ret));
+            return ret;
+        }
+
+        ret = avcodec_parameters_copy(ist->par, ds->bsf->par_out);
+        if (ret < 0)
+            return ret;
+    }
+
     ds->codec_desc = avcodec_descriptor_get(ist->par->codec_id);
 
     return 0;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index c189cf373b..76b50c0bad 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1919,7 +1919,7 @@ const OptionDef options[] = {
         "0 = use frame rate (video) or sample rate (audio),"
         "-1 = match source time base", "ratio" },
 
-    { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT,
+    { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT | OPT_INPUT,
         { .off = OFFSET(bitstream_filters) },
         "A comma-separated list of bitstream filters", "bitstream_filters", },
 
diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index 1bfd5c1b31..df955df4d0 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -256,3 +256,8 @@ FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MPEGVIDEO, MPEG2VIDEO) += fate-ffmpeg-input
 fate-ffmpeg-error-rate-fail: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null -; test $$? -eq 69
 fate-ffmpeg-error-rate-pass: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null - -max_error_rate 1
 FATE_SAMPLES_FFMPEG-$(call ENCDEC, PCM_S16LE TTA, NULL MATROSKA) += fate-ffmpeg-error-rate-fail fate-ffmpeg-error-rate-pass
+
+# test input -bsf
+# use -stream_loop, because it tests flushing bsfs
+fate-ffmpeg-bsf-input: CMD = framecrc -stream_loop 2 -bsf setts=PTS*2 -i $(TARGET_SAMPLES)/hevc/extradata-reload-multi-stsd.mov -c copy
+FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MOV, , SETTS_BSF) += fate-ffmpeg-bsf-input
diff --git a/tests/ref/fate/ffmpeg-bsf-input b/tests/ref/fate/ffmpeg-bsf-input
new file mode 100644
index 0000000000..67dd57cf6d
--- /dev/null
+++ b/tests/ref/fate/ffmpeg-bsf-input
@@ -0,0 +1,18 @@
+#extradata 0:      110, 0xb4031479
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: hevc
+#dimensions 0: 128x128
+#sar 0: 1/1
+0,          0,          0,        1,     2108, 0x57c38f64
+0,          2,          2,        1,       31, 0xabe10d25, F=0x0
+0,          4,          4,        1,     1915, 0xd430347f, S=1,      109
+0,          6,          6,        1,       35, 0xc4ad0d4c, F=0x0
+0,          8,          8,        1,     2108, 0x57c38f64, S=1,      110
+0,         10,         10,        1,       31, 0xabe10d25, F=0x0
+0,         12,         12,        1,     1915, 0xd430347f, S=1,      109
+0,         14,         14,        1,       35, 0xc4ad0d4c, F=0x0
+0,         16,         16,        1,     2108, 0x57c38f64, S=1,      110
+0,         18,         18,        1,       31, 0xabe10d25, F=0x0
+0,         20,         20,        1,     1915, 0xd430347f, S=1,      109
+0,         22,         22,        1,       35, 0xc4ad0d4c, F=0x0
-- 
2.42.0

_______________________________________________
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 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option

[Anton Khirnov]

---
 fftools/ffmpeg_opt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 76b50c0bad..ea995f2b5f 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1531,7 +1531,7 @@ const OptionDef options[] = {
     { "program",                OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT,
         { .off = OFFSET(program) },
         "add program with specified streams", "title=string:st=number..." },
-    { "stream_group",           OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT,
+    { "stream_group",           OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT | OPT_EXPERT,
         { .off = OFFSET(stream_groups) },
         "add stream group with specified streams and group type-specific arguments", "id=number:st=number..." },
     { "dframes",                OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_PERFILE | OPT_EXPERT | OPT_OUTPUT | OPT_HAS_CANON,
-- 
2.42.0

_______________________________________________
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 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0)

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:44 +0100, Anton Khirnov wrote:
> This is the standard way to mark unreachable cases in a switch
> ---
>  fftools/ffmpeg_demux.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 95af31e9ef..5d07b7153d 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -1223,8 +1223,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      case AVMEDIA_TYPE_ATTACHMENT:
>      case AVMEDIA_TYPE_UNKNOWN:
>          break;
> -    default:
> -        abort();
> +    default: av_assert0(0);

LGTM, also probably we might employ a self-documentation trick of the
kind:
av_assert0(!"handled media type");
_______________________________________________
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 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:45 +0100, Anton Khirnov wrote:
> ---
>  doc/ffmpeg.texi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 7246a46d2f..d75517b443 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -1643,8 +1643,6 @@ option to disable streams individually.
>  As an output option, disables subtitle recording i.e. automatic selection or
>  mapping of any subtitle stream. For full manual control see the @code{-map}
>  option.
> -@item -sbsf @var{bitstream_filter}
> -Deprecated, see -bsf
>  @end table

LGTM given this has been deprecated for twelve years and counting.
_______________________________________________
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 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:46 +0100, Anton Khirnov wrote:
> Add it to decoder options instead, to be processed when opening the
> decoder. This way it won't be overridden by flags the user might be
> setting otherwise.
> ---
>  fftools/ffmpeg_demux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 5d07b7153d..cacdc76a71 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -1156,7 +1156,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      }
>  
>      if (o->bitexact)
> -        ist->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
> +        av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY);

LGTM.
_______________________________________________
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 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:47 +0100, Anton Khirnov wrote:
> This avoids the requirement to always have a decoder context.
> ---
>  fftools/ffmpeg_demux.c | 43 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index cacdc76a71..892094c512 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -899,19 +899,18 @@ static int choose_decoder(const OptionsContext *o, AVFormatContext *s, AVStream
>      }
>  }
>  
> -static int guess_input_channel_layout(InputStream *ist, int guess_layout_max)
> +static int guess_input_channel_layout(InputStream *ist, AVCodecParameters *par,
> +                                      int guess_layout_max)
>  {
> -    AVCodecContext *dec = ist->dec_ctx;
> -
> -    if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> +    if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
>          char layout_name[256];
>  
> -        if (dec->ch_layout.nb_channels > guess_layout_max)
> +        if (par->ch_layout.nb_channels > guess_layout_max)
>              return 0;
> -        av_channel_layout_default(&dec->ch_layout, dec->ch_layout.nb_channels);
> -        if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC)
> +        av_channel_layout_default(&par->ch_layout, par->ch_layout.nb_channels);
> +        if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC)
>              return 0;
> -        av_channel_layout_describe(&dec->ch_layout, layout_name, sizeof(layout_name));
> +        av_channel_layout_describe(&par->ch_layout, layout_name, sizeof(layout_name));
>          av_log(ist, AV_LOG_WARNING, "Guessed Channel Layout: %s\n", layout_name);
>      }
>      return 1;
> @@ -1145,16 +1144,6 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>          ist->user_set_discard = ist->st->discard;
>      }
>  
> -    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> -    if (!ist->dec_ctx)
> -        return AVERROR(ENOMEM);
> -
> -    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
> -    if (ret < 0) {
> -        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> -        return ret;
> -    }
> -
>      if (o->bitexact)
>          av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY);
>  
> @@ -1181,7 +1170,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      case AVMEDIA_TYPE_AUDIO: {
>          int guess_layout_max = INT_MAX;
>          MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st);
> -        guess_input_channel_layout(ist, guess_layout_max);
> +        guess_input_channel_layout(ist, par, guess_layout_max);
>          break;
>      }
>      case AVMEDIA_TYPE_DATA:
> @@ -1190,7 +1179,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>          MATCH_PER_STREAM_OPT(fix_sub_duration, i, ist->fix_sub_duration, ic, st);
>          MATCH_PER_STREAM_OPT(canvas_sizes, str, canvas_size, ic, st);
>          if (canvas_size) {
> -            ret = av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height,
> +            ret = av_parse_video_size(&par->width, &par->height,
>                                        canvas_size);
>              if (ret < 0) {
>                  av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size);
> @@ -1201,8 +1190,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>          /* Compute the size of the canvas for the subtitles stream.
>             If the subtitles codecpar has set a size, use it. Otherwise use the
>             maximum dimensions of the video streams in the same file. */
> -        ist->sub2video.w = ist->dec_ctx->width;
> -        ist->sub2video.h = ist->dec_ctx->height;
> +        ist->sub2video.w = par->width;
> +        ist->sub2video.h = par->height;
>          if (!(ist->sub2video.w && ist->sub2video.h)) {
>              for (int j = 0; j < ic->nb_streams; j++) {
>                  AVCodecParameters *par1 = ic->streams[j]->codecpar;
> @@ -1226,6 +1215,16 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      default: av_assert0(0);
>      }
>  
> +    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> +    if (!ist->dec_ctx)
> +        return AVERROR(ENOMEM);
> +
> +    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
> +    if (ret < 0) {
> +        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> +        return ret;
> +    }
> +

LGTM.
_______________________________________________
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 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:48 +0100, Anton Khirnov wrote:
> It is not needed otherwise.
> ---
>  fftools/ffmpeg_demux.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 892094c512..c51140b1c5 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed)
>      if (decoding_needed && ds->sch_idx_dec < 0) {
>          int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
>  
> +        ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> +        if (!ist->dec_ctx)
> +            return AVERROR(ENOMEM);
> +
> +        ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par);
> +        if (ret < 0) {

> +            av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");

unrelated, here and below: might be useful to notify the failing
stream identifier, assuming it is not already shown in the log

> +            return ret;
> +        }
> +
>          ret = sch_add_dec(d->sch, decoder_thread, ist, d->loop && is_audio);
>          if (ret < 0)
>              return ret;
> @@ -1215,23 +1225,13 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      default: av_assert0(0);
>      }
>  
> -    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> -    if (!ist->dec_ctx)
> -        return AVERROR(ENOMEM);
> -
> -    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
> -    if (ret < 0) {
> -        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> -        return ret;
> -    }
> -
>      ist->par = avcodec_parameters_alloc();
>      if (!ist->par)
>          return AVERROR(ENOMEM);
>  
> -    ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx);
> +    ret = avcodec_parameters_copy(ist->par, par);
>      if (ret < 0) {
> -        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> +        av_log(ist, AV_LOG_ERROR, "Error exporting stream parameters.\n");
>          return ret;
>      }

LGTM anyway.
_______________________________________________
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 6/8] fftools/ffmpeg_demux: add demuxing thread private data

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:49 +0100, Anton Khirnov wrote:
> To be used for data that never needs to be visible outside of the
> demuxer thread, similarly as was previously done for other components.
> ---
>  fftools/ffmpeg_demux.c | 67 ++++++++++++++++++++++++++++--------------
>  1 file changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index c51140b1c5..eae1f0bde5 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -115,6 +115,11 @@ typedef struct Demuxer {
>      int                   nb_streams_finished;
>  } Demuxer;
>  

> +typedef struct DemuxThreadContext {
> +    // packet used for reading from the demuxer

> +    AVPacket *pkt_demux;

nit: you might drop the _demux suffix since this is already clear from
the context and it's adding no information

> +} DemuxThreadContext;
> +
>  static DemuxStream *ds_from_ist(InputStream *ist)
>  {
>      return (DemuxStream*)ist;
> @@ -565,18 +570,36 @@ static void thread_set_name(InputFile *f)
>      ff_thread_setname(name);
>  }
>  
> +static void demux_thread_uninit(DemuxThreadContext *dt)
> +{
> +    av_packet_free(&dt->pkt_demux);
> +
> +    memset(dt, 0, sizeof(*dt));
> +}
> +
> +static int demux_thread_init(DemuxThreadContext *dt)
> +{
> +    memset(dt, 0, sizeof(*dt));
> +
> +    dt->pkt_demux = av_packet_alloc();
> +    if (!dt->pkt_demux)
> +        return AVERROR(ENOMEM);
> +
> +    return 0;
> +}
> +
>  static void *input_thread(void *arg)
>  {
>      Demuxer   *d = arg;
>      InputFile *f = &d->f;
> -    AVPacket *pkt;
> +
> +    DemuxThreadContext dt;
> +

nit++: weird style, you might drop the empty lines around the
declaration

>      int ret = 0;

>  
> -    pkt = av_packet_alloc();
> -    if (!pkt) {
> -        ret = AVERROR(ENOMEM);
> +    ret = demux_thread_init(&dt);
> +    if (ret < 0)
>          goto finish;
> -    }
>  
>      thread_set_name(f);
>  
> @@ -589,7 +612,7 @@ static void *input_thread(void *arg)
>          DemuxStream *ds;
>          unsigned send_flags = 0;
>  
> -        ret = av_read_frame(f->ctx, pkt);
> +        ret = av_read_frame(f->ctx, dt.pkt_demux);
>  
>          if (ret == AVERROR(EAGAIN)) {
>              av_usleep(10000);
> @@ -598,12 +621,12 @@ static void *input_thread(void *arg)
>          if (ret < 0) {
>              if (d->loop) {
>                  /* signal looping to our consumers */
> -                pkt->stream_index = -1;
> +                dt.pkt_demux->stream_index = -1;
>  
> -                ret = sch_demux_send(d->sch, f->index, pkt, 0);
> +                ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0);
>                  if (ret >= 0)
> -                    ret = seek_to_start(d, (Timestamp){ .ts = pkt->pts,
> -                                                        .tb = pkt->time_base });
> +                    ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts,
> +                                                        .tb = dt.pkt_demux->time_base });
>                  if (ret >= 0)
>                      continue;
>  
> @@ -622,39 +645,39 @@ static void *input_thread(void *arg)
>          }
>  
>          if (do_pkt_dump) {
> -            av_pkt_dump_log2(NULL, AV_LOG_INFO, pkt, do_hex_dump,
> -                             f->ctx->streams[pkt->stream_index]);
> +            av_pkt_dump_log2(NULL, AV_LOG_INFO, dt.pkt_demux, do_hex_dump,
> +                             f->ctx->streams[dt.pkt_demux->stream_index]);
>          }
>  
>          /* the following test is needed in case new streams appear
>             dynamically in stream : we ignore them */
> -        ds = pkt->stream_index < f->nb_streams ?
> -             ds_from_ist(f->streams[pkt->stream_index]) : NULL;
> +        ds = dt.pkt_demux->stream_index < f->nb_streams ?
> +             ds_from_ist(f->streams[dt.pkt_demux->stream_index]) : NULL;
>          if (!ds || ds->discard || ds->finished) {
> -            report_new_stream(d, pkt);
> -            av_packet_unref(pkt);
> +            report_new_stream(d, dt.pkt_demux);
> +            av_packet_unref(dt.pkt_demux);
>              continue;
>          }
>  
> -        if (pkt->flags & AV_PKT_FLAG_CORRUPT) {
> +        if (dt.pkt_demux->flags & AV_PKT_FLAG_CORRUPT) {
>              av_log(d, exit_on_error ? AV_LOG_FATAL : AV_LOG_WARNING,
>                     "corrupt input packet in stream %d\n",
> -                   pkt->stream_index);
> +                   dt.pkt_demux->stream_index);
>              if (exit_on_error) {
> -                av_packet_unref(pkt);
> +                av_packet_unref(dt.pkt_demux);
>                  ret = AVERROR_INVALIDDATA;
>                  break;
>              }
>          }
>  
> -        ret = input_packet_process(d, pkt, &send_flags);
> +        ret = input_packet_process(d, dt.pkt_demux, &send_flags);
>          if (ret < 0)
>              break;
>  
>          if (d->readrate)
>              readrate_sleep(d);
>  
> -        ret = demux_send(d, ds, pkt, send_flags);
> +        ret = demux_send(d, ds, dt.pkt_demux, send_flags);
>          if (ret < 0)
>              break;
>      }
> @@ -664,7 +687,7 @@ static void *input_thread(void *arg)
>          ret = 0;
>  
>  finish:
> -    av_packet_free(&pkt);
> +    demux_thread_uninit(&dt);

LGTM if useful for the following changes.
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:50 +0100, Anton Khirnov wrote:
> Previously bitstream filters could only be applied right before muxing,
> this allows to apply them right after demuxing.
> ---
>  Changelog                       |   1 +
>  doc/ffmpeg.texi                 |  22 +++--
>  fftools/ffmpeg_demux.c          | 139 ++++++++++++++++++++++++++++----
>  fftools/ffmpeg_opt.c            |   2 +-
>  tests/fate/ffmpeg.mak           |   5 ++
>  tests/ref/fate/ffmpeg-bsf-input |  18 +++++
>  6 files changed, 164 insertions(+), 23 deletions(-)
>  create mode 100644 tests/ref/fate/ffmpeg-bsf-input
> 
> diff --git a/Changelog b/Changelog
> index 5b2899d05b..f8191d88c7 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -18,6 +18,7 @@ version <next>:
>  - lavu/eval: introduce randomi() function in expressions
>  - VVC decoder
>  - fsync filter
> +- ffmpeg CLI -bsf option may now be used for input as well as output
>  
>  version 6.1:
>  - libaribcaption decoder
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index d75517b443..59f7badcb6 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -2093,26 +2093,34 @@ an output mpegts file:
>  ffmpeg -i inurl -streamid 0:33 -streamid 1:36 out.ts
>  @end example
>  
> -@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{output,per-stream})
> -Apply bitstream filters to matching streams.
> +@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{input/output,per-stream})
> +Apply bitstream filters to matching streams. The filters are applied to each
> +packet as it is received from the demuxer (when used as an input option) or
> +before it is sent to the muxer (when used as an output option).
>  
>  @var{bitstream_filters} is a comma-separated list of bitstream filter
> -specifications. The specified bitstream filters are applied to coded packets in
> -the order they are written in. Each bitstream filter specification is of the
> -form
> +specifications, each of the form
>  @example
>  @var{filter}[=@var{optname0}=@var{optval0}:@var{optname1}=@var{optval1}:...]
>  @end example
>  Any of the ',=:' characters that are to be a part of an option value need to be
>  escaped with a backslash.
>  
> -Use the @code{-bsfs} option to get the list of bitstream filters.
> +Use the @code{-bsfs} option to get the list of bitstream filters. E.g.

This looks spurious, since this suggests the example is about the
listing, and it's applying a weird order of example/explanation
(rather than the opposite).

What about something as:
--------------------------
[...]
Use the @code{-bsfs} option to get the list of bitstream filters.

Some examples follow.

To apply the @code{h264_mp4toannexb} bitstream filter (which converts
MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream:
@example
...
@end example

To apply the @code{mov2textsub} bitstream filter (which extracts text from MOV
subtitles) to the @emph{output} subtitle stream:
@example
...
@end example

Note, however, that since both examples use @code{-c copy}, it matters
little whether the filters are applied on input or output - that would
change if transcoding was happening.
--------------------------

[...]
> +on input or output - that would change if transcoding was hapenning.

hapenning typo

>  
>  @item -tag[:@var{stream_specifier}] @var{codec_tag} (@emph{input/output,per-stream})
>  Force a tag/fourcc for matching streams.
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index eae1f0bde5..16d4f67e59 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -34,6 +34,7 @@
>  #include "libavutil/time.h"
>  #include "libavutil/timestamp.h"
>  
> +#include "libavcodec/bsf.h"
>  #include "libavcodec/packet.h"
>  
>  #include "libavformat/avformat.h"
> @@ -71,6 +72,8 @@ typedef struct DemuxStream {
>  
>      const AVCodecDescriptor *codec_desc;
>  
> +    AVBSFContext *bsf;
> +
>      /* number of packets successfully read for this stream */
>      uint64_t nb_packets;
>      // combined size of all the packets read
> @@ -118,6 +121,8 @@ typedef struct Demuxer {
>  typedef struct DemuxThreadContext {
>      // packet used for reading from the demuxer
>      AVPacket *pkt_demux;
> +    // packet for reading from BSFs
> +    AVPacket *pkt_bsf;
>  } DemuxThreadContext;
>  
>  static DemuxStream *ds_from_ist(InputStream *ist)
> @@ -513,13 +518,17 @@ static int do_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags,
>      return 0;
>  }
>  
> -static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags)
> +static int demux_send(Demuxer *d, DemuxThreadContext *dt, DemuxStream *ds,
> +                      AVPacket *pkt, unsigned flags)
>  {
>      InputFile  *f = &d->f;
>      int ret;
>  
> +    // pkt can be NULL only when flushing BSFs
> +    av_assert0(ds->bsf || pkt);
> +
>      // send heartbeat for sub2video streams
> -    if (d->pkt_heartbeat && pkt->pts != AV_NOPTS_VALUE) {
> +    if (d->pkt_heartbeat && pkt && pkt->pts != AV_NOPTS_VALUE) {
>          for (int i = 0; i < f->nb_streams; i++) {
>              DemuxStream *ds1 = ds_from_ist(f->streams[i]);
>  
> @@ -537,10 +546,69 @@ static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags
>          }
>      }
>  
> -    ret = do_send(d, ds, pkt, flags, "demuxed");
> -    if (ret < 0)
> -        return ret;
> +    if (ds->bsf) {
> +        if (pkt)
> +            av_packet_rescale_ts(pkt, pkt->time_base, ds->bsf->time_base_in);
>  
> +        ret = av_bsf_send_packet(ds->bsf, pkt);
> +        if (ret < 0) {
> +            if (pkt)
> +                av_packet_unref(pkt);

> +            av_log(ds, AV_LOG_ERROR, "Error submitting a packet for filtering: %s\n",

might be useful to signal the stream identifier

> +                   av_err2str(ret));

possibly redundant? (IIRC this is shown anyway in the outer level failure message)

> +            return ret;
> +        }
> +

> +        while (1) {
> +            ret = av_bsf_receive_packet(ds->bsf, dt->pkt_bsf);
> +            if (ret == AVERROR(EAGAIN))
> +                return 0;
> +            else if (ret < 0) {
> +                if (ret != AVERROR_EOF)

> +                    av_log(ds, AV_LOG_ERROR,
> +                           "Error applying bitstream filters to a packet: %s\n",
> +                           av_err2str(ret));

ditto

> +                return ret;
> +            }
> +
> +            dt->pkt_bsf->time_base = ds->bsf->time_base_out;
> +
> +            ret = do_send(d, ds, dt->pkt_bsf, 0, "filtered");
> +            if (ret < 0) {
> +                av_packet_unref(dt->pkt_bsf);
> +                return ret;
> +            }
> +        }
> +    } else {
> +        ret = do_send(d, ds, pkt, flags, "demuxed");
> +        if (ret < 0)
> +            return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +static int demux_bsf_flush(Demuxer *d, DemuxThreadContext *dt)
> +{
> +    InputFile *f = &d->f;
> +    int ret;
> +
> +    for (unsigned i = 0; i < f->nb_streams; i++) {
> +        DemuxStream *ds = ds_from_ist(f->streams[i]);
> +
> +        if (!ds->bsf)
> +            continue;
> +
> +        ret = demux_send(d, dt, ds, NULL, 0);
> +        ret = (ret == AVERROR_EOF) ? 0 : (ret < 0) ? ret : AVERROR_BUG;
> +        if (ret < 0) {

> +            av_log(ds, AV_LOG_ERROR, "Error flushing BSFs: %s\n",
> +                   av_err2str(ret));

ditto

> +            return ret;
> +        }
> +
> +        av_bsf_flush(ds->bsf);
> +    }
>  
>      return 0;
>  }
> @@ -573,6 +641,7 @@ static void thread_set_name(InputFile *f)
>  static void demux_thread_uninit(DemuxThreadContext *dt)
>  {
>      av_packet_free(&dt->pkt_demux);
> +    av_packet_free(&dt->pkt_bsf);
>  
>      memset(dt, 0, sizeof(*dt));
>  }
> @@ -585,6 +654,10 @@ static int demux_thread_init(DemuxThreadContext *dt)
>      if (!dt->pkt_demux)
>          return AVERROR(ENOMEM);
>  
> +    dt->pkt_bsf = av_packet_alloc();
> +    if (!dt->pkt_bsf)
> +        return AVERROR(ENOMEM);
> +
>      return 0;
>  }
>  
> @@ -619,10 +692,22 @@ static void *input_thread(void *arg)
>              continue;
>          }
>          if (ret < 0) {
> +            int ret_bsf;
> +
> +            if (ret == AVERROR_EOF)
> +                av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n");
> +            else {
> +                av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n",
> +                       av_err2str(ret));
> +                ret = exit_on_error ? ret : 0;
> +            }
> +
> +            ret_bsf = demux_bsf_flush(d, &dt);
> +            ret = err_merge(ret == AVERROR_EOF ? 0 : ret, ret_bsf);
> +
>              if (d->loop) {
>                  /* signal looping to our consumers */
>                  dt.pkt_demux->stream_index = -1;
> -
>                  ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0);
>                  if (ret >= 0)
>                      ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts,
> @@ -633,14 +718,6 @@ static void *input_thread(void *arg)
>                  /* fallthrough to the error path */
>              }
>  
> -            if (ret == AVERROR_EOF)
> -                av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n");
> -            else {
> -                av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n",
> -                       av_err2str(ret));
> -                ret = exit_on_error ? ret : 0;
> -            }
> -
>              break;
>          }
>  
> @@ -677,7 +754,7 @@ static void *input_thread(void *arg)
>          if (d->readrate)
>              readrate_sleep(d);
>  
> -        ret = demux_send(d, ds, dt.pkt_demux, send_flags);
> +        ret = demux_send(d, &dt, ds, dt.pkt_demux, send_flags);
>          if (ret < 0)
>              break;
>      }
> @@ -735,9 +812,11 @@ static void demux_final_stats(Demuxer *d)
>  static void ist_free(InputStream **pist)
>  {
>      InputStream *ist = *pist;
> +    DemuxStream *ds;
>  
>      if (!ist)
>          return;
> +    ds = ds_from_ist(ist);
>  
>      dec_free(&ist->decoder);
>  
> @@ -749,6 +828,8 @@ static void ist_free(InputStream **pist)
>      avcodec_free_context(&ist->dec_ctx);
>      avcodec_parameters_free(&ist->par);
>  
> +    av_bsf_free(&ds->bsf);
> +
>      av_freep(pist);
>  }
>  
> @@ -1039,6 +1120,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      const char *hwaccel = NULL;
>      char *hwaccel_output_format = NULL;
>      char *codec_tag = NULL;
> +    char *bsfs = NULL;
>      char *next;
>      char *discard_str = NULL;
>      int ret;
> @@ -1258,6 +1340,33 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>          return ret;
>      }
>  
> +    MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, ic, st);
> +    if (bsfs) {
> +        ret = av_bsf_list_parse_str(bsfs, &ds->bsf);
> +        if (ret < 0) {
> +            av_log(ist, AV_LOG_ERROR,
> +                   "Error parsing bitstream filter sequence '%s': %s\n",
> +                   bsfs, av_err2str(ret));
> +            return ret;
> +        }
> +
> +        ret = avcodec_parameters_copy(ds->bsf->par_in, ist->par);
> +        if (ret < 0)
> +            return ret;
> +        ds->bsf->time_base_in = ist->st->time_base;
> +
> +        ret = av_bsf_init(ds->bsf);
> +        if (ret < 0) {
> +            av_log(ist, AV_LOG_ERROR, "Error initializing bitstream filters: %s\n",
> +                   av_err2str(ret));
> +            return ret;
> +        }
> +
> +        ret = avcodec_parameters_copy(ist->par, ds->bsf->par_out);
> +        if (ret < 0)
> +            return ret;
> +    }
> +
>      ds->codec_desc = avcodec_descriptor_get(ist->par->codec_id);
>  
>      return 0;
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index c189cf373b..76b50c0bad 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1919,7 +1919,7 @@ const OptionDef options[] = {
>          "0 = use frame rate (video) or sample rate (audio),"
>          "-1 = match source time base", "ratio" },
>  
> -    { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT,
> +    { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT | OPT_INPUT,
>          { .off = OFFSET(bitstream_filters) },
>          "A comma-separated list of bitstream filters", "bitstream_filters", },
>  
> diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
> index 1bfd5c1b31..df955df4d0 100644
> --- a/tests/fate/ffmpeg.mak
> +++ b/tests/fate/ffmpeg.mak
> @@ -256,3 +256,8 @@ FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MPEGVIDEO, MPEG2VIDEO) += fate-ffmpeg-input
>  fate-ffmpeg-error-rate-fail: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null -; test $$? -eq 69
>  fate-ffmpeg-error-rate-pass: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null - -max_error_rate 1
>  FATE_SAMPLES_FFMPEG-$(call ENCDEC, PCM_S16LE TTA, NULL MATROSKA) += fate-ffmpeg-error-rate-fail fate-ffmpeg-error-rate-pass
> +
> +# test input -bsf
> +# use -stream_loop, because it tests flushing bsfs
> +fate-ffmpeg-bsf-input: CMD = framecrc -stream_loop 2 -bsf setts=PTS*2 -i $(TARGET_SAMPLES)/hevc/extradata-reload-multi-stsd.mov -c copy
> +FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MOV, , SETTS_BSF) += fate-ffmpeg-bsf-input
> diff --git a/tests/ref/fate/ffmpeg-bsf-input b/tests/ref/fate/ffmpeg-bsf-input
> new file mode 100644
> index 0000000000..67dd57cf6d
> --- /dev/null
> +++ b/tests/ref/fate/ffmpeg-bsf-input
> @@ -0,0 +1,18 @@
> +#extradata 0:      110, 0xb4031479
> +#tb 0: 1/25
> +#media_type 0: video
> +#codec_id 0: hevc
> +#dimensions 0: 128x128
> +#sar 0: 1/1
> +0,          0,          0,        1,     2108, 0x57c38f64
> +0,          2,          2,        1,       31, 0xabe10d25, F=0x0
> +0,          4,          4,        1,     1915, 0xd430347f, S=1,      109
> +0,          6,          6,        1,       35, 0xc4ad0d4c, F=0x0
> +0,          8,          8,        1,     2108, 0x57c38f64, S=1,      110
> +0,         10,         10,        1,       31, 0xabe10d25, F=0x0
> +0,         12,         12,        1,     1915, 0xd430347f, S=1,      109
> +0,         14,         14,        1,       35, 0xc4ad0d4c, F=0x0
> +0,         16,         16,        1,     2108, 0x57c38f64, S=1,      110
> +0,         18,         18,        1,       31, 0xabe10d25, F=0x0
> +0,         20,         20,        1,     1915, 0xd430347f, S=1,      109
> +0,         22,         22,        1,       35, 0xc4ad0d4c, F=0x0

LGTM otherwise (and nice feature!).
_______________________________________________
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 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option

[Stefano Sabatini]

On date Friday 2024-01-05 17:42:51 +0100, Anton Khirnov wrote:
> ---
>  fftools/ffmpeg_opt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 76b50c0bad..ea995f2b5f 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1531,7 +1531,7 @@ const OptionDef options[] = {
>      { "program",                OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT,
>          { .off = OFFSET(program) },
>          "add program with specified streams", "title=string:st=number..." },
> -    { "stream_group",           OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT,
> +    { "stream_group",           OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT | OPT_EXPERT,
>          { .off = OFFSET(stream_groups) },
>          "add stream group with specified streams and group type-specific arguments", "id=number:st=number..." },
>      { "dframes",                OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_PERFILE | OPT_EXPERT | OPT_OUTPUT | OPT_HAS_CANON,

Possibly ok.
_______________________________________________
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 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder

[James Almer]

On 1/5/2024 1:42 PM, Anton Khirnov wrote:
> This avoids the requirement to always have a decoder context.
> ---
>   fftools/ffmpeg_demux.c | 43 +++++++++++++++++++++---------------------
>   1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index cacdc76a71..892094c512 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -899,19 +899,18 @@ static int choose_decoder(const OptionsContext *o, AVFormatContext *s, AVStream
>       }
>   }
>   
> -static int guess_input_channel_layout(InputStream *ist, int guess_layout_max)
> +static int guess_input_channel_layout(InputStream *ist, AVCodecParameters *par,
> +                                      int guess_layout_max)
>   {
> -    AVCodecContext *dec = ist->dec_ctx;
> -
> -    if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> +    if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
>           char layout_name[256];
>   
> -        if (dec->ch_layout.nb_channels > guess_layout_max)
> +        if (par->ch_layout.nb_channels > guess_layout_max)
>               return 0;
> -        av_channel_layout_default(&dec->ch_layout, dec->ch_layout.nb_channels);
> -        if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC)
> +        av_channel_layout_default(&par->ch_layout, par->ch_layout.nb_channels);
> +        if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC)
>               return 0;
> -        av_channel_layout_describe(&dec->ch_layout, layout_name, sizeof(layout_name));
> +        av_channel_layout_describe(&par->ch_layout, layout_name, sizeof(layout_name));
>           av_log(ist, AV_LOG_WARNING, "Guessed Channel Layout: %s\n", layout_name);
>       }
>       return 1;
> @@ -1145,16 +1144,6 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>           ist->user_set_discard = ist->st->discard;
>       }
>   
> -    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> -    if (!ist->dec_ctx)
> -        return AVERROR(ENOMEM);
> -
> -    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
> -    if (ret < 0) {
> -        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> -        return ret;
> -    }
> -
>       if (o->bitexact)
>           av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY);
>   
> @@ -1181,7 +1170,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>       case AVMEDIA_TYPE_AUDIO: {
>           int guess_layout_max = INT_MAX;
>           MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st);
> -        guess_input_channel_layout(ist, guess_layout_max);
> +        guess_input_channel_layout(ist, par, guess_layout_max);
>           break;
>       }
>       case AVMEDIA_TYPE_DATA:
> @@ -1190,7 +1179,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>           MATCH_PER_STREAM_OPT(fix_sub_duration, i, ist->fix_sub_duration, ic, st);
>           MATCH_PER_STREAM_OPT(canvas_sizes, str, canvas_size, ic, st);
>           if (canvas_size) {
> -            ret = av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height,
> +            ret = av_parse_video_size(&par->width, &par->height,
>                                         canvas_size);
>               if (ret < 0) {
>                   av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size);
> @@ -1201,8 +1190,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>           /* Compute the size of the canvas for the subtitles stream.
>              If the subtitles codecpar has set a size, use it. Otherwise use the
>              maximum dimensions of the video streams in the same file. */
> -        ist->sub2video.w = ist->dec_ctx->width;
> -        ist->sub2video.h = ist->dec_ctx->height;
> +        ist->sub2video.w = par->width;
> +        ist->sub2video.h = par->height;
>           if (!(ist->sub2video.w && ist->sub2video.h)) {
>               for (int j = 0; j < ic->nb_streams; j++) {
>                   AVCodecParameters *par1 = ic->streams[j]->codecpar;
> @@ -1226,6 +1215,16 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>       default: av_assert0(0);
>       }
>   
> +    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> +    if (!ist->dec_ctx)
> +        return AVERROR(ENOMEM);
> +
> +    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
> +    if (ret < 0) {
> +        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> +        return ret;
> +    }
> +
>       ist->par = avcodec_parameters_alloc();
>       if (!ist->par)
>           return AVERROR(ENOMEM);

This does not fix the issue of the guessed channel layout not being 
present on the output stream, but might be a change in the right direction.

before:

> $ ./ffmpeg -i ~/samples/wav/200828-005.wav out.wav
> [aist#0:0/pcm_s16le @ 00000252b1d2d420] Guessed Channel Layout: stereo
> Input #0, wav, from '../samples/wav/200828-005.wav':
>   Duration: 00:00:12.80, bitrate: 1536 kb/s
>   Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, 2 channels, s16, 1536 kb/s
> Stream mapping:
>   Stream #0:0 -> #0:0 (pcm_s16le (native) -> pcm_s16le (native))
> Press [q] to stop, [?] for help
> Output #0, wav, to 'out.wav':
>   Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s
> 
> 
> $ ./ffmpeg -i out.wav
> [aist#0:0/pcm_s16le @ 000001d616b1d3e0] Guessed Channel Layout: stereo
>   Duration: 00:00:12.80, bitrate: 1536 kb/s
>   Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, 2 channels, s16, 1536 kb/s

After:

> $ ./ffmpeg -i ../samples/wav/200828-005.wav out.wav
> [aist#0:0/pcm_s16le @ 0000024603c9d420] Guessed Channel Layout: stereo
> Input #0, wav, from '../samples/wav/200828-005.wav':
>   Duration: 00:00:12.80, bitrate: 1536 kb/
>   Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s
> Stream mapping:
>   Stream #0:0 -> #0:0 (pcm_s16le (native) -> pcm_s16le (native))
> Press [q] to stop, [?] for help
> Output #0, wav, to 'out.wav':
>   Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s
> 
> $ ./ffmpeg -i out.wav
> [aist#0:0/pcm_s16le @ 000001a1467ed3e0] Guessed Channel Layout: stereo
>   Duration: 00:00:12.80, bitrate: 1536 kb/s
>   Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s

So the printed output shows the guessed layout for the input stream, but 
the written output stream is still unspec.
_______________________________________________
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 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder

[Anton Khirnov]

Quoting James Almer (2024-01-06 15:22:10)
> This does not fix the issue of the guessed channel layout not being 
> present on the output stream,

Well, it wasn't supposed to fix anything. This patch exists as a
prerequisite for the next one.

-- 
Anton Khirnov
_______________________________________________
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 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-06 12:34:53)
> On date Friday 2024-01-05 17:42:48 +0100, Anton Khirnov wrote:
> > It is not needed otherwise.
> > ---
> >  fftools/ffmpeg_demux.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> > index 892094c512..c51140b1c5 100644
> > --- a/fftools/ffmpeg_demux.c
> > +++ b/fftools/ffmpeg_demux.c
> > @@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed)
> >      if (decoding_needed && ds->sch_idx_dec < 0) {
> >          int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
> >  
> > +        ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> > +        if (!ist->dec_ctx)
> > +            return AVERROR(ENOMEM);
> > +
> > +        ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par);
> > +        if (ret < 0) {
> 
> > +            av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> 
> unrelated, here and below: might be useful to notify the failing
> stream identifier, assuming it is not already shown in the log

It is, logging to ist prints something like
[aist#0:0/pcm_s16le @ 0x563a40304b40] ....

-- 
Anton Khirnov
_______________________________________________
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 6/8] fftools/ffmpeg_demux: add demuxing thread private data

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-06 12:44:27)
> On date Friday 2024-01-05 17:42:49 +0100, Anton Khirnov wrote:
> > To be used for data that never needs to be visible outside of the
> > demuxer thread, similarly as was previously done for other components.
> > ---
> >  fftools/ffmpeg_demux.c | 67 ++++++++++++++++++++++++++++--------------
> >  1 file changed, 45 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> > index c51140b1c5..eae1f0bde5 100644
> > --- a/fftools/ffmpeg_demux.c
> > +++ b/fftools/ffmpeg_demux.c
> > @@ -115,6 +115,11 @@ typedef struct Demuxer {
> >      int                   nb_streams_finished;
> >  } Demuxer;
> >  
> 
> > +typedef struct DemuxThreadContext {
> > +    // packet used for reading from the demuxer
> 
> > +    AVPacket *pkt_demux;
> 
> nit: you might drop the _demux suffix since this is already clear from
> the context and it's adding no information

I'm adding the suffix because a following commit adds another packet to
the struct, so I prefer to make it clear what each one's role is.

> >  static void *input_thread(void *arg)
> >  {
> >      Demuxer   *d = arg;
> >      InputFile *f = &d->f;
> > -    AVPacket *pkt;
> > +
> > +    DemuxThreadContext dt;
> > +
> 
> nit++: weird style, you might drop the empty lines around the
> declaration

I prefer it this way.

-- 
Anton Khirnov
_______________________________________________
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 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder

[James Almer]

On 1/16/2024 4:49 PM, Anton Khirnov wrote:
> Quoting James Almer (2024-01-06 15:22:10)
>> This does not fix the issue of the guessed channel layout not being
>> present on the output stream,
> 
> Well, it wasn't supposed to fix anything. This patch exists as a
> prerequisite for the next one.

I know, hence the "but might be a change in the right direction." part 
followed by the changed output you removed from the quote :p
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-06 13:12:19)
> 
> This looks spurious, since this suggests the example is about the
> listing, and it's applying a weird order of example/explanation
> (rather than the opposite).

I see nothing weird about this order, it's the standard way it is done
in most literature I encounter. I find the reverse order you're
suggesting far more weird and unnatural.

-- 
Anton Khirnov
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Stefano Sabatini]

On date Wednesday 2024-01-17 10:02:31 +0100, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2024-01-06 13:12:19)
> > 
> > This looks spurious, since this suggests the example is about the
> > listing, and it's applying a weird order of example/explanation
> > (rather than the opposite).

Use the @code{-bsfs} option to get the list of bitstream filters. E.g.
@example
...

The problem here is that "E.g." is placed close to a statement about
the listing, therefore it might sound like the example is about the
listing (which is not).

> I see nothing weird about this order, it's the standard way it is done
> in most literature I encounter. I find the reverse order you're
> suggesting far more weird and unnatural.

When you present an example you usually start with an explanation
(what it does) and then present the command, not the other way around.

Also the following:
--------------------------------------
ffmpeg -bsf:v h264_mp4toannexb -i h264.mp4 -c:v copy -an out.h264
@end example
applies the @code{h264_mp4toannexb} bitstream filter (which converts
MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream.

On the other hand,
@example
ffmpeg -i file.mov -an -vn -bsf:s mov2textsub -c:s copy -f rawvideo sub.txt
@end example
applies the @code{mov2textsub} bitstream filter (which extracts text from MOV
subtitles) to the @emph{output} subtitle stream. Note, however, that since both
examples use @code{-c copy}, it matters little whether the filters are applied
on input or output - that would change if transcoding was hapenning.
---------------------------------------

this makes the reader need to correlate the two examples to figure
them out, that's why I reworked the presentation in my suggestion as a
more linear sequence of presentation/command/presentation/command.

In general examples should focus on how a task can be done, not on the
explanation of the command itself.
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-20 12:32:42)
> On date Wednesday 2024-01-17 10:02:31 +0100, Anton Khirnov wrote:
> > Quoting Stefano Sabatini (2024-01-06 13:12:19)
> > > 
> > > This looks spurious, since this suggests the example is about the
> > > listing, and it's applying a weird order of example/explanation
> > > (rather than the opposite).
> 
> Use the @code{-bsfs} option to get the list of bitstream filters. E.g.
> @example
> ...
> 
> The problem here is that "E.g." is placed close to a statement about
> the listing, therefore it might sound like the example is about the
> listing (which is not).

I moved it to a new paragraph.

> > I see nothing weird about this order, it's the standard way it is done
> > in most literature I encounter. I find the reverse order you're
> > suggesting far more weird and unnatural.
> 
> When you present an example you usually start with an explanation
> (what it does) and then present the command, not the other way around.

I don't, neither does most literature I can recall. Typically you first
present a thing, then explain its structure. Explaning the structure of
something the reader has not seen yet is backwards, unnatural, and hard
to understand.

> 
> Also the following:
> --------------------------------------
> ffmpeg -bsf:v h264_mp4toannexb -i h264.mp4 -c:v copy -an out.h264
> @end example
> applies the @code{h264_mp4toannexb} bitstream filter (which converts
> MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream.
> 
> On the other hand,
> @example
> ffmpeg -i file.mov -an -vn -bsf:s mov2textsub -c:s copy -f rawvideo sub.txt
> @end example
> applies the @code{mov2textsub} bitstream filter (which extracts text from MOV
> subtitles) to the @emph{output} subtitle stream. Note, however, that since both
> examples use @code{-c copy}, it matters little whether the filters are applied
> on input or output - that would change if transcoding was hapenning.
> ---------------------------------------
> 
> this makes the reader need to correlate the two examples to figure
> them out, that's why I reworked the presentation in my suggestion as a
> more linear sequence of presentation/command/presentation/command.
> 
> In general examples should focus on how a task can be done, not on the
> explanation of the command itself.

I disagree. Examples should focus on whatever can be usefully explained
with an example.

-- 
Anton Khirnov
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Stefano Sabatini]

On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2024-01-20 12:32:42)
[...]
> > When you present an example you usually start with an explanation
> > (what it does) and then present the command, not the other way around.
> 
> I don't, neither does most literature I can recall. Typically you first
> present a thing, then explain its structure. Explaning the structure of
> something the reader has not seen yet is backwards, unnatural, and hard
> to understand.

I still don't understand what "literature" you are referring to.

If you see most examples in the FFmpeg docs they are in the form:
@item
This does this and that...:
@example
...
@end example

An explanation is presented *before* introducing the example itself,
in other words plain English before the actual command/code.
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-21 19:22:35)
> On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote:
> > Quoting Stefano Sabatini (2024-01-20 12:32:42)
> [...]
> > > When you present an example you usually start with an explanation
> > > (what it does) and then present the command, not the other way around.
> > 
> > I don't, neither does most literature I can recall. Typically you first
> > present a thing, then explain its structure. Explaning the structure of
> > something the reader has not seen yet is backwards, unnatural, and hard
> > to understand.
> 
> I still don't understand what "literature" you are referring to.

Various manuals and textbooks I've read.

> If you see most examples in the FFmpeg docs they are in the form:

Our documentation is widely considered to be somewhere between atrocious
and unusable (and sometimes actively misleading), so the fact that it
does something in a specific way does not at all mean that it's a good
idea.

I have also personally seen (and fixed) countless instances of
mindlessly perpetuated cargo cults in it.

-- 
Anton Khirnov
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Stefano Sabatini]

On date Sunday 2024-01-21 19:35:01 +0100, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2024-01-21 19:22:35)
> > On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote:
> > > Quoting Stefano Sabatini (2024-01-20 12:32:42)
> > [...]
> > > > When you present an example you usually start with an explanation
> > > > (what it does) and then present the command, not the other way around.
> > > 
> > > I don't, neither does most literature I can recall. Typically you first
> > > present a thing, then explain its structure. Explaning the structure of
> > > something the reader has not seen yet is backwards, unnatural, and hard
> > > to understand.
> > 
> > I still don't understand what "literature" you are referring to.
> 
> Various manuals and textbooks I've read.
> 
> > If you see most examples in the FFmpeg docs they are in the form:
> 

> Our documentation is widely considered to be somewhere between atrocious
> and unusable

nah, it's not so bad, also this applies to most documentation

Besides FFmpeg is possibly the most sophisticated existing toolkit in
terms of features/configuration, so this is somehow expected (at least
if you expect a tutorial rather than a reference).

> (and sometimes actively misleading), so the fact that it
> does something in a specific way does not at all mean that it's a good
> idea.

So what do you propose instead? The fact that it is not perfect does
not mean that everything is bad.
_______________________________________________
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 7/8] fftools/ffmpeg_demux: implement -bsf for input

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-21 20:15:46)
> On date Sunday 2024-01-21 19:35:01 +0100, Anton Khirnov wrote:
> > Quoting Stefano Sabatini (2024-01-21 19:22:35)
> > > On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote:
> > > > Quoting Stefano Sabatini (2024-01-20 12:32:42)
> > > [...]
> > > > > When you present an example you usually start with an explanation
> > > > > (what it does) and then present the command, not the other way around.
> > > > 
> > > > I don't, neither does most literature I can recall. Typically you first
> > > > present a thing, then explain its structure. Explaning the structure of
> > > > something the reader has not seen yet is backwards, unnatural, and hard
> > > > to understand.
> > > 
> > > I still don't understand what "literature" you are referring to.
> > 
> > Various manuals and textbooks I've read.
> > 
> > > If you see most examples in the FFmpeg docs they are in the form:
> > 
> 
> > Our documentation is widely considered to be somewhere between atrocious
> > and unusable
> 
> nah, it's not so bad, also this applies to most documentation
> 
> Besides FFmpeg is possibly the most sophisticated existing toolkit in
> terms of features/configuration, so this is somehow expected (at least
> if you expect a tutorial rather than a reference).

I wouldn't be so sure. E.g. Qt has a bigger and more complex API than
ours, yet its documentation is more complete and coherent.

> > (and sometimes actively misleading), so the fact that it
> > does something in a specific way does not at all mean that it's a good
> > idea.
> 
> So what do you propose instead? The fact that it is not perfect does
> not mean that everything is bad.

I'm not saying everything is bad. I'm saying this specific way of
writing examples is bad and should be changed. Which is what I'm doing
in this patch.

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