Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions
@ 2022-01-11  9:58 Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 02/28] ffmpeg: store the output file index in OutputFile Anton Khirnov
                   ` (26 more replies)
  0 siblings, 27 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Stop accessing OutputFile.ctx. This will be useful in the following
commits, where it will become hidden.
---
 fftools/ffmpeg_opt.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 9c820ab73f..f638edace9 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2094,10 +2094,10 @@ static int opt_streamid(void *optctx, const char *opt, const char *arg)
     return 0;
 }
 
-static int copy_chapters(InputFile *ifile, OutputFile *ofile, int copy_metadata)
+static int copy_chapters(InputFile *ifile, OutputFile *ofile, AVFormatContext *os,
+                         int copy_metadata)
 {
     AVFormatContext *is = ifile->ctx;
-    AVFormatContext *os = ofile->ctx;
     AVChapter **tmp;
     int i;
 
@@ -2137,14 +2137,14 @@ static int copy_chapters(InputFile *ifile, OutputFile *ofile, int copy_metadata)
     return 0;
 }
 
-static int set_dispositions(OutputFile *of)
+static int set_dispositions(OutputFile *of, AVFormatContext *ctx)
 {
     int nb_streams[AVMEDIA_TYPE_NB]   = { 0 };
     int have_default[AVMEDIA_TYPE_NB] = { 0 };
     int have_manual = 0;
 
     // first, copy the input dispositions
-    for (int i = 0; i< of->ctx->nb_streams; i++) {
+    for (int i = 0; i < ctx->nb_streams; i++) {
         OutputStream *ost = output_streams[of->ost_index + i];
 
         nb_streams[ost->st->codecpar->codec_type]++;
@@ -2161,7 +2161,7 @@ static int set_dispositions(OutputFile *of)
 
     if (have_manual) {
         // process manually set dispositions - they override the above copy
-        for (int i = 0; i< of->ctx->nb_streams; i++) {
+        for (int i = 0; i < ctx->nb_streams; i++) {
             OutputStream *ost = output_streams[of->ost_index + i];
             int ret;
 
@@ -2187,7 +2187,7 @@ static int set_dispositions(OutputFile *of)
         // For each media type with more than one stream, find a suitable stream to
         // mark as default, unless one is already marked default.
         // "Suitable" means the first of that type, skipping attached pictures.
-        for (int i = 0; i< of->ctx->nb_streams; i++) {
+        for (int i = 0; i < ctx->nb_streams; i++) {
             OutputStream *ost = output_streams[of->ost_index + i];
             enum AVMediaType type = ost->st->codecpar->codec_type;
 
@@ -2725,7 +2725,7 @@ loop_end:
         }
     }
     if (o->chapters_input_file >= 0)
-        copy_chapters(input_files[o->chapters_input_file], of,
+        copy_chapters(input_files[o->chapters_input_file], of, oc,
                       !o->metadata_chapters_manual);
 
     /* copy global metadata by default */
@@ -2878,7 +2878,7 @@ loop_end:
         }
     }
 
-    err = set_dispositions(of);
+    err = set_dispositions(of, oc);
     if (err < 0) {
         av_log(NULL, AV_LOG_FATAL, "Error setting output stream dispositions\n");
         exit_program(1);
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 02/28] ffmpeg: store the output file index in OutputFile
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 03/28] ffmpeg: move some muxing-related code into a separate file Anton Khirnov
                   ` (25 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Use it to simplify check_init_output_file(). Will allow further
simplifications in the following commits.
---
 fftools/ffmpeg.c     | 10 +++++-----
 fftools/ffmpeg.h     |  2 ++
 fftools/ffmpeg_opt.c |  1 +
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 5d134b025f..1961653dcc 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2941,7 +2941,7 @@ static int compare_int64(const void *a, const void *b)
 }
 
 /* open the muxer when all the streams are initialized */
-static int check_init_output_file(OutputFile *of, int file_index)
+static int check_init_output_file(OutputFile *of)
 {
     int ret, i;
 
@@ -2956,13 +2956,13 @@ static int check_init_output_file(OutputFile *of, int file_index)
         av_log(NULL, AV_LOG_ERROR,
                "Could not write header for output file #%d "
                "(incorrect codec parameters ?): %s\n",
-               file_index, av_err2str(ret));
+               of->index, av_err2str(ret));
         return ret;
     }
     //assert_avoptions(of->opts);
     of->header_written = 1;
 
-    av_dump_format(of->ctx, file_index, of->ctx->url, 1);
+    av_dump_format(of->ctx, of->index, of->ctx->url, 1);
     nb_output_dumped++;
 
     if (sdp_filename || want_sdp) {
@@ -3565,7 +3565,7 @@ static int init_output_stream(OutputStream *ost, AVFrame *frame,
 
     ost->initialized = 1;
 
-    ret = check_init_output_file(output_files[ost->file_index], ost->file_index);
+    ret = check_init_output_file(output_files[ost->file_index]);
     if (ret < 0)
         return ret;
 
@@ -3668,7 +3668,7 @@ static int transcode_init(void)
     for (i = 0; i < nb_output_files; i++) {
         oc = output_files[i]->ctx;
         if (oc->oformat->flags & AVFMT_NOSTREAMS && oc->nb_streams == 0) {
-            ret = check_init_output_file(output_files[i], i);
+            ret = check_init_output_file(output_files[i]);
             if (ret < 0)
                 goto dump_format;
         }
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 9b200b806a..5fd5d2606b 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -574,6 +574,8 @@ typedef struct OutputStream {
 } OutputStream;
 
 typedef struct OutputFile {
+    int index;
+
     AVFormatContext *ctx;
     AVDictionary *opts;
     int ost_index;       /* index of the first stream in output_streams */
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index f638edace9..e8db515e0c 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2283,6 +2283,7 @@ static int open_output_file(OptionsContext *o, const char *filename)
 
     of = ALLOC_ARRAY_ELEM(output_files, nb_output_files);
 
+    of->index          = nb_output_files - 1;
     of->ost_index      = nb_output_streams;
     of->recording_time = o->recording_time;
     of->start_time     = o->start_time;
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 03/28] ffmpeg: move some muxing-related code into a separate file
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 02/28] ffmpeg: store the output file index in OutputFile Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 04/28] ffmpeg: move writing the trailer to ffmpeg_mux.c Anton Khirnov
                   ` (24 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

This is a first step towards making muxers more independent from the
rest of the code.
---
 fftools/Makefile     |   6 +-
 fftools/ffmpeg.c     | 273 ++--------------------------------------
 fftools/ffmpeg.h     |  10 ++
 fftools/ffmpeg_mux.c | 293 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 315 insertions(+), 267 deletions(-)
 create mode 100644 fftools/ffmpeg_mux.c

diff --git a/fftools/Makefile b/fftools/Makefile
index da420786eb..27a22250b3 100644
--- a/fftools/Makefile
+++ b/fftools/Makefile
@@ -9,7 +9,11 @@ AVBASENAMES  = ffmpeg ffplay ffprobe
 ALLAVPROGS   = $(AVBASENAMES:%=%$(PROGSSUF)$(EXESUF))
 ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
 
-OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o fftools/ffmpeg_hw.o
+OBJS-ffmpeg +=                  \
+    fftools/ffmpeg_filter.o     \
+    fftools/ffmpeg_hw.o         \
+    fftools/ffmpeg_mux.o        \
+    fftools/ffmpeg_opt.o        \
 
 define DOFFTOOL
 OBJS-$(1) += fftools/cmdutils.o fftools/$(1).o $(OBJS-$(1)-yes)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 1961653dcc..e0aa533dc9 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -136,9 +136,9 @@ static int nb_frames_dup = 0;
 static unsigned dup_warning = 1000;
 static int nb_frames_drop = 0;
 static int64_t decode_error_stat[2];
-static unsigned nb_output_dumped = 0;
+unsigned nb_output_dumped = 0;
 
-static int want_sdp = 1;
+int want_sdp = 1;
 
 static BenchmarkTimeStamps current_time;
 AVIOContext *progress_avio = NULL;
@@ -344,7 +344,7 @@ static volatile int received_sigterm = 0;
 static volatile int received_nb_signals = 0;
 static atomic_int transcode_init_done = ATOMIC_VAR_INIT(0);
 static volatile int ffmpeg_exited = 0;
-static int main_return_code = 0;
+int main_return_code = 0;
 static int64_t copy_ts_first_pts = AV_NOPTS_VALUE;
 
 static void
@@ -715,159 +715,6 @@ static void update_benchmark(const char *fmt, ...)
     }
 }
 
-static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
-{
-    int i;
-    for (i = 0; i < nb_output_streams; i++) {
-        OutputStream *ost2 = output_streams[i];
-        ost2->finished |= ost == ost2 ? this_stream : others;
-    }
-}
-
-static void write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost, int unqueue)
-{
-    AVFormatContext *s = of->ctx;
-    AVStream *st = ost->st;
-    int ret;
-
-    /*
-     * Audio encoders may split the packets --  #frames in != #packets out.
-     * But there is no reordering, so we can limit the number of output packets
-     * by simply dropping them here.
-     * Counting encoded video frames needs to be done separately because of
-     * reordering, see do_video_out().
-     * Do not count the packet when unqueued because it has been counted when queued.
-     */
-    if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed) && !unqueue) {
-        if (ost->frame_number >= ost->max_frames) {
-            av_packet_unref(pkt);
-            return;
-        }
-        ost->frame_number++;
-    }
-
-    if (!of->header_written) {
-        AVPacket *tmp_pkt;
-        /* the muxer is not initialized yet, buffer the packet */
-        if (!av_fifo_space(ost->muxing_queue)) {
-            size_t cur_size = av_fifo_size(ost->muxing_queue);
-            unsigned int are_we_over_size =
-                (ost->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
-            size_t limit    = are_we_over_size ? ost->max_muxing_queue_size : INT_MAX;
-            size_t new_size = FFMIN(2 * cur_size, limit);
-
-            if (new_size <= cur_size) {
-                av_log(NULL, AV_LOG_ERROR,
-                       "Too many packets buffered for output stream %d:%d.\n",
-                       ost->file_index, ost->st->index);
-                exit_program(1);
-            }
-            ret = av_fifo_realloc2(ost->muxing_queue, new_size);
-            if (ret < 0)
-                exit_program(1);
-        }
-        ret = av_packet_make_refcounted(pkt);
-        if (ret < 0)
-            exit_program(1);
-        tmp_pkt = av_packet_alloc();
-        if (!tmp_pkt)
-            exit_program(1);
-        av_packet_move_ref(tmp_pkt, pkt);
-        ost->muxing_queue_data_size += tmp_pkt->size;
-        av_fifo_generic_write(ost->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
-        return;
-    }
-
-    if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && video_sync_method == VSYNC_DROP) ||
-        (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && audio_sync_method < 0))
-        pkt->pts = pkt->dts = AV_NOPTS_VALUE;
-
-    if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
-        int i;
-        uint8_t *sd = av_packet_get_side_data(pkt, AV_PKT_DATA_QUALITY_STATS,
-                                              NULL);
-        ost->quality = sd ? AV_RL32(sd) : -1;
-        ost->pict_type = sd ? sd[4] : AV_PICTURE_TYPE_NONE;
-
-        for (i = 0; i<FF_ARRAY_ELEMS(ost->error); i++) {
-            if (sd && i < sd[5])
-                ost->error[i] = AV_RL64(sd + 8 + 8*i);
-            else
-                ost->error[i] = -1;
-        }
-
-        if (ost->frame_rate.num && ost->is_cfr) {
-            if (pkt->duration > 0)
-                av_log(NULL, AV_LOG_WARNING, "Overriding packet duration by frame rate, this should not happen\n");
-            pkt->duration = av_rescale_q(1, av_inv_q(ost->frame_rate),
-                                         ost->mux_timebase);
-        }
-    }
-
-    av_packet_rescale_ts(pkt, ost->mux_timebase, ost->st->time_base);
-
-    if (!(s->oformat->flags & AVFMT_NOTIMESTAMPS)) {
-        if (pkt->dts != AV_NOPTS_VALUE &&
-            pkt->pts != AV_NOPTS_VALUE &&
-            pkt->dts > pkt->pts) {
-            av_log(s, AV_LOG_WARNING, "Invalid DTS: %"PRId64" PTS: %"PRId64" in output stream %d:%d, replacing by guess\n",
-                   pkt->dts, pkt->pts,
-                   ost->file_index, ost->st->index);
-            pkt->pts =
-            pkt->dts = pkt->pts + pkt->dts + ost->last_mux_dts + 1
-                     - FFMIN3(pkt->pts, pkt->dts, ost->last_mux_dts + 1)
-                     - FFMAX3(pkt->pts, pkt->dts, ost->last_mux_dts + 1);
-        }
-        if ((st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO || st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO || st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) &&
-            pkt->dts != AV_NOPTS_VALUE &&
-            !(st->codecpar->codec_id == AV_CODEC_ID_VP9 && ost->stream_copy) &&
-            ost->last_mux_dts != AV_NOPTS_VALUE) {
-            int64_t max = ost->last_mux_dts + !(s->oformat->flags & AVFMT_TS_NONSTRICT);
-            if (pkt->dts < max) {
-                int loglevel = max - pkt->dts > 2 || st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO ? AV_LOG_WARNING : AV_LOG_DEBUG;
-                if (exit_on_error)
-                    loglevel = AV_LOG_ERROR;
-                av_log(s, loglevel, "Non-monotonous DTS in output stream "
-                       "%d:%d; previous: %"PRId64", current: %"PRId64"; ",
-                       ost->file_index, ost->st->index, ost->last_mux_dts, pkt->dts);
-                if (exit_on_error) {
-                    av_log(NULL, AV_LOG_FATAL, "aborting.\n");
-                    exit_program(1);
-                }
-                av_log(s, loglevel, "changing to %"PRId64". This may result "
-                       "in incorrect timestamps in the output file.\n",
-                       max);
-                if (pkt->pts >= pkt->dts)
-                    pkt->pts = FFMAX(pkt->pts, max);
-                pkt->dts = max;
-            }
-        }
-    }
-    ost->last_mux_dts = pkt->dts;
-
-    ost->data_size += pkt->size;
-    ost->packets_written++;
-
-    pkt->stream_index = ost->index;
-
-    if (debug_ts) {
-        av_log(NULL, AV_LOG_INFO, "muxer <- type:%s "
-                "pkt_pts:%s pkt_pts_time:%s pkt_dts:%s pkt_dts_time:%s size:%d\n",
-                av_get_media_type_string(ost->enc_ctx->codec_type),
-                av_ts2str(pkt->pts), av_ts2timestr(pkt->pts, &ost->st->time_base),
-                av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &ost->st->time_base),
-                pkt->size
-              );
-    }
-
-    ret = av_interleaved_write_frame(s, pkt);
-    if (ret < 0) {
-        print_error("av_interleaved_write_frame()", ret);
-        main_return_code = 1;
-        close_all_output_streams(ost, MUXER_FINISHED | ENCODER_FINISHED, ENCODER_FINISHED);
-    }
-}
-
 static void close_output_stream(OutputStream *ost)
 {
     OutputFile *of = output_files[ost->file_index];
@@ -902,11 +749,11 @@ static void output_packet(OutputFile *of, AVPacket *pkt,
         if (ret < 0)
             goto finish;
         while ((ret = av_bsf_receive_packet(ost->bsf_ctx, pkt)) >= 0)
-            write_packet(of, pkt, ost, 0);
+            of_write_packet(of, pkt, ost, 0);
         if (ret == AVERROR(EAGAIN))
             ret = 0;
     } else if (!eof)
-        write_packet(of, pkt, ost, 0);
+        of_write_packet(of, pkt, ost, 0);
 
 finish:
     if (ret < 0 && ret != AVERROR_EOF) {
@@ -2761,59 +2608,6 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
     return !eof_reached;
 }
 
-static int print_sdp(void)
-{
-    char sdp[16384];
-    int i;
-    int j, ret;
-    AVIOContext *sdp_pb;
-    AVFormatContext **avc;
-
-    for (i = 0; i < nb_output_files; i++) {
-        if (!output_files[i]->header_written)
-            return 0;
-    }
-
-    avc = av_malloc_array(nb_output_files, sizeof(*avc));
-    if (!avc)
-        exit_program(1);
-    for (i = 0, j = 0; i < nb_output_files; i++) {
-        if (!strcmp(output_files[i]->ctx->oformat->name, "rtp")) {
-            avc[j] = output_files[i]->ctx;
-            j++;
-        }
-    }
-
-    if (!j) {
-        av_log(NULL, AV_LOG_ERROR, "No output streams in the SDP.\n");
-        ret = AVERROR(EINVAL);
-        goto fail;
-    }
-
-    ret = av_sdp_create(avc, j, sdp, sizeof(sdp));
-    if (ret < 0)
-        goto fail;
-
-    if (!sdp_filename) {
-        printf("SDP:\n%s\n", sdp);
-        fflush(stdout);
-    } else {
-        ret = avio_open2(&sdp_pb, sdp_filename, AVIO_FLAG_WRITE, &int_cb, NULL);
-        if (ret < 0) {
-            av_log(NULL, AV_LOG_ERROR, "Failed to open sdp file '%s'\n", sdp_filename);
-            goto fail;
-        }
-
-        avio_print(sdp_pb, sdp);
-        avio_closep(&sdp_pb);
-        av_freep(&sdp_filename);
-    }
-
-fail:
-    av_freep(&avc);
-    return ret;
-}
-
 static enum AVPixelFormat get_format(AVCodecContext *s, const enum AVPixelFormat *pix_fmts)
 {
     InputStream *ist = s->opaque;
@@ -2940,59 +2734,6 @@ static int compare_int64(const void *a, const void *b)
     return FFDIFFSIGN(*(const int64_t *)a, *(const int64_t *)b);
 }
 
-/* open the muxer when all the streams are initialized */
-static int check_init_output_file(OutputFile *of)
-{
-    int ret, i;
-
-    for (i = 0; i < of->ctx->nb_streams; i++) {
-        OutputStream *ost = output_streams[of->ost_index + i];
-        if (!ost->initialized)
-            return 0;
-    }
-
-    ret = avformat_write_header(of->ctx, &of->opts);
-    if (ret < 0) {
-        av_log(NULL, AV_LOG_ERROR,
-               "Could not write header for output file #%d "
-               "(incorrect codec parameters ?): %s\n",
-               of->index, av_err2str(ret));
-        return ret;
-    }
-    //assert_avoptions(of->opts);
-    of->header_written = 1;
-
-    av_dump_format(of->ctx, of->index, of->ctx->url, 1);
-    nb_output_dumped++;
-
-    if (sdp_filename || want_sdp) {
-        ret = print_sdp();
-        if (ret < 0) {
-            av_log(NULL, AV_LOG_ERROR, "Error writing the SDP.\n");
-            return ret;
-        }
-    }
-
-    /* flush the muxing queues */
-    for (i = 0; i < of->ctx->nb_streams; i++) {
-        OutputStream *ost = output_streams[of->ost_index + i];
-
-        /* try to improve muxing time_base (only possible if nothing has been written yet) */
-        if (!av_fifo_size(ost->muxing_queue))
-            ost->mux_timebase = ost->st->time_base;
-
-        while (av_fifo_size(ost->muxing_queue)) {
-            AVPacket *pkt;
-            av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
-            ost->muxing_queue_data_size -= pkt->size;
-            write_packet(of, pkt, ost, 1);
-            av_packet_free(&pkt);
-        }
-    }
-
-    return 0;
-}
-
 static int init_output_bsfs(OutputStream *ost)
 {
     AVBSFContext *ctx = ost->bsf_ctx;
@@ -3565,7 +3306,7 @@ static int init_output_stream(OutputStream *ost, AVFrame *frame,
 
     ost->initialized = 1;
 
-    ret = check_init_output_file(output_files[ost->file_index]);
+    ret = of_check_init(output_files[ost->file_index]);
     if (ret < 0)
         return ret;
 
@@ -3668,7 +3409,7 @@ static int transcode_init(void)
     for (i = 0; i < nb_output_files; i++) {
         oc = output_files[i]->ctx;
         if (oc->oformat->flags & AVFMT_NOSTREAMS && oc->nb_streams == 0) {
-            ret = check_init_output_file(output_files[i]);
+            ret = of_check_init(output_files[i]);
             if (ret < 0)
                 goto dump_format;
         }
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 5fd5d2606b..fed34b06f8 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -644,6 +644,10 @@ extern char *qsv_device;
 #endif
 extern HWDevice *filter_hw_device;
 
+extern int want_sdp;
+extern unsigned nb_output_dumped;
+extern int main_return_code;
+
 
 void term_init(void);
 void term_exit(void);
@@ -680,4 +684,10 @@ int hw_device_setup_for_filter(FilterGraph *fg);
 
 int hwaccel_decode_init(AVCodecContext *avctx);
 
+/* open the muxer when all the streams are initialized */
+int of_check_init(OutputFile *of);
+
+void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
+                     int unqueue);
+
 #endif /* FFTOOLS_FFMPEG_H */
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
new file mode 100644
index 0000000000..3c2980ba90
--- /dev/null
+++ b/fftools/ffmpeg_mux.c
@@ -0,0 +1,293 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "ffmpeg.h"
+
+#include "libavutil/fifo.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/log.h"
+#include "libavutil/mem.h"
+#include "libavutil/timestamp.h"
+
+#include "libavcodec/packet.h"
+
+#include "libavformat/avformat.h"
+#include "libavformat/avio.h"
+
+static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
+{
+    int i;
+    for (i = 0; i < nb_output_streams; i++) {
+        OutputStream *ost2 = output_streams[i];
+        ost2->finished |= ost == ost2 ? this_stream : others;
+    }
+}
+
+void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
+                     int unqueue)
+{
+    AVFormatContext *s = of->ctx;
+    AVStream *st = ost->st;
+    int ret;
+
+    /*
+     * Audio encoders may split the packets --  #frames in != #packets out.
+     * But there is no reordering, so we can limit the number of output packets
+     * by simply dropping them here.
+     * Counting encoded video frames needs to be done separately because of
+     * reordering, see do_video_out().
+     * Do not count the packet when unqueued because it has been counted when queued.
+     */
+    if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed) && !unqueue) {
+        if (ost->frame_number >= ost->max_frames) {
+            av_packet_unref(pkt);
+            return;
+        }
+        ost->frame_number++;
+    }
+
+    if (!of->header_written) {
+        AVPacket *tmp_pkt;
+        /* the muxer is not initialized yet, buffer the packet */
+        if (!av_fifo_space(ost->muxing_queue)) {
+            size_t cur_size = av_fifo_size(ost->muxing_queue);
+            unsigned int are_we_over_size =
+                (ost->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
+            size_t limit    = are_we_over_size ? ost->max_muxing_queue_size : INT_MAX;
+            size_t new_size = FFMIN(2 * cur_size, limit);
+
+            if (new_size <= cur_size) {
+                av_log(NULL, AV_LOG_ERROR,
+                       "Too many packets buffered for output stream %d:%d.\n",
+                       ost->file_index, ost->st->index);
+                exit_program(1);
+            }
+            ret = av_fifo_realloc2(ost->muxing_queue, new_size);
+            if (ret < 0)
+                exit_program(1);
+        }
+        ret = av_packet_make_refcounted(pkt);
+        if (ret < 0)
+            exit_program(1);
+        tmp_pkt = av_packet_alloc();
+        if (!tmp_pkt)
+            exit_program(1);
+        av_packet_move_ref(tmp_pkt, pkt);
+        ost->muxing_queue_data_size += tmp_pkt->size;
+        av_fifo_generic_write(ost->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
+        return;
+    }
+
+    if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && video_sync_method == VSYNC_DROP) ||
+        (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && audio_sync_method < 0))
+        pkt->pts = pkt->dts = AV_NOPTS_VALUE;
+
+    if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+        int i;
+        uint8_t *sd = av_packet_get_side_data(pkt, AV_PKT_DATA_QUALITY_STATS,
+                                              NULL);
+        ost->quality = sd ? AV_RL32(sd) : -1;
+        ost->pict_type = sd ? sd[4] : AV_PICTURE_TYPE_NONE;
+
+        for (i = 0; i<FF_ARRAY_ELEMS(ost->error); i++) {
+            if (sd && i < sd[5])
+                ost->error[i] = AV_RL64(sd + 8 + 8*i);
+            else
+                ost->error[i] = -1;
+        }
+
+        if (ost->frame_rate.num && ost->is_cfr) {
+            if (pkt->duration > 0)
+                av_log(NULL, AV_LOG_WARNING, "Overriding packet duration by frame rate, this should not happen\n");
+            pkt->duration = av_rescale_q(1, av_inv_q(ost->frame_rate),
+                                         ost->mux_timebase);
+        }
+    }
+
+    av_packet_rescale_ts(pkt, ost->mux_timebase, ost->st->time_base);
+
+    if (!(s->oformat->flags & AVFMT_NOTIMESTAMPS)) {
+        if (pkt->dts != AV_NOPTS_VALUE &&
+            pkt->pts != AV_NOPTS_VALUE &&
+            pkt->dts > pkt->pts) {
+            av_log(s, AV_LOG_WARNING, "Invalid DTS: %"PRId64" PTS: %"PRId64" in output stream %d:%d, replacing by guess\n",
+                   pkt->dts, pkt->pts,
+                   ost->file_index, ost->st->index);
+            pkt->pts =
+            pkt->dts = pkt->pts + pkt->dts + ost->last_mux_dts + 1
+                     - FFMIN3(pkt->pts, pkt->dts, ost->last_mux_dts + 1)
+                     - FFMAX3(pkt->pts, pkt->dts, ost->last_mux_dts + 1);
+        }
+        if ((st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO || st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO || st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) &&
+            pkt->dts != AV_NOPTS_VALUE &&
+            !(st->codecpar->codec_id == AV_CODEC_ID_VP9 && ost->stream_copy) &&
+            ost->last_mux_dts != AV_NOPTS_VALUE) {
+            int64_t max = ost->last_mux_dts + !(s->oformat->flags & AVFMT_TS_NONSTRICT);
+            if (pkt->dts < max) {
+                int loglevel = max - pkt->dts > 2 || st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO ? AV_LOG_WARNING : AV_LOG_DEBUG;
+                if (exit_on_error)
+                    loglevel = AV_LOG_ERROR;
+                av_log(s, loglevel, "Non-monotonous DTS in output stream "
+                       "%d:%d; previous: %"PRId64", current: %"PRId64"; ",
+                       ost->file_index, ost->st->index, ost->last_mux_dts, pkt->dts);
+                if (exit_on_error) {
+                    av_log(NULL, AV_LOG_FATAL, "aborting.\n");
+                    exit_program(1);
+                }
+                av_log(s, loglevel, "changing to %"PRId64". This may result "
+                       "in incorrect timestamps in the output file.\n",
+                       max);
+                if (pkt->pts >= pkt->dts)
+                    pkt->pts = FFMAX(pkt->pts, max);
+                pkt->dts = max;
+            }
+        }
+    }
+    ost->last_mux_dts = pkt->dts;
+
+    ost->data_size += pkt->size;
+    ost->packets_written++;
+
+    pkt->stream_index = ost->index;
+
+    if (debug_ts) {
+        av_log(NULL, AV_LOG_INFO, "muxer <- type:%s "
+                "pkt_pts:%s pkt_pts_time:%s pkt_dts:%s pkt_dts_time:%s size:%d\n",
+                av_get_media_type_string(ost->enc_ctx->codec_type),
+                av_ts2str(pkt->pts), av_ts2timestr(pkt->pts, &ost->st->time_base),
+                av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &ost->st->time_base),
+                pkt->size
+              );
+    }
+
+    ret = av_interleaved_write_frame(s, pkt);
+    if (ret < 0) {
+        print_error("av_interleaved_write_frame()", ret);
+        main_return_code = 1;
+        close_all_output_streams(ost, MUXER_FINISHED | ENCODER_FINISHED, ENCODER_FINISHED);
+    }
+}
+
+static int print_sdp(void)
+{
+    char sdp[16384];
+    int i;
+    int j, ret;
+    AVIOContext *sdp_pb;
+    AVFormatContext **avc;
+
+    for (i = 0; i < nb_output_files; i++) {
+        if (!output_files[i]->header_written)
+            return 0;
+    }
+
+    avc = av_malloc_array(nb_output_files, sizeof(*avc));
+    if (!avc)
+        exit_program(1);
+    for (i = 0, j = 0; i < nb_output_files; i++) {
+        if (!strcmp(output_files[i]->ctx->oformat->name, "rtp")) {
+            avc[j] = output_files[i]->ctx;
+            j++;
+        }
+    }
+
+    if (!j) {
+        av_log(NULL, AV_LOG_ERROR, "No output streams in the SDP.\n");
+        ret = AVERROR(EINVAL);
+        goto fail;
+    }
+
+    ret = av_sdp_create(avc, j, sdp, sizeof(sdp));
+    if (ret < 0)
+        goto fail;
+
+    if (!sdp_filename) {
+        printf("SDP:\n%s\n", sdp);
+        fflush(stdout);
+    } else {
+        ret = avio_open2(&sdp_pb, sdp_filename, AVIO_FLAG_WRITE, &int_cb, NULL);
+        if (ret < 0) {
+            av_log(NULL, AV_LOG_ERROR, "Failed to open sdp file '%s'\n", sdp_filename);
+            goto fail;
+        }
+
+        avio_print(sdp_pb, sdp);
+        avio_closep(&sdp_pb);
+        av_freep(&sdp_filename);
+    }
+
+fail:
+    av_freep(&avc);
+    return ret;
+}
+
+/* open the muxer when all the streams are initialized */
+int of_check_init(OutputFile *of)
+{
+    int ret, i;
+
+    for (i = 0; i < of->ctx->nb_streams; i++) {
+        OutputStream *ost = output_streams[of->ost_index + i];
+        if (!ost->initialized)
+            return 0;
+    }
+
+    ret = avformat_write_header(of->ctx, &of->opts);
+    if (ret < 0) {
+        av_log(NULL, AV_LOG_ERROR,
+               "Could not write header for output file #%d "
+               "(incorrect codec parameters ?): %s\n",
+               of->index, av_err2str(ret));
+        return ret;
+    }
+    //assert_avoptions(of->opts);
+    of->header_written = 1;
+
+    av_dump_format(of->ctx, of->index, of->ctx->url, 1);
+    nb_output_dumped++;
+
+    if (sdp_filename || want_sdp) {
+        ret = print_sdp();
+        if (ret < 0) {
+            av_log(NULL, AV_LOG_ERROR, "Error writing the SDP.\n");
+            return ret;
+        }
+    }
+
+    /* flush the muxing queues */
+    for (i = 0; i < of->ctx->nb_streams; i++) {
+        OutputStream *ost = output_streams[of->ost_index + i];
+
+        /* try to improve muxing time_base (only possible if nothing has been written yet) */
+        if (!av_fifo_size(ost->muxing_queue))
+            ost->mux_timebase = ost->st->time_base;
+
+        while (av_fifo_size(ost->muxing_queue)) {
+            AVPacket *pkt;
+            av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
+            ost->muxing_queue_data_size -= pkt->size;
+            of_write_packet(of, pkt, ost, 1);
+            av_packet_free(&pkt);
+        }
+    }
+
+    return 0;
+}
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 04/28] ffmpeg: move writing the trailer to ffmpeg_mux.c
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 02/28] ffmpeg: store the output file index in OutputFile Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 03/28] ffmpeg: move some muxing-related code into a separate file Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 05/28] ffmpeg: move freeing the output file " Anton Khirnov
                   ` (23 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

---
 fftools/ffmpeg.c     | 16 +++-------------
 fftools/ffmpeg.h     |  1 +
 fftools/ffmpeg_mux.c | 21 +++++++++++++++++++++
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index e0aa533dc9..87e8b0be59 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -4440,19 +4440,9 @@ static int transcode(void)
 
     /* write the trailer if needed */
     for (i = 0; i < nb_output_files; i++) {
-        os = output_files[i]->ctx;
-        if (!output_files[i]->header_written) {
-            av_log(NULL, AV_LOG_ERROR,
-                   "Nothing was written into output file %d (%s), because "
-                   "at least one of its streams received no packets.\n",
-                   i, os->url);
-            continue;
-        }
-        if ((ret = av_write_trailer(os)) < 0) {
-            av_log(NULL, AV_LOG_ERROR, "Error writing trailer of %s: %s\n", os->url, av_err2str(ret));
-            if (exit_on_error)
-                exit_program(1);
-        }
+        ret = of_write_trailer(output_files[i]);
+        if (ret < 0 && exit_on_error)
+            exit_program(1);
     }
 
     /* dump report by using the first video and audio streams */
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index fed34b06f8..91c313d8ef 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -686,6 +686,7 @@ int hwaccel_decode_init(AVCodecContext *avctx);
 
 /* open the muxer when all the streams are initialized */
 int of_check_init(OutputFile *of);
+int of_write_trailer(OutputFile *of);
 
 void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
                      int unqueue);
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 3c2980ba90..c65ed3631a 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -291,3 +291,24 @@ int of_check_init(OutputFile *of)
 
     return 0;
 }
+
+int of_write_trailer(OutputFile *of)
+{
+    int ret;
+
+    if (!of->header_written) {
+        av_log(NULL, AV_LOG_ERROR,
+               "Nothing was written into output file %d (%s), because "
+               "at least one of its streams received no packets.\n",
+               of->index, of->ctx->url);
+        return AVERROR(EINVAL);
+    }
+
+    ret = av_write_trailer(of->ctx);
+    if (ret < 0) {
+        av_log(NULL, AV_LOG_ERROR, "Error writing trailer of %s: %s\n", of->ctx->url, av_err2str(ret));
+        return ret;
+    }
+
+    return 0;
+}
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 05/28] ffmpeg: move freeing the output file to ffmpeg_mux.c
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (2 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 04/28] ffmpeg: move writing the trailer to ffmpeg_mux.c Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 06/28] ffmpeg: store output format separately from the muxer context Anton Khirnov
                   ` (22 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

---
 fftools/ffmpeg.c     | 14 ++------------
 fftools/ffmpeg.h     |  1 +
 fftools/ffmpeg_mux.c | 17 +++++++++++++++++
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 87e8b0be59..320ab8e0ca 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -567,19 +567,9 @@ static void ffmpeg_cleanup(int ret)
     av_freep(&subtitle_out);
 
     /* close files */
-    for (i = 0; i < nb_output_files; i++) {
-        OutputFile *of = output_files[i];
-        AVFormatContext *s;
-        if (!of)
-            continue;
-        s = of->ctx;
-        if (s && s->oformat && !(s->oformat->flags & AVFMT_NOFILE))
-            avio_closep(&s->pb);
-        avformat_free_context(s);
-        av_dict_free(&of->opts);
+    for (i = 0; i < nb_output_files; i++)
+        of_close(&output_files[i]);
 
-        av_freep(&output_files[i]);
-    }
     for (i = 0; i < nb_output_streams; i++) {
         OutputStream *ost = output_streams[i];
 
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 91c313d8ef..288e0f2981 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -687,6 +687,7 @@ int hwaccel_decode_init(AVCodecContext *avctx);
 /* open the muxer when all the streams are initialized */
 int of_check_init(OutputFile *of);
 int of_write_trailer(OutputFile *of);
+void of_close(OutputFile **pof);
 
 void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
                      int unqueue);
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index c65ed3631a..12ee3c2357 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -312,3 +312,20 @@ int of_write_trailer(OutputFile *of)
 
     return 0;
 }
+
+void of_close(OutputFile **pof)
+{
+    OutputFile *of = *pof;
+    AVFormatContext *s;
+
+    if (!of)
+        return;
+
+    s = of->ctx;
+    if (s && s->oformat && !(s->oformat->flags & AVFMT_NOFILE))
+        avio_closep(&s->pb);
+    avformat_free_context(s);
+    av_dict_free(&of->opts);
+
+    av_freep(pof);
+}
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 06/28] ffmpeg: store output format separately from the muxer context
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (3 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 05/28] ffmpeg: move freeing the output file " Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 07/28] ffmpeg_mux: add private " Anton Khirnov
                   ` (21 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Allows accessing it without going through the muxer context. This will
be useful in the following commits, where the muxer context will be
hidden.
---
 fftools/ffmpeg.c     | 18 ++++++++++--------
 fftools/ffmpeg.h     |  2 ++
 fftools/ffmpeg_opt.c |  1 +
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 320ab8e0ca..9360c8a475 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2783,9 +2783,9 @@ static int init_output_stream_streamcopy(OutputStream *ost)
 
     if (!codec_tag) {
         unsigned int codec_tag_tmp;
-        if (!of->ctx->oformat->codec_tag ||
-            av_codec_get_id (of->ctx->oformat->codec_tag, par_src->codec_tag) == par_src->codec_id ||
-            !av_codec_get_tag2(of->ctx->oformat->codec_tag, par_src->codec_id, &codec_tag_tmp))
+        if (!of->format->codec_tag ||
+            av_codec_get_id (of->format->codec_tag, par_src->codec_tag) == par_src->codec_id ||
+            !av_codec_get_tag2(of->format->codec_tag, par_src->codec_id, &codec_tag_tmp))
             codec_tag = par_src->codec_tag;
     }
 
@@ -2803,7 +2803,7 @@ static int init_output_stream_streamcopy(OutputStream *ost)
     else
         ost->st->avg_frame_rate = ist->st->avg_frame_rate;
 
-    ret = avformat_transfer_internal_stream_timing_info(of->ctx->oformat, ost->st, ist->st, copy_tb);
+    ret = avformat_transfer_internal_stream_timing_info(of->format, ost->st, ist->st, copy_tb);
     if (ret < 0)
         return ret;
 
@@ -3005,7 +3005,8 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
     InputStream *ist = get_input_stream(ost);
     AVCodecContext *enc_ctx = ost->enc_ctx;
     AVCodecContext *dec_ctx = NULL;
-    AVFormatContext *oc = output_files[ost->file_index]->ctx;
+    OutputFile      *of = output_files[ost->file_index];
+    AVFormatContext *oc = of->ctx;
     int ret;
 
     set_encoder_id(output_files[ost->file_index], ost);
@@ -3065,7 +3066,8 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
         if (!(enc_ctx->time_base.num && enc_ctx->time_base.den))
             enc_ctx->time_base = av_buffersink_get_time_base(ost->filter->filter);
         if (   av_q2d(enc_ctx->time_base) < 0.001 && video_sync_method != VSYNC_PASSTHROUGH
-           && (video_sync_method == VSYNC_CFR || video_sync_method == VSYNC_VSCFR || (video_sync_method == VSYNC_AUTO && !(oc->oformat->flags & AVFMT_VARIABLE_FPS)))){
+           && (video_sync_method == VSYNC_CFR || video_sync_method == VSYNC_VSCFR ||
+               (video_sync_method == VSYNC_AUTO && !(of->format->flags & AVFMT_VARIABLE_FPS)))){
             av_log(oc, AV_LOG_WARNING, "Frame rate very high for a muxer not efficiently supporting it.\n"
                                        "Please consider specifying a lower framerate, a different muxer or -vsync 2\n");
         }
@@ -3398,7 +3400,7 @@ static int transcode_init(void)
     /* write headers for files with no streams */
     for (i = 0; i < nb_output_files; i++) {
         oc = output_files[i]->ctx;
-        if (oc->oformat->flags & AVFMT_NOSTREAMS && oc->nb_streams == 0) {
+        if (output_files[i]->format->flags & AVFMT_NOSTREAMS && oc->nb_streams == 0) {
             ret = of_check_init(output_files[i]);
             if (ret < 0)
                 goto dump_format;
@@ -4605,7 +4607,7 @@ int main(int argc, char **argv)
     }
 
     for (i = 0; i < nb_output_files; i++) {
-        if (strcmp(output_files[i]->ctx->oformat->name, "rtp"))
+        if (strcmp(output_files[i]->format->name, "rtp"))
             want_sdp = 0;
     }
 
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 288e0f2981..c28acad4f1 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -576,6 +576,8 @@ typedef struct OutputStream {
 typedef struct OutputFile {
     int index;
 
+    const AVOutputFormat *format;
+
     AVFormatContext *ctx;
     AVDictionary *opts;
     int ost_index;       /* index of the first stream in output_streams */
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index e8db515e0c..433102bb49 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2301,6 +2301,7 @@ static int open_output_file(OptionsContext *o, const char *filename)
     }
 
     of->ctx = oc;
+    of->format = oc->oformat;
     if (o->recording_time != INT64_MAX)
         oc->duration = o->recording_time;
 
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 07/28] ffmpeg_mux: add private muxer context
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (4 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 06/28] ffmpeg: store output format separately from the muxer context Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 08/28] ffmpeg: add a helper function to access output file size Anton Khirnov
                   ` (20 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Move header_written into it, which is not (and should not be) used by
any code outside of ffmpeg_mux.

In the future this context will contain more muxer-private state that
should not be visible to other code.
---
 fftools/ffmpeg.h     |  6 ++++--
 fftools/ffmpeg_mux.c | 26 ++++++++++++++++++++++----
 fftools/ffmpeg_opt.c |  6 ++++++
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index c28acad4f1..279a99cc48 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -573,9 +573,12 @@ typedef struct OutputStream {
     int64_t error[4];
 } OutputStream;
 
+typedef struct Muxer Muxer;
+
 typedef struct OutputFile {
     int index;
 
+    Muxer                *mux;
     const AVOutputFormat *format;
 
     AVFormatContext *ctx;
@@ -586,8 +589,6 @@ typedef struct OutputFile {
     uint64_t limit_filesize; /* filesize limit expressed in bytes */
 
     int shortest;
-
-    int header_written;
 } OutputFile;
 
 extern InputStream **input_streams;
@@ -686,6 +687,7 @@ int hw_device_setup_for_filter(FilterGraph *fg);
 
 int hwaccel_decode_init(AVCodecContext *avctx);
 
+int of_muxer_init(OutputFile *of);
 /* open the muxer when all the streams are initialized */
 int of_check_init(OutputFile *of);
 int of_write_trailer(OutputFile *of);
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 12ee3c2357..538f153484 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -32,6 +32,10 @@
 #include "libavformat/avformat.h"
 #include "libavformat/avio.h"
 
+struct Muxer {
+    int header_written;
+};
+
 static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
 {
     int i;
@@ -64,7 +68,7 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
         ost->frame_number++;
     }
 
-    if (!of->header_written) {
+    if (!of->mux->header_written) {
         AVPacket *tmp_pkt;
         /* the muxer is not initialized yet, buffer the packet */
         if (!av_fifo_space(ost->muxing_queue)) {
@@ -195,7 +199,7 @@ static int print_sdp(void)
     AVFormatContext **avc;
 
     for (i = 0; i < nb_output_files; i++) {
-        if (!output_files[i]->header_written)
+        if (!output_files[i]->mux->header_written)
             return 0;
     }
 
@@ -259,7 +263,7 @@ int of_check_init(OutputFile *of)
         return ret;
     }
     //assert_avoptions(of->opts);
-    of->header_written = 1;
+    of->mux->header_written = 1;
 
     av_dump_format(of->ctx, of->index, of->ctx->url, 1);
     nb_output_dumped++;
@@ -296,7 +300,7 @@ int of_write_trailer(OutputFile *of)
 {
     int ret;
 
-    if (!of->header_written) {
+    if (!of->mux->header_written) {
         av_log(NULL, AV_LOG_ERROR,
                "Nothing was written into output file %d (%s), because "
                "at least one of its streams received no packets.\n",
@@ -327,5 +331,19 @@ void of_close(OutputFile **pof)
     avformat_free_context(s);
     av_dict_free(&of->opts);
 
+    av_freep(&of->mux);
+
     av_freep(pof);
 }
+
+int of_muxer_init(OutputFile *of)
+{
+    Muxer *mux = av_mallocz(sizeof(*mux));
+
+    if (!mux)
+        return AVERROR(ENOMEM);
+
+    of->mux  = mux;
+
+    return 0;
+}
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 433102bb49..2ed09d4a7a 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2886,6 +2886,12 @@ loop_end:
         exit_program(1);
     }
 
+    err = of_muxer_init(of);
+    if (err < 0) {
+        av_log(NULL, AV_LOG_FATAL, "Error initializing internal muxing state\n");
+        exit_program(1);
+    }
+
     return 0;
 }
 
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 08/28] ffmpeg: add a helper function to access output file size
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (5 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 07/28] ffmpeg_mux: add private " Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 09/28] ffmpeg: fix the type of limit_filesize Anton Khirnov
                   ` (19 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Stop accessing muxer internals from outside of ffmpeg_mux.
---
 fftools/ffmpeg.c     | 10 +---------
 fftools/ffmpeg.h     |  1 +
 fftools/ffmpeg_mux.c | 14 ++++++++++++++
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 9360c8a475..1b5cf4d9af 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1514,8 +1514,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
 {
     AVBPrint buf, buf_script;
     OutputStream *ost;
-    AVFormatContext *oc;
-    int64_t total_size;
+    int64_t total_size = of_filesize(output_files[0]);
     AVCodecContext *enc;
     int frame_number, vid, i;
     double bitrate;
@@ -1544,13 +1543,6 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
 
     t = (cur_time-timer_start) / 1000000.0;
 
-
-    oc = output_files[0]->ctx;
-
-    total_size = avio_size(oc->pb);
-    if (total_size <= 0) // FIXME improve avio_size() so it works with non seekable output too
-        total_size = avio_tell(oc->pb);
-
     vid = 0;
     av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
     av_bprint_init(&buf_script, 0, AV_BPRINT_SIZE_AUTOMATIC);
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 279a99cc48..dfcaa875d9 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -695,5 +695,6 @@ void of_close(OutputFile **pof);
 
 void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
                      int unqueue);
+int64_t of_filesize(OutputFile *of);
 
 #endif /* FFTOOLS_FFMPEG_H */
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 538f153484..7c8414ac93 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -347,3 +347,17 @@ int of_muxer_init(OutputFile *of)
 
     return 0;
 }
+
+int64_t of_filesize(OutputFile *of)
+{
+    AVIOContext *pb = of->ctx->pb;
+    int64_t ret = -1;
+
+    if (pb) {
+        ret = avio_size(pb);
+        if (ret <= 0) // FIXME improve avio_size() so it works with non seekable output too
+            ret = avio_tell(pb);
+    }
+
+    return ret;
+}
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 09/28] ffmpeg: fix the type of limit_filesize
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (6 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 08/28] ffmpeg: add a helper function to access output file size Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 10/28] ffmpeg: refactor limiting output file size with -fs Anton Khirnov
                   ` (18 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

The option is parsed as INT64 (signed). It is also compared to the
output of avio_tell(), which is also int64_t.
---
 fftools/ffmpeg.h     | 4 ++--
 fftools/ffmpeg_opt.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index dfcaa875d9..80b90e372d 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -145,7 +145,7 @@ typedef struct OptionsContext {
 
     int64_t recording_time;
     int64_t stop_time;
-    uint64_t limit_filesize;
+    int64_t limit_filesize;
     float mux_preload;
     float mux_max_delay;
     int shortest;
@@ -586,7 +586,7 @@ typedef struct OutputFile {
     int ost_index;       /* index of the first stream in output_streams */
     int64_t recording_time;  ///< desired length of the resulting file in microseconds == AV_TIME_BASE units
     int64_t start_time;      ///< start time in microseconds == AV_TIME_BASE units
-    uint64_t limit_filesize; /* filesize limit expressed in bytes */
+    int64_t limit_filesize; /* filesize limit expressed in bytes */
 
     int shortest;
 } OutputFile;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 2ed09d4a7a..4d2b80683e 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -220,7 +220,7 @@ static void init_options(OptionsContext *o)
     o->start_time     = AV_NOPTS_VALUE;
     o->start_time_eof = AV_NOPTS_VALUE;
     o->recording_time = INT64_MAX;
-    o->limit_filesize = UINT64_MAX;
+    o->limit_filesize = INT64_MAX;
     o->chapters_input_file = INT_MAX;
     o->accurate_seek  = 1;
     o->thread_queue_size = -1;
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 10/28] ffmpeg: refactor limiting output file size with -fs
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (7 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 09/28] ffmpeg: fix the type of limit_filesize Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 11/28] ffmpeg: set want_sdp when initializing the muxer Anton Khirnov
                   ` (17 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Move the file size checking code to ffmpeg_mux. Use the recently
introduced of_filesize(), making this code consistent with the size
shown by print_report().
---
 fftools/ffmpeg.c     |  4 +---
 fftools/ffmpeg.h     |  4 ++--
 fftools/ffmpeg_mux.c | 11 ++++++++++-
 fftools/ffmpeg_opt.c |  3 +--
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 1b5cf4d9af..37c4a0d285 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3501,10 +3501,8 @@ static int need_output(void)
     for (i = 0; i < nb_output_streams; i++) {
         OutputStream *ost    = output_streams[i];
         OutputFile *of       = output_files[ost->file_index];
-        AVFormatContext *os  = output_files[ost->file_index]->ctx;
 
-        if (ost->finished ||
-            (os->pb && avio_tell(os->pb) >= of->limit_filesize))
+        if (ost->finished || of_finished(of))
             continue;
         if (ost->frame_number >= ost->max_frames) {
             int j;
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 80b90e372d..cae9800da3 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -586,7 +586,6 @@ typedef struct OutputFile {
     int ost_index;       /* index of the first stream in output_streams */
     int64_t recording_time;  ///< desired length of the resulting file in microseconds == AV_TIME_BASE units
     int64_t start_time;      ///< start time in microseconds == AV_TIME_BASE units
-    int64_t limit_filesize; /* filesize limit expressed in bytes */
 
     int shortest;
 } OutputFile;
@@ -687,7 +686,7 @@ int hw_device_setup_for_filter(FilterGraph *fg);
 
 int hwaccel_decode_init(AVCodecContext *avctx);
 
-int of_muxer_init(OutputFile *of);
+int of_muxer_init(OutputFile *of, int64_t limit_filesize);
 /* open the muxer when all the streams are initialized */
 int of_check_init(OutputFile *of);
 int of_write_trailer(OutputFile *of);
@@ -695,6 +694,7 @@ void of_close(OutputFile **pof);
 
 void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
                      int unqueue);
+int of_finished(OutputFile *of);
 int64_t of_filesize(OutputFile *of);
 
 #endif /* FFTOOLS_FFMPEG_H */
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 7c8414ac93..aff40776f6 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -33,6 +33,8 @@
 #include "libavformat/avio.h"
 
 struct Muxer {
+    /* filesize limit expressed in bytes */
+    int64_t limit_filesize;
     int header_written;
 };
 
@@ -336,7 +338,7 @@ void of_close(OutputFile **pof)
     av_freep(pof);
 }
 
-int of_muxer_init(OutputFile *of)
+int of_muxer_init(OutputFile *of, int64_t limit_filesize)
 {
     Muxer *mux = av_mallocz(sizeof(*mux));
 
@@ -345,9 +347,16 @@ int of_muxer_init(OutputFile *of)
 
     of->mux  = mux;
 
+    mux->limit_filesize = limit_filesize;
+
     return 0;
 }
 
+int of_finished(OutputFile *of)
+{
+    return of_filesize(of) >= of->mux->limit_filesize;
+}
+
 int64_t of_filesize(OutputFile *of)
 {
     AVIOContext *pb = of->ctx->pb;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 4d2b80683e..0b4f437912 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2287,7 +2287,6 @@ static int open_output_file(OptionsContext *o, const char *filename)
     of->ost_index      = nb_output_streams;
     of->recording_time = o->recording_time;
     of->start_time     = o->start_time;
-    of->limit_filesize = o->limit_filesize;
     of->shortest       = o->shortest;
     av_dict_copy(&of->opts, o->g->format_opts, 0);
 
@@ -2886,7 +2885,7 @@ loop_end:
         exit_program(1);
     }
 
-    err = of_muxer_init(of);
+    err = of_muxer_init(of, o->limit_filesize);
     if (err < 0) {
         av_log(NULL, AV_LOG_FATAL, "Error initializing internal muxing state\n");
         exit_program(1);
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 11/28] ffmpeg: set want_sdp when initializing the muxer
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (8 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 10/28] ffmpeg: refactor limiting output file size with -fs Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 12/28] ffmpeg: write the header for stream-less outputs " Anton Khirnov
                   ` (16 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Allows making the variable local to ffmpeg_mux.
---
 fftools/ffmpeg.c     | 9 +--------
 fftools/ffmpeg.h     | 1 -
 fftools/ffmpeg_mux.c | 5 +++++
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 37c4a0d285..d856b5b38f 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -138,8 +138,6 @@ static int nb_frames_drop = 0;
 static int64_t decode_error_stat[2];
 unsigned nb_output_dumped = 0;
 
-int want_sdp = 1;
-
 static BenchmarkTimeStamps current_time;
 AVIOContext *progress_avio = NULL;
 
@@ -4553,7 +4551,7 @@ static void log_callback_null(void *ptr, int level, const char *fmt, va_list vl)
 
 int main(int argc, char **argv)
 {
-    int i, ret;
+    int ret;
     BenchmarkTimeStamps ti;
 
     init_dynload();
@@ -4596,11 +4594,6 @@ int main(int argc, char **argv)
         exit_program(1);
     }
 
-    for (i = 0; i < nb_output_files; i++) {
-        if (strcmp(output_files[i]->format->name, "rtp"))
-            want_sdp = 0;
-    }
-
     current_time = ti = get_benchmark_time_stamps();
     if (transcode() < 0)
         exit_program(1);
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index cae9800da3..d926254a73 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -646,7 +646,6 @@ extern char *qsv_device;
 #endif
 extern HWDevice *filter_hw_device;
 
-extern int want_sdp;
 extern unsigned nb_output_dumped;
 extern int main_return_code;
 
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index aff40776f6..6c3052ebe1 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -38,6 +38,8 @@ struct Muxer {
     int header_written;
 };
 
+static int want_sdp = 1;
+
 static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
 {
     int i;
@@ -349,6 +351,9 @@ int of_muxer_init(OutputFile *of, int64_t limit_filesize)
 
     mux->limit_filesize = limit_filesize;
 
+    if (strcmp(of->format->name, "rtp"))
+        want_sdp = 0;
+
     return 0;
 }
 
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 12/28] ffmpeg: write the header for stream-less outputs when initializing the muxer
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (9 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 11/28] ffmpeg: set want_sdp when initializing the muxer Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 13/28] ffmpeg: move closing the file into of_write_trailer() Anton Khirnov
                   ` (15 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

There is no reason to delay this.
---
 fftools/ffmpeg.c     | 11 -----------
 fftools/ffmpeg_mux.c |  7 +++++++
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index d856b5b38f..f28cc17f51 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3313,7 +3313,6 @@ static void report_new_stream(int input_index, AVPacket *pkt)
 static int transcode_init(void)
 {
     int ret = 0, i, j, k;
-    AVFormatContext *oc;
     OutputStream *ost;
     InputStream *ist;
     char error[1024] = {0};
@@ -3387,16 +3386,6 @@ static int transcode_init(void)
         }
     }
 
-    /* write headers for files with no streams */
-    for (i = 0; i < nb_output_files; i++) {
-        oc = output_files[i]->ctx;
-        if (output_files[i]->format->flags & AVFMT_NOSTREAMS && oc->nb_streams == 0) {
-            ret = of_check_init(output_files[i]);
-            if (ret < 0)
-                goto dump_format;
-        }
-    }
-
  dump_format:
     /* dump the stream mapping */
     av_log(NULL, AV_LOG_INFO, "Stream mapping:\n");
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 6c3052ebe1..3b322ecb71 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -354,6 +354,13 @@ int of_muxer_init(OutputFile *of, int64_t limit_filesize)
     if (strcmp(of->format->name, "rtp"))
         want_sdp = 0;
 
+    /* write the header for files with no streams */
+    if (of->format->flags & AVFMT_NOSTREAMS && of->ctx->nb_streams == 0) {
+        int ret = of_check_init(of);
+        if (ret < 0)
+            return ret;
+    }
+
     return 0;
 }
 
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 13/28] ffmpeg: move closing the file into of_write_trailer()
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (10 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 12/28] ffmpeg: write the header for stream-less outputs " Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 14/28] ffmpeg: refactor the code checking for bitexact output Anton Khirnov
                   ` (14 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

The current code postpones closing the files until after printing the
final report, which accesses the output file size. Deal with this by
storing the final file size before closing the file.
---
 fftools/ffmpeg.c     | 13 -------------
 fftools/ffmpeg_mux.c | 16 +++++++++++++++-
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index f28cc17f51..4f5c2cc49a 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -4348,7 +4348,6 @@ static int transcode_step(void)
 static int transcode(void)
 {
     int ret, i;
-    AVFormatContext *os;
     OutputStream *ost;
     InputStream *ist;
     int64_t timer_start;
@@ -4417,18 +4416,6 @@ static int transcode(void)
     /* dump report by using the first video and audio streams */
     print_report(1, timer_start, av_gettime_relative());
 
-    /* close the output files */
-    for (i = 0; i < nb_output_files; i++) {
-        os = output_files[i]->ctx;
-        if (os && os->oformat && !(os->oformat->flags & AVFMT_NOFILE)) {
-            if ((ret = avio_closep(&os->pb)) < 0) {
-                av_log(NULL, AV_LOG_ERROR, "Error closing file %s: %s\n", os->url, av_err2str(ret));
-                if (exit_on_error)
-                    exit_program(1);
-            }
-        }
-    }
-
     /* close each encoder */
     for (i = 0; i < nb_output_streams; i++) {
         ost = output_streams[i];
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 3b322ecb71..13b5d2d220 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -35,6 +35,7 @@
 struct Muxer {
     /* filesize limit expressed in bytes */
     int64_t limit_filesize;
+    int64_t final_filesize;
     int header_written;
 };
 
@@ -318,6 +319,17 @@ int of_write_trailer(OutputFile *of)
         return ret;
     }
 
+    of->mux->final_filesize = of_filesize(of);
+
+    if (!(of->format->flags & AVFMT_NOFILE)) {
+        ret = avio_closep(&of->ctx->pb);
+        if (ret < 0) {
+            av_log(NULL, AV_LOG_ERROR, "Error closing file %s: %s\n",
+                   of->ctx->url, av_err2str(ret));
+            return ret;
+        }
+    }
+
     return 0;
 }
 
@@ -374,7 +386,9 @@ int64_t of_filesize(OutputFile *of)
     AVIOContext *pb = of->ctx->pb;
     int64_t ret = -1;
 
-    if (pb) {
+    if (of->mux->final_filesize)
+        ret = of->mux->final_filesize;
+    else if (pb) {
         ret = avio_size(pb);
         if (ret <= 0) // FIXME improve avio_size() so it works with non seekable output too
             ret = avio_tell(pb);
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 14/28] ffmpeg: refactor the code checking for bitexact output
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (11 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 13/28] ffmpeg: move closing the file into of_write_trailer() Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 15/28] ffmpeg: access output file chapters through a wrapper Anton Khirnov
                   ` (13 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Figure out earlier whether the output stream/file should be bitexact and
store this information in a flag in OutputFile/OutputStream.

Stop accessing the muxer in set_encoder_id(), which will become
forbidden in future commits.
---
 fftools/ffmpeg.c     | 21 +--------------------
 fftools/ffmpeg.h     |  2 ++
 fftools/ffmpeg_opt.c | 27 ++++++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 4f5c2cc49a..5165573674 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2864,37 +2864,18 @@ static int init_output_stream_streamcopy(OutputStream *ost)
 
 static void set_encoder_id(OutputFile *of, OutputStream *ost)
 {
-    const AVDictionaryEntry *e;
-
     uint8_t *encoder_string;
     int encoder_string_len;
-    int format_flags = 0;
-    int codec_flags = ost->enc_ctx->flags;
 
     if (av_dict_get(ost->st->metadata, "encoder",  NULL, 0))
         return;
 
-    e = av_dict_get(of->opts, "fflags", NULL, 0);
-    if (e) {
-        const AVOption *o = av_opt_find(of->ctx, "fflags", NULL, 0, 0);
-        if (!o)
-            return;
-        av_opt_eval_flags(of->ctx, o, e->value, &format_flags);
-    }
-    e = av_dict_get(ost->encoder_opts, "flags", NULL, 0);
-    if (e) {
-        const AVOption *o = av_opt_find(ost->enc_ctx, "flags", NULL, 0, 0);
-        if (!o)
-            return;
-        av_opt_eval_flags(ost->enc_ctx, o, e->value, &codec_flags);
-    }
-
     encoder_string_len = sizeof(LIBAVCODEC_IDENT) + strlen(ost->enc->name) + 2;
     encoder_string     = av_mallocz(encoder_string_len);
     if (!encoder_string)
         exit_program(1);
 
-    if (!(format_flags & AVFMT_FLAG_BITEXACT) && !(codec_flags & AV_CODEC_FLAG_BITEXACT))
+    if (!of->bitexact && !ost->bitexact)
         av_strlcpy(encoder_string, LIBAVCODEC_IDENT " ", encoder_string_len);
     else
         av_strlcpy(encoder_string, "Lavc ", encoder_string_len);
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index d926254a73..66062f0072 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -491,6 +491,7 @@ typedef struct OutputStream {
     int top_field_first;
     int rotate_overridden;
     int autoscale;
+    int bitexact;
     int bits_per_raw_sample;
     double rotate_override_value;
 
@@ -588,6 +589,7 @@ typedef struct OutputFile {
     int64_t start_time;      ///< start time in microseconds == AV_TIME_BASE units
 
     int shortest;
+    int bitexact;
 } OutputFile;
 
 extern InputStream **input_streams;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 0b4f437912..ed3fd818d0 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1427,6 +1427,22 @@ static int choose_encoder(OptionsContext *o, AVFormatContext *s, OutputStream *o
     return 0;
 }
 
+static int check_opt_bitexact(void *ctx, const AVDictionary *opts,
+                              const char *opt_name, int flag)
+{
+    const AVDictionaryEntry *e = av_dict_get(opts, opt_name, NULL, 0);
+
+    if (e) {
+        const AVOption *o = av_opt_find(ctx, opt_name, NULL, 0, 0);
+        int val = 0;
+        if (!o)
+            return 0;
+        av_opt_eval_flags(ctx, o, e->value, &val);
+        return !!(val & flag);
+    }
+    return 0;
+}
+
 static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, enum AVMediaType type, int source_index)
 {
     OutputStream *ost;
@@ -1522,8 +1538,13 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
     }
 
 
-    if (o->bitexact)
+    if (o->bitexact) {
         ost->enc_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
+        ost->bitexact        = 1;
+    } else {
+        ost->bitexact        = check_opt_bitexact(ost->enc_ctx, ost->encoder_opts, "flags",
+                                                  AV_CODEC_FLAG_BITEXACT);
+    }
 
     MATCH_PER_STREAM_OPT(time_bases, str, time_base, oc, st);
     if (time_base) {
@@ -2308,6 +2329,10 @@ static int open_output_file(OptionsContext *o, const char *filename)
 
     if (o->bitexact) {
         oc->flags    |= AVFMT_FLAG_BITEXACT;
+        of->bitexact  = 1;
+    } else {
+        of->bitexact  = check_opt_bitexact(oc, of->opts, "fflags",
+                                           AVFMT_FLAG_BITEXACT);
     }
 
     /* create streams for all unlabeled output pads */
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 15/28] ffmpeg: access output file chapters through a wrapper
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (12 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 14/28] ffmpeg: refactor the code checking for bitexact output Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 16/28] ffmpeg: do not log to the muxer context Anton Khirnov
                   ` (12 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Avoid accessing the muxer context directly, as this will become
forbidden in future commits.
---
 fftools/ffmpeg.c     | 15 +++++++++------
 fftools/ffmpeg.h     |  2 ++
 fftools/ffmpeg_mux.c |  7 +++++++
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 5165573674..2637b25526 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2909,12 +2909,15 @@ static void parse_forced_key_frames(char *kf, OutputStream *ost,
             *next++ = 0;
 
         if (!memcmp(p, "chapters", 8)) {
-
-            AVFormatContext *avf = output_files[ost->file_index]->ctx;
+            OutputFile *of = output_files[ost->file_index];
+            AVChapter * const *ch;
+            unsigned int    nb_ch;
             int j;
 
-            if (avf->nb_chapters > INT_MAX - size ||
-                !(pts = av_realloc_f(pts, size += avf->nb_chapters - 1,
+            ch = of_get_chapters(of, &nb_ch);
+
+            if (nb_ch > INT_MAX - size ||
+                !(pts = av_realloc_f(pts, size += nb_ch - 1,
                                      sizeof(*pts)))) {
                 av_log(NULL, AV_LOG_FATAL,
                        "Could not allocate forced key frames array.\n");
@@ -2923,8 +2926,8 @@ static void parse_forced_key_frames(char *kf, OutputStream *ost,
             t = p[8] ? parse_time_or_die("force_key_frames", p + 8, 1) : 0;
             t = av_rescale_q(t, AV_TIME_BASE_Q, avctx->time_base);
 
-            for (j = 0; j < avf->nb_chapters; j++) {
-                AVChapter *c = avf->chapters[j];
+            for (j = 0; j < nb_ch; j++) {
+                const AVChapter *c = ch[j];
                 av_assert1(index < size);
                 pts[index++] = av_rescale_q(c->start, c->time_base,
                                             avctx->time_base) + t;
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 66062f0072..e828f71dc0 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -697,5 +697,7 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
                      int unqueue);
 int of_finished(OutputFile *of);
 int64_t of_filesize(OutputFile *of);
+AVChapter * const *
+of_get_chapters(OutputFile *of, unsigned int *nb_chapters);
 
 #endif /* FFTOOLS_FFMPEG_H */
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 13b5d2d220..f4d76e1533 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -396,3 +396,10 @@ int64_t of_filesize(OutputFile *of)
 
     return ret;
 }
+
+AVChapter * const *
+of_get_chapters(OutputFile *of, unsigned int *nb_chapters)
+{
+    *nb_chapters = of->ctx->nb_chapters;
+    return of->ctx->chapters;
+}
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 16/28] ffmpeg: do not log to the muxer context
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (13 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 15/28] ffmpeg: access output file chapters through a wrapper Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data Anton Khirnov
                   ` (11 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

All other logging goes to NULL context.
---
 fftools/ffmpeg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 2637b25526..6c774e9615 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2954,7 +2954,6 @@ static void init_encoder_time_base(OutputStream *ost, AVRational default_time_ba
 {
     InputStream *ist = get_input_stream(ost);
     AVCodecContext *enc_ctx = ost->enc_ctx;
-    AVFormatContext *oc;
 
     if (ost->enc_timebase.num > 0) {
         enc_ctx->time_base = ost->enc_timebase;
@@ -2967,8 +2966,9 @@ static void init_encoder_time_base(OutputStream *ost, AVRational default_time_ba
             return;
         }
 
-        oc = output_files[ost->file_index]->ctx;
-        av_log(oc, AV_LOG_WARNING, "Input stream data not available, using default time base\n");
+        av_log(NULL, AV_LOG_WARNING,
+               "Input stream data for output stream #%d:%d not available, "
+               "using default time base\n", ost->file_index, ost->index);
     }
 
     enc_ctx->time_base = default_time_base;
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (14 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 16/28] ffmpeg: do not log to the muxer context Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-13 10:50   ` Andreas Rheinhardt
  2022-01-17 22:29   ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 18/28] ffmpeg: fix initial muxing queue size Anton Khirnov
                   ` (10 subsequent siblings)
  26 siblings, 2 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

The muxing queue currently lives in OutputStream, which is a very large
struct storing the state for both encoding and muxing. The muxing queue
is only used by the code in ffmpeg_mux, so it makes sense to restrict it
to that file.

This makes the first step towards reducing the scope of OutputStream.
---
 fftools/ffmpeg.c     |  9 -----
 fftools/ffmpeg.h     |  9 -----
 fftools/ffmpeg_mux.c | 91 ++++++++++++++++++++++++++++++++++++--------
 fftools/ffmpeg_opt.c |  6 ---
 4 files changed, 76 insertions(+), 39 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 6c774e9615..c1bb3926c4 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -595,15 +595,6 @@ static void ffmpeg_cleanup(int ret)
         avcodec_free_context(&ost->enc_ctx);
         avcodec_parameters_free(&ost->ref_par);
 
-        if (ost->muxing_queue) {
-            while (av_fifo_size(ost->muxing_queue)) {
-                AVPacket *pkt;
-                av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
-                av_packet_free(&pkt);
-            }
-            av_fifo_freep(&ost->muxing_queue);
-        }
-
         av_freep(&output_streams[i]);
     }
 #if HAVE_THREADS
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index e828f71dc0..28df1b179f 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -555,15 +555,6 @@ typedef struct OutputStream {
 
     int max_muxing_queue_size;
 
-    /* the packets are buffered here until the muxer is ready to be initialized */
-    AVFifoBuffer *muxing_queue;
-
-    /*
-     * The size of the AVPackets' buffers in queue.
-     * Updated when a packet is either pushed or pulled from the queue.
-     */
-    size_t muxing_queue_data_size;
-
     /* Threshold after which max_muxing_queue_size will be in effect */
     size_t muxing_queue_data_threshold;
 
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index f4d76e1533..f03202bbb7 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -32,7 +32,20 @@
 #include "libavformat/avformat.h"
 #include "libavformat/avio.h"
 
+typedef struct MuxStream {
+    /* the packets are buffered here until the muxer is ready to be initialized */
+    AVFifoBuffer *muxing_queue;
+
+    /*
+     * The size of the AVPackets' buffers in queue.
+     * Updated when a packet is either pushed or pulled from the queue.
+     */
+    size_t muxing_queue_data_size;
+} MuxStream;
+
 struct Muxer {
+    MuxStream *streams;
+
     /* filesize limit expressed in bytes */
     int64_t limit_filesize;
     int64_t final_filesize;
@@ -55,6 +68,7 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
 {
     AVFormatContext *s = of->ctx;
     AVStream *st = ost->st;
+    MuxStream *ms = &of->mux->streams[st->index];
     int ret;
 
     /*
@@ -76,10 +90,10 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
     if (!of->mux->header_written) {
         AVPacket *tmp_pkt;
         /* the muxer is not initialized yet, buffer the packet */
-        if (!av_fifo_space(ost->muxing_queue)) {
-            size_t cur_size = av_fifo_size(ost->muxing_queue);
+        if (!av_fifo_space(ms->muxing_queue)) {
+            size_t cur_size = av_fifo_size(ms->muxing_queue);
             unsigned int are_we_over_size =
-                (ost->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
+                (ms->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
             size_t limit    = are_we_over_size ? ost->max_muxing_queue_size : INT_MAX;
             size_t new_size = FFMIN(2 * cur_size, limit);
 
@@ -89,7 +103,7 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
                        ost->file_index, ost->st->index);
                 exit_program(1);
             }
-            ret = av_fifo_realloc2(ost->muxing_queue, new_size);
+            ret = av_fifo_realloc2(ms->muxing_queue, new_size);
             if (ret < 0)
                 exit_program(1);
         }
@@ -100,8 +114,8 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
         if (!tmp_pkt)
             exit_program(1);
         av_packet_move_ref(tmp_pkt, pkt);
-        ost->muxing_queue_data_size += tmp_pkt->size;
-        av_fifo_generic_write(ost->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
+        ms->muxing_queue_data_size += tmp_pkt->size;
+        av_fifo_generic_write(ms->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
         return;
     }
 
@@ -283,16 +297,17 @@ int of_check_init(OutputFile *of)
 
     /* flush the muxing queues */
     for (i = 0; i < of->ctx->nb_streams; i++) {
+        MuxStream     *ms = &of->mux->streams[i];
         OutputStream *ost = output_streams[of->ost_index + i];
 
         /* try to improve muxing time_base (only possible if nothing has been written yet) */
-        if (!av_fifo_size(ost->muxing_queue))
+        if (!av_fifo_size(ms->muxing_queue))
             ost->mux_timebase = ost->st->time_base;
 
-        while (av_fifo_size(ost->muxing_queue)) {
+        while (av_fifo_size(ms->muxing_queue)) {
             AVPacket *pkt;
-            av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
-            ost->muxing_queue_data_size -= pkt->size;
+            av_fifo_generic_read(ms->muxing_queue, &pkt, sizeof(pkt), NULL);
+            ms->muxing_queue_data_size -= pkt->size;
             of_write_packet(of, pkt, ost, 1);
             av_packet_free(&pkt);
         }
@@ -333,6 +348,31 @@ int of_write_trailer(OutputFile *of)
     return 0;
 }
 
+static void mux_free(Muxer **pmux, int nb_streams)
+{
+    Muxer *mux = *pmux;
+
+    if (!mux)
+        return;
+
+    for (int i = 0; i < nb_streams; i++) {
+        MuxStream *ms = &mux->streams[i];
+
+        if (!ms->muxing_queue)
+            continue;
+
+        while (av_fifo_size(ms->muxing_queue)) {
+            AVPacket *pkt;
+            av_fifo_generic_read(ms->muxing_queue, &pkt, sizeof(pkt), NULL);
+            av_packet_free(&pkt);
+        }
+        av_fifo_freep(&ms->muxing_queue);
+    }
+    av_freep(&mux->streams);
+
+    av_freep(pmux);
+}
+
 void of_close(OutputFile **pof)
 {
     OutputFile *of = *pof;
@@ -342,25 +382,42 @@ void of_close(OutputFile **pof)
         return;
 
     s = of->ctx;
+
+    mux_free(&of->mux, s ? s->nb_streams : 0);
+
     if (s && s->oformat && !(s->oformat->flags & AVFMT_NOFILE))
         avio_closep(&s->pb);
     avformat_free_context(s);
     av_dict_free(&of->opts);
 
-    av_freep(&of->mux);
-
     av_freep(pof);
 }
 
 int of_muxer_init(OutputFile *of, int64_t limit_filesize)
 {
     Muxer *mux = av_mallocz(sizeof(*mux));
+    int ret = 0;
 
     if (!mux)
         return AVERROR(ENOMEM);
 
+    mux->streams = av_calloc(of->ctx->nb_streams, sizeof(*mux->streams));
+    if (!mux->streams) {
+        av_freep(&mux);
+        return AVERROR(ENOMEM);
+    }
+
     of->mux  = mux;
 
+    for (int i = 0; i < of->ctx->nb_streams; i++) {
+        MuxStream *ms = &mux->streams[i];
+        ms->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket));
+        if (!ms->muxing_queue) {
+            ret = AVERROR(ENOMEM);
+            goto fail;
+        }
+    }
+
     mux->limit_filesize = limit_filesize;
 
     if (strcmp(of->format->name, "rtp"))
@@ -368,12 +425,16 @@ int of_muxer_init(OutputFile *of, int64_t limit_filesize)
 
     /* write the header for files with no streams */
     if (of->format->flags & AVFMT_NOSTREAMS && of->ctx->nb_streams == 0) {
-        int ret = of_check_init(of);
+        ret = of_check_init(of);
         if (ret < 0)
-            return ret;
+            goto fail;
     }
 
-    return 0;
+fail:
+    if (ret < 0)
+        mux_free(&of->mux, of->ctx->nb_streams);
+
+    return ret;
 }
 
 int of_finished(OutputFile *of)
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index ed3fd818d0..c7d1d21a37 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1613,8 +1613,6 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
     ost->max_muxing_queue_size = FFMIN(ost->max_muxing_queue_size, INT_MAX / sizeof(ost->pkt));
     ost->max_muxing_queue_size *= sizeof(ost->pkt);
 
-    ost->muxing_queue_data_size = 0;
-
     ost->muxing_queue_data_threshold = 50*1024*1024;
     MATCH_PER_STREAM_OPT(muxing_queue_data_threshold, i, ost->muxing_queue_data_threshold, oc, st);
 
@@ -1638,10 +1636,6 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
     }
     ost->last_mux_dts = AV_NOPTS_VALUE;
 
-    ost->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket));
-    if (!ost->muxing_queue)
-        exit_program(1);
-
     return ost;
 }
 
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 18/28] ffmpeg: fix initial muxing queue size
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (15 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 19/28] ffmpeg_mux: split queuing packets into a separate function Anton Khirnov
                   ` (9 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

It stores pointers to AVPacket, not AVPackets themselves.
---
 fftools/ffmpeg_mux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index f03202bbb7..f48031096d 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -411,7 +411,7 @@ int of_muxer_init(OutputFile *of, int64_t limit_filesize)
 
     for (int i = 0; i < of->ctx->nb_streams; i++) {
         MuxStream *ms = &mux->streams[i];
-        ms->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket));
+        ms->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket*));
         if (!ms->muxing_queue) {
             ret = AVERROR(ENOMEM);
             goto fail;
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 19/28] ffmpeg_mux: split queuing packets into a separate function
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (16 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 18/28] ffmpeg: fix initial muxing queue size Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 20/28] ffmpeg_mux: split of_write_packet() Anton Khirnov
                   ` (8 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

---
 fftools/ffmpeg_mux.c | 72 +++++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 28 deletions(-)

diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index f48031096d..4ab2279739 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -63,12 +63,50 @@ static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream,
     }
 }
 
+static int queue_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
+{
+    MuxStream *ms = &of->mux->streams[ost->index];
+    AVPacket *tmp_pkt;
+    int ret;
+
+    if (!av_fifo_space(ms->muxing_queue)) {
+        size_t cur_size = av_fifo_size(ms->muxing_queue);
+        unsigned int are_we_over_size =
+            (ms->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
+        size_t limit    = are_we_over_size ? ost->max_muxing_queue_size : INT_MAX;
+        size_t new_size = FFMIN(2 * cur_size, limit);
+
+        if (new_size <= cur_size) {
+            av_log(NULL, AV_LOG_ERROR,
+                   "Too many packets buffered for output stream %d:%d.\n",
+                   ost->file_index, ost->st->index);
+            return AVERROR(ENOSPC);
+        }
+        ret = av_fifo_realloc2(ms->muxing_queue, new_size);
+        if (ret < 0)
+            return ret;
+    }
+
+    ret = av_packet_make_refcounted(pkt);
+    if (ret < 0)
+        return ret;
+
+    tmp_pkt = av_packet_alloc();
+    if (!tmp_pkt)
+        return AVERROR(ENOMEM);
+
+    av_packet_move_ref(tmp_pkt, pkt);
+    ms->muxing_queue_data_size += tmp_pkt->size;
+    av_fifo_generic_write(ms->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
+
+    return 0;
+}
+
 void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
                      int unqueue)
 {
     AVFormatContext *s = of->ctx;
     AVStream *st = ost->st;
-    MuxStream *ms = &of->mux->streams[st->index];
     int ret;
 
     /*
@@ -87,35 +125,13 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
         ost->frame_number++;
     }
 
+    /* the muxer is not initialized yet, buffer the packet */
     if (!of->mux->header_written) {
-        AVPacket *tmp_pkt;
-        /* the muxer is not initialized yet, buffer the packet */
-        if (!av_fifo_space(ms->muxing_queue)) {
-            size_t cur_size = av_fifo_size(ms->muxing_queue);
-            unsigned int are_we_over_size =
-                (ms->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
-            size_t limit    = are_we_over_size ? ost->max_muxing_queue_size : INT_MAX;
-            size_t new_size = FFMIN(2 * cur_size, limit);
-
-            if (new_size <= cur_size) {
-                av_log(NULL, AV_LOG_ERROR,
-                       "Too many packets buffered for output stream %d:%d.\n",
-                       ost->file_index, ost->st->index);
-                exit_program(1);
-            }
-            ret = av_fifo_realloc2(ms->muxing_queue, new_size);
-            if (ret < 0)
-                exit_program(1);
-        }
-        ret = av_packet_make_refcounted(pkt);
-        if (ret < 0)
-            exit_program(1);
-        tmp_pkt = av_packet_alloc();
-        if (!tmp_pkt)
+        ret = queue_packet(of, ost, pkt);
+        if (ret < 0) {
+            av_packet_unref(pkt);
             exit_program(1);
-        av_packet_move_ref(tmp_pkt, pkt);
-        ms->muxing_queue_data_size += tmp_pkt->size;
-        av_fifo_generic_write(ms->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
+        }
         return;
     }
 
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 20/28] ffmpeg_mux: split of_write_packet()
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (17 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 19/28] ffmpeg_mux: split queuing packets into a separate function Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 21/28] ffmpeg: move a comment to a more appropriate place Anton Khirnov
                   ` (7 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

It is currently called from two places:
- output_packet() in ffmpeg.c, which submits the newly available output
  packet to the muxer
- from of_check_init() in ffmpeg_mux.c after the header has been
  written, to flush the muxing queue

Some packets will thus be processed by this function twice, so it
requires an extra parameter to indicate the place it is called from and
avoid modifying some state twice.

This is fragile and hard to follow, so split this function into two.
Also rename of_write_packet() to of_submit_packet() to better reflect
its new purpose.
---
 fftools/ffmpeg.c     |  4 +--
 fftools/ffmpeg.h     |  3 +--
 fftools/ffmpeg_mux.c | 63 ++++++++++++++++++++++++--------------------
 3 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index c1bb3926c4..4215be0098 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -728,11 +728,11 @@ static void output_packet(OutputFile *of, AVPacket *pkt,
         if (ret < 0)
             goto finish;
         while ((ret = av_bsf_receive_packet(ost->bsf_ctx, pkt)) >= 0)
-            of_write_packet(of, pkt, ost, 0);
+            of_submit_packet(of, pkt, ost);
         if (ret == AVERROR(EAGAIN))
             ret = 0;
     } else if (!eof)
-        of_write_packet(of, pkt, ost, 0);
+        of_submit_packet(of, pkt, ost);
 
 finish:
     if (ret < 0 && ret != AVERROR_EOF) {
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 28df1b179f..374dca9189 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -684,8 +684,7 @@ int of_check_init(OutputFile *of);
 int of_write_trailer(OutputFile *of);
 void of_close(OutputFile **pof);
 
-void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
-                     int unqueue);
+void of_submit_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost);
 int of_finished(OutputFile *of);
 int64_t of_filesize(OutputFile *of);
 AVChapter * const *
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 4ab2279739..36f781ae11 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -102,39 +102,12 @@ static int queue_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
     return 0;
 }
 
-void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
-                     int unqueue)
+static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
 {
     AVFormatContext *s = of->ctx;
     AVStream *st = ost->st;
     int ret;
 
-    /*
-     * Audio encoders may split the packets --  #frames in != #packets out.
-     * But there is no reordering, so we can limit the number of output packets
-     * by simply dropping them here.
-     * Counting encoded video frames needs to be done separately because of
-     * reordering, see do_video_out().
-     * Do not count the packet when unqueued because it has been counted when queued.
-     */
-    if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed) && !unqueue) {
-        if (ost->frame_number >= ost->max_frames) {
-            av_packet_unref(pkt);
-            return;
-        }
-        ost->frame_number++;
-    }
-
-    /* the muxer is not initialized yet, buffer the packet */
-    if (!of->mux->header_written) {
-        ret = queue_packet(of, ost, pkt);
-        if (ret < 0) {
-            av_packet_unref(pkt);
-            exit_program(1);
-        }
-        return;
-    }
-
     if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && video_sync_method == VSYNC_DROP) ||
         (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && audio_sync_method < 0))
         pkt->pts = pkt->dts = AV_NOPTS_VALUE;
@@ -225,6 +198,38 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
     }
 }
 
+void of_submit_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost)
+{
+    AVStream *st = ost->st;
+    int ret;
+
+    /*
+     * Audio encoders may split the packets --  #frames in != #packets out.
+     * But there is no reordering, so we can limit the number of output packets
+     * by simply dropping them here.
+     * Counting encoded video frames needs to be done separately because of
+     * reordering, see do_video_out().
+     */
+    if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed)) {
+        if (ost->frame_number >= ost->max_frames) {
+            av_packet_unref(pkt);
+            return;
+        }
+        ost->frame_number++;
+    }
+
+    if (of->mux->header_written) {
+        write_packet(of, ost, pkt);
+    } else {
+        /* the muxer is not initialized yet, buffer the packet */
+        ret = queue_packet(of, ost, pkt);
+        if (ret < 0) {
+            av_packet_unref(pkt);
+            exit_program(1);
+        }
+    }
+}
+
 static int print_sdp(void)
 {
     char sdp[16384];
@@ -324,7 +329,7 @@ int of_check_init(OutputFile *of)
             AVPacket *pkt;
             av_fifo_generic_read(ms->muxing_queue, &pkt, sizeof(pkt), NULL);
             ms->muxing_queue_data_size -= pkt->size;
-            of_write_packet(of, pkt, ost, 1);
+            write_packet(of, ost, pkt);
             av_packet_free(&pkt);
         }
     }
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 21/28] ffmpeg: move a comment to a more appropriate place
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (18 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 20/28] ffmpeg_mux: split of_write_packet() Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 22/28] ffmpeg: move output file opts into private context Anton Khirnov
                   ` (6 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

---
 fftools/ffmpeg.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 4215be0098..f39b1b9375 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1064,6 +1064,11 @@ static void do_video_out(OutputFile *of,
         }
     }
 
+    /*
+     * For video, number of frames in == number of packets out.
+     * But there may be reordering, so we can't throw away frames on encoder
+     * flush, we need to limit them here, before they go into encoder.
+     */
     nb_frames = FFMIN(nb_frames, ost->max_frames - ost->frame_number);
     nb0_frames = FFMIN(nb0_frames, nb_frames);
 
@@ -1217,11 +1222,6 @@ static void do_video_out(OutputFile *of,
             }
         }
         ost->sync_opts++;
-        /*
-         * For video, number of frames in == number of packets out.
-         * But there may be reordering, so we can't throw away frames on encoder
-         * flush, we need to limit them here, before they go into encoder.
-         */
         ost->frame_number++;
 
         if (vstats_filename && frame_size)
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 22/28] ffmpeg: move output file opts into private context
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (19 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 21/28] ffmpeg: move a comment to a more appropriate place Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 23/28] ffmpeg: move processing video stats to ffmpeg_mux Anton Khirnov
                   ` (5 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

It is private to the muxer, no reason to access it from outside.
---
 fftools/ffmpeg.h     |  3 +--
 fftools/ffmpeg_mux.c |  9 ++++++---
 fftools/ffmpeg_opt.c | 12 ++++++------
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 374dca9189..d9f997bf7a 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -574,7 +574,6 @@ typedef struct OutputFile {
     const AVOutputFormat *format;
 
     AVFormatContext *ctx;
-    AVDictionary *opts;
     int ost_index;       /* index of the first stream in output_streams */
     int64_t recording_time;  ///< desired length of the resulting file in microseconds == AV_TIME_BASE units
     int64_t start_time;      ///< start time in microseconds == AV_TIME_BASE units
@@ -678,7 +677,7 @@ int hw_device_setup_for_filter(FilterGraph *fg);
 
 int hwaccel_decode_init(AVCodecContext *avctx);
 
-int of_muxer_init(OutputFile *of, int64_t limit_filesize);
+int of_muxer_init(OutputFile *of, AVDictionary *opts, int64_t limit_filesize);
 /* open the muxer when all the streams are initialized */
 int of_check_init(OutputFile *of);
 int of_write_trailer(OutputFile *of);
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 36f781ae11..729cc1a7d8 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -46,6 +46,8 @@ typedef struct MuxStream {
 struct Muxer {
     MuxStream *streams;
 
+    AVDictionary *opts;
+
     /* filesize limit expressed in bytes */
     int64_t limit_filesize;
     int64_t final_filesize;
@@ -294,7 +296,7 @@ int of_check_init(OutputFile *of)
             return 0;
     }
 
-    ret = avformat_write_header(of->ctx, &of->opts);
+    ret = avformat_write_header(of->ctx, &of->mux->opts);
     if (ret < 0) {
         av_log(NULL, AV_LOG_ERROR,
                "Could not write header for output file #%d "
@@ -390,6 +392,7 @@ static void mux_free(Muxer **pmux, int nb_streams)
         av_fifo_freep(&ms->muxing_queue);
     }
     av_freep(&mux->streams);
+    av_dict_free(&mux->opts);
 
     av_freep(pmux);
 }
@@ -409,12 +412,11 @@ void of_close(OutputFile **pof)
     if (s && s->oformat && !(s->oformat->flags & AVFMT_NOFILE))
         avio_closep(&s->pb);
     avformat_free_context(s);
-    av_dict_free(&of->opts);
 
     av_freep(pof);
 }
 
-int of_muxer_init(OutputFile *of, int64_t limit_filesize)
+int of_muxer_init(OutputFile *of, AVDictionary *opts, int64_t limit_filesize)
 {
     Muxer *mux = av_mallocz(sizeof(*mux));
     int ret = 0;
@@ -440,6 +442,7 @@ int of_muxer_init(OutputFile *of, int64_t limit_filesize)
     }
 
     mux->limit_filesize = limit_filesize;
+    mux->opts           = opts;
 
     if (strcmp(of->format->name, "rtp"))
         want_sdp = 0;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index c7d1d21a37..cb21a6a42c 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2278,7 +2278,7 @@ static int open_output_file(OptionsContext *o, const char *filename)
     OutputFile *of;
     OutputStream *ost;
     InputStream  *ist;
-    AVDictionary *unused_opts = NULL;
+    AVDictionary *unused_opts = NULL, *format_opts = NULL;
     const AVDictionaryEntry *e = NULL;
 
     if (o->stop_time != INT64_MAX && o->recording_time != INT64_MAX) {
@@ -2303,7 +2303,7 @@ static int open_output_file(OptionsContext *o, const char *filename)
     of->recording_time = o->recording_time;
     of->start_time     = o->start_time;
     of->shortest       = o->shortest;
-    av_dict_copy(&of->opts, o->g->format_opts, 0);
+    av_dict_copy(&format_opts, o->g->format_opts, 0);
 
     if (!strcmp(filename, "-"))
         filename = "pipe:";
@@ -2325,7 +2325,7 @@ static int open_output_file(OptionsContext *o, const char *filename)
         oc->flags    |= AVFMT_FLAG_BITEXACT;
         of->bitexact  = 1;
     } else {
-        of->bitexact  = check_opt_bitexact(oc, of->opts, "fflags",
+        of->bitexact  = check_opt_bitexact(oc, format_opts, "fflags",
                                            AVFMT_FLAG_BITEXACT);
     }
 
@@ -2702,7 +2702,7 @@ loop_end:
         /* open the file */
         if ((err = avio_open2(&oc->pb, filename, AVIO_FLAG_WRITE,
                               &oc->interrupt_callback,
-                              &of->opts)) < 0) {
+                              &format_opts)) < 0) {
             print_error(filename, err);
             exit_program(1);
         }
@@ -2710,7 +2710,7 @@ loop_end:
         assert_file_overwrite(filename);
 
     if (o->mux_preload) {
-        av_dict_set_int(&of->opts, "preload", o->mux_preload*AV_TIME_BASE, 0);
+        av_dict_set_int(&format_opts, "preload", o->mux_preload*AV_TIME_BASE, 0);
     }
     oc->max_delay = (int)(o->mux_max_delay * AV_TIME_BASE);
 
@@ -2904,7 +2904,7 @@ loop_end:
         exit_program(1);
     }
 
-    err = of_muxer_init(of, o->limit_filesize);
+    err = of_muxer_init(of, format_opts, o->limit_filesize);
     if (err < 0) {
         av_log(NULL, AV_LOG_FATAL, "Error initializing internal muxing state\n");
         exit_program(1);
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 23/28] ffmpeg: move processing video stats to ffmpeg_mux
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (20 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 22/28] ffmpeg: move output file opts into private context Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation Anton Khirnov
                   ` (4 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Currently it is called from
- do_video_out(), at the end of each encode loop iteration
- from flush_encoders(), after muxing each packet

Since this function processes the data from the last encoded packet,
neither of the above is fully correct, because
- an encoder can in principle produce multiple packets per one submitted
  frame
- bitstream filters may modify the number of encoded packets or their
  properties.

It thus makes most sense to call this function right before sending the
packet to the muxer.
---
 fftools/ffmpeg.c     | 61 +-------------------------------------------
 fftools/ffmpeg.h     |  5 ++++
 fftools/ffmpeg_mux.c | 47 ++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 60 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index f39b1b9375..c89a95937e 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -109,7 +109,7 @@
 const char program_name[] = "ffmpeg";
 const int program_birth_year = 2000;
 
-static FILE *vstats_file;
+FILE *vstats_file;
 
 const char *const forced_keyframes_const_names[] = {
     "n",
@@ -126,7 +126,6 @@ typedef struct BenchmarkTimeStamps {
     int64_t sys_usec;
 } BenchmarkTimeStamps;
 
-static void do_video_stats(OutputStream *ost, int frame_size);
 static BenchmarkTimeStamps get_benchmark_time_stamps(void);
 static int64_t getmaxrss(void);
 static int ifilter_has_all_input_formats(FilterGraph *fg);
@@ -977,7 +976,6 @@ static void do_video_out(OutputFile *of,
     double delta, delta0;
     double duration = 0;
     double sync_ipts = AV_NOPTS_VALUE;
-    int frame_size = 0;
     InputStream *ist = NULL;
     AVFilterContext *filter = ost->filter->filter;
 
@@ -1213,7 +1211,6 @@ static void do_video_out(OutputFile *of,
                     av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &ost->mux_timebase));
             }
 
-            frame_size = pkt->size;
             output_packet(of, pkt, ost, 0);
 
             /* if two pass, output log */
@@ -1223,9 +1220,6 @@ static void do_video_out(OutputFile *of,
         }
         ost->sync_opts++;
         ost->frame_number++;
-
-        if (vstats_filename && frame_size)
-            do_video_stats(ost, frame_size);
     }
 
     av_frame_unref(ost->last_frame);
@@ -1238,54 +1232,6 @@ error:
     exit_program(1);
 }
 
-static double psnr(double d)
-{
-    return -10.0 * log10(d);
-}
-
-static void do_video_stats(OutputStream *ost, int frame_size)
-{
-    AVCodecContext *enc;
-    int frame_number;
-    double ti1, bitrate, avg_bitrate;
-
-    /* this is executed just the first time do_video_stats is called */
-    if (!vstats_file) {
-        vstats_file = fopen(vstats_filename, "w");
-        if (!vstats_file) {
-            perror("fopen");
-            exit_program(1);
-        }
-    }
-
-    enc = ost->enc_ctx;
-    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
-        frame_number = ost->st->nb_frames;
-        if (vstats_version <= 1) {
-            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
-                    ost->quality / (float)FF_QP2LAMBDA);
-        } else  {
-            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
-                    ost->quality / (float)FF_QP2LAMBDA);
-        }
-
-        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
-            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
-
-        fprintf(vstats_file,"f_size= %6d ", frame_size);
-        /* compute pts value */
-        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
-        if (ti1 < 0.01)
-            ti1 = 0.01;
-
-        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
-        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
-        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
-               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
-        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
-    }
-}
-
 static void finish_output_stream(OutputStream *ost)
 {
     OutputFile *of = output_files[ost->file_index];
@@ -1764,7 +1710,6 @@ static void flush_encoders(void)
         for (;;) {
             const char *desc = NULL;
             AVPacket *pkt = ost->pkt;
-            int pkt_size;
 
             switch (enc->codec_type) {
             case AVMEDIA_TYPE_AUDIO:
@@ -1808,11 +1753,7 @@ static void flush_encoders(void)
                 continue;
             }
             av_packet_rescale_ts(pkt, enc->time_base, ost->mux_timebase);
-            pkt_size = pkt->size;
             output_packet(of, pkt, ost, 0);
-            if (ost->enc_ctx->codec_type == AVMEDIA_TYPE_VIDEO && vstats_filename) {
-                do_video_stats(ost, pkt_size);
-            }
         }
     }
 }
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index d9f997bf7a..908dc2eb38 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -624,6 +624,7 @@ extern int stdin_interaction;
 extern int frame_bits_per_raw_sample;
 extern AVIOContext *progress_avio;
 extern float max_error_rate;
+extern FILE *vstats_file;
 
 extern char *filter_nbthreads;
 extern int filter_complex_nbthreads;
@@ -641,6 +642,10 @@ extern HWDevice *filter_hw_device;
 extern unsigned nb_output_dumped;
 extern int main_return_code;
 
+static inline double psnr(double d)
+{
+    return -10.0 * log10(d);
+}
 
 void term_init(void);
 void term_exit(void);
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 729cc1a7d8..76d9d4b9c4 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -56,6 +56,49 @@ struct Muxer {
 
 static int want_sdp = 1;
 
+static void do_video_stats(OutputStream *ost, int frame_size)
+{
+    AVCodecContext *enc;
+    int frame_number;
+    double ti1, bitrate, avg_bitrate;
+
+    /* this is executed just the first time do_video_stats is called */
+    if (!vstats_file) {
+        vstats_file = fopen(vstats_filename, "w");
+        if (!vstats_file) {
+            perror("fopen");
+            exit_program(1);
+        }
+    }
+
+    enc = ost->enc_ctx;
+    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
+        frame_number = ost->st->nb_frames;
+        if (vstats_version <= 1) {
+            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
+                    ost->quality / (float)FF_QP2LAMBDA);
+        } else  {
+            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
+                    ost->quality / (float)FF_QP2LAMBDA);
+        }
+
+        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
+            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
+
+        fprintf(vstats_file,"f_size= %6d ", frame_size);
+        /* compute pts value */
+        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
+        if (ti1 < 0.01)
+            ti1 = 0.01;
+
+        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
+        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
+        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
+               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
+        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
+    }
+}
+
 static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
 {
     int i;
@@ -180,6 +223,10 @@ static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
     ost->data_size += pkt->size;
     ost->packets_written++;
 
+    if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
+        ost->encoding_needed && vstats_filename)
+        do_video_stats(ost, pkt->size);
+
     pkt->stream_index = ost->index;
 
     if (debug_ts) {
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (21 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 23/28] ffmpeg: move processing video stats to ffmpeg_mux Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-13 10:54   ` Andreas Rheinhardt
  2022-01-18 10:16   ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 25/28] ffmpeg_mux: stop using AVStream.nb_frames in do_video_stats() Anton Khirnov
                   ` (3 subsequent siblings)
  26 siblings, 2 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

do_video_stats() is only ever called for video.
---
 fftools/ffmpeg_mux.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 76d9d4b9c4..8a64661c9c 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -72,31 +72,29 @@ static void do_video_stats(OutputStream *ost, int frame_size)
     }
 
     enc = ost->enc_ctx;
-    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
-        frame_number = ost->st->nb_frames;
-        if (vstats_version <= 1) {
-            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
-                    ost->quality / (float)FF_QP2LAMBDA);
-        } else  {
-            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
-                    ost->quality / (float)FF_QP2LAMBDA);
-        }
+    frame_number = ost->st->nb_frames;
+    if (vstats_version <= 1) {
+        fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
+                ost->quality / (float)FF_QP2LAMBDA);
+    } else  {
+        fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
+                ost->quality / (float)FF_QP2LAMBDA);
+    }
 
-        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
-            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
+    if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
+        fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
 
-        fprintf(vstats_file,"f_size= %6d ", frame_size);
-        /* compute pts value */
-        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
-        if (ti1 < 0.01)
-            ti1 = 0.01;
+    fprintf(vstats_file,"f_size= %6d ", frame_size);
+    /* compute pts value */
+    ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
+    if (ti1 < 0.01)
+        ti1 = 0.01;
 
-        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
-        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
-        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
-               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
-        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
-    }
+    bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
+    avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
+    fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
+           (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
+    fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
 }
 
 static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 25/28] ffmpeg_mux: stop using AVStream.nb_frames in do_video_stats()
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (22 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 26/28] ffmpeg_mux: stop using av_stream_get_end_pts() " Anton Khirnov
                   ` (2 subsequent siblings)
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

Its use for muxing is not documented, in practice it is incremented per
each packet successfully passed to the muxer's write_packet(). Since
there is a lot of indirection between ffmpeg submitting a packet to the
muxer and it actually being written (e.g. the interleaving queue), using
nb_frames to count packets sent to the muxer is incorrect. Use
OutputStream.packets_written instead.
---
 fftools/ffmpeg_mux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 8a64661c9c..ef6b7ddd97 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -72,7 +72,7 @@ static void do_video_stats(OutputStream *ost, int frame_size)
     }
 
     enc = ost->enc_ctx;
-    frame_number = ost->st->nb_frames;
+    frame_number = ost->packets_written - 1;
     if (vstats_version <= 1) {
         fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
                 ost->quality / (float)FF_QP2LAMBDA);
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 26/28] ffmpeg_mux: stop using av_stream_get_end_pts() in do_video_stats()
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (23 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 25/28] ffmpeg_mux: stop using AVStream.nb_frames in do_video_stats() Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 27/28] ffmpeg_mux: merge variable declaration and initialization Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 28/28] ffmpeg_mux: move processing AV_PKT_DATA_QUALITY_STATS to do_video_stats() Anton Khirnov
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

It retrieves libavformat's internal dts value (contrary to the
function's name), which is not necessary here because we can access the
packet directly.
---
 fftools/ffmpeg_mux.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index ef6b7ddd97..54020880eb 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -56,7 +56,7 @@ struct Muxer {
 
 static int want_sdp = 1;
 
-static void do_video_stats(OutputStream *ost, int frame_size)
+static void do_video_stats(OutputStream *ost, const AVPacket *pkt)
 {
     AVCodecContext *enc;
     int frame_number;
@@ -84,13 +84,13 @@ static void do_video_stats(OutputStream *ost, int frame_size)
     if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
         fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
 
-    fprintf(vstats_file,"f_size= %6d ", frame_size);
+    fprintf(vstats_file,"f_size= %6d ", pkt->size);
     /* compute pts value */
-    ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
+    ti1 = pkt->dts * av_q2d(ost->st->time_base);
     if (ti1 < 0.01)
         ti1 = 0.01;
 
-    bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
+    bitrate     = (pkt->size * 8) / av_q2d(enc->time_base) / 1000.0;
     avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
     fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
            (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
@@ -223,7 +223,7 @@ static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
 
     if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
         ost->encoding_needed && vstats_filename)
-        do_video_stats(ost, pkt->size);
+        do_video_stats(ost, pkt);
 
     pkt->stream_index = ost->index;
 
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 27/28] ffmpeg_mux: merge variable declaration and initialization
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (24 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 26/28] ffmpeg_mux: stop using av_stream_get_end_pts() " Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 28/28] ffmpeg_mux: move processing AV_PKT_DATA_QUALITY_STATS to do_video_stats() Anton Khirnov
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

---
 fftools/ffmpeg_mux.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 54020880eb..21771b3ae6 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -58,7 +58,7 @@ static int want_sdp = 1;
 
 static void do_video_stats(OutputStream *ost, const AVPacket *pkt)
 {
-    AVCodecContext *enc;
+    AVCodecContext *enc = ost->enc_ctx;
     int frame_number;
     double ti1, bitrate, avg_bitrate;
 
@@ -71,7 +71,6 @@ static void do_video_stats(OutputStream *ost, const AVPacket *pkt)
         }
     }
 
-    enc = ost->enc_ctx;
     frame_number = ost->packets_written - 1;
     if (vstats_version <= 1) {
         fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
-- 
2.33.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".

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

* [FFmpeg-devel] [PATCH 28/28] ffmpeg_mux: move processing AV_PKT_DATA_QUALITY_STATS to do_video_stats()
  2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
                   ` (25 preceding siblings ...)
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 27/28] ffmpeg_mux: merge variable declaration and initialization Anton Khirnov
@ 2022-01-11  9:58 ` Anton Khirnov
  26 siblings, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-11  9:58 UTC (permalink / raw)
  To: ffmpeg-devel

This is a more appropriate place for this code, since the values we read
from AV_PKT_DATA_QUALITY_STATS side data are primarily written into
video stats.

Rename the function to update_video_stats() to better reflect its new
purpose.
---
 fftools/ffmpeg_mux.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 21771b3ae6..52986b002a 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -56,13 +56,28 @@ struct Muxer {
 
 static int want_sdp = 1;
 
-static void do_video_stats(OutputStream *ost, const AVPacket *pkt)
+static void update_video_stats(OutputStream *ost, const AVPacket *pkt, int write_vstats)
 {
+    const uint8_t *sd = av_packet_get_side_data(pkt, AV_PKT_DATA_QUALITY_STATS,
+                                                NULL);
     AVCodecContext *enc = ost->enc_ctx;
     int frame_number;
     double ti1, bitrate, avg_bitrate;
 
-    /* this is executed just the first time do_video_stats is called */
+    ost->quality = sd ? AV_RL32(sd) : -1;
+    ost->pict_type = sd ? sd[4] : AV_PICTURE_TYPE_NONE;
+
+    for (int i = 0; i < FF_ARRAY_ELEMS(ost->error); i++) {
+        if (sd && i < sd[5])
+            ost->error[i] = AV_RL64(sd + 8 + 8 * i);
+        else
+            ost->error[i] = -1;
+    }
+
+    if (!write_vstats)
+        return;
+
+    /* this is executed just the first time update_video_stats is called */
     if (!vstats_file) {
         vstats_file = fopen(vstats_filename, "w");
         if (!vstats_file) {
@@ -155,19 +170,6 @@ static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
         pkt->pts = pkt->dts = AV_NOPTS_VALUE;
 
     if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
-        int i;
-        uint8_t *sd = av_packet_get_side_data(pkt, AV_PKT_DATA_QUALITY_STATS,
-                                              NULL);
-        ost->quality = sd ? AV_RL32(sd) : -1;
-        ost->pict_type = sd ? sd[4] : AV_PICTURE_TYPE_NONE;
-
-        for (i = 0; i<FF_ARRAY_ELEMS(ost->error); i++) {
-            if (sd && i < sd[5])
-                ost->error[i] = AV_RL64(sd + 8 + 8*i);
-            else
-                ost->error[i] = -1;
-        }
-
         if (ost->frame_rate.num && ost->is_cfr) {
             if (pkt->duration > 0)
                 av_log(NULL, AV_LOG_WARNING, "Overriding packet duration by frame rate, this should not happen\n");
@@ -220,9 +222,8 @@ static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
     ost->data_size += pkt->size;
     ost->packets_written++;
 
-    if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
-        ost->encoding_needed && vstats_filename)
-        do_video_stats(ost, pkt);
+    if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed)
+        update_video_stats(ost, pkt, !!vstats_filename);
 
     pkt->stream_index = ost->index;
 
-- 
2.33.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".

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

* Re: [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data Anton Khirnov
@ 2022-01-13 10:50   ` Andreas Rheinhardt
  2022-01-17 22:29   ` Anton Khirnov
  1 sibling, 0 replies; 36+ messages in thread
From: Andreas Rheinhardt @ 2022-01-13 10:50 UTC (permalink / raw)
  To: ffmpeg-devel

Anton Khirnov:
> The muxing queue currently lives in OutputStream, which is a very large
> struct storing the state for both encoding and muxing. The muxing queue
> is only used by the code in ffmpeg_mux, so it makes sense to restrict it
> to that file.
> 
> This makes the first step towards reducing the scope of OutputStream.
> ---
>  fftools/ffmpeg.c     |  9 -----
>  fftools/ffmpeg.h     |  9 -----
>  fftools/ffmpeg_mux.c | 91 ++++++++++++++++++++++++++++++++++++--------
>  fftools/ffmpeg_opt.c |  6 ---
>  4 files changed, 76 insertions(+), 39 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 6c774e9615..c1bb3926c4 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -595,15 +595,6 @@ static void ffmpeg_cleanup(int ret)
>          avcodec_free_context(&ost->enc_ctx);
>          avcodec_parameters_free(&ost->ref_par);
>  
> -        if (ost->muxing_queue) {
> -            while (av_fifo_size(ost->muxing_queue)) {
> -                AVPacket *pkt;
> -                av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
> -                av_packet_free(&pkt);
> -            }
> -            av_fifo_freep(&ost->muxing_queue);
> -        }
> -
>          av_freep(&output_streams[i]);
>      }
>  #if HAVE_THREADS
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index e828f71dc0..28df1b179f 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -555,15 +555,6 @@ typedef struct OutputStream {
>  
>      int max_muxing_queue_size;
>  
> -    /* the packets are buffered here until the muxer is ready to be initialized */
> -    AVFifoBuffer *muxing_queue;
> -
> -    /*
> -     * The size of the AVPackets' buffers in queue.
> -     * Updated when a packet is either pushed or pulled from the queue.
> -     */
> -    size_t muxing_queue_data_size;
> -
>      /* Threshold after which max_muxing_queue_size will be in effect */
>      size_t muxing_queue_data_threshold;
>  
> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> index f4d76e1533..f03202bbb7 100644
> --- a/fftools/ffmpeg_mux.c
> +++ b/fftools/ffmpeg_mux.c
> @@ -32,7 +32,20 @@
>  #include "libavformat/avformat.h"
>  #include "libavformat/avio.h"
>  
> +typedef struct MuxStream {
> +    /* the packets are buffered here until the muxer is ready to be initialized */
> +    AVFifoBuffer *muxing_queue;
> +
> +    /*
> +     * The size of the AVPackets' buffers in queue.
> +     * Updated when a packet is either pushed or pulled from the queue.
> +     */
> +    size_t muxing_queue_data_size;
> +} MuxStream;
> +
>  struct Muxer {
> +    MuxStream *streams;
> +
>      /* filesize limit expressed in bytes */
>      int64_t limit_filesize;
>      int64_t final_filesize;
> @@ -55,6 +68,7 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
>  {
>      AVFormatContext *s = of->ctx;
>      AVStream *st = ost->st;
> +    MuxStream *ms = &of->mux->streams[st->index];
>      int ret;
>  
>      /*
> @@ -76,10 +90,10 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
>      if (!of->mux->header_written) {
>          AVPacket *tmp_pkt;
>          /* the muxer is not initialized yet, buffer the packet */
> -        if (!av_fifo_space(ost->muxing_queue)) {
> -            size_t cur_size = av_fifo_size(ost->muxing_queue);
> +        if (!av_fifo_space(ms->muxing_queue)) {
> +            size_t cur_size = av_fifo_size(ms->muxing_queue);
>              unsigned int are_we_over_size =
> -                (ost->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
> +                (ms->muxing_queue_data_size + pkt->size) > ost->muxing_queue_data_threshold;
>              size_t limit    = are_we_over_size ? ost->max_muxing_queue_size : INT_MAX;
>              size_t new_size = FFMIN(2 * cur_size, limit);
>  
> @@ -89,7 +103,7 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
>                         ost->file_index, ost->st->index);
>                  exit_program(1);
>              }
> -            ret = av_fifo_realloc2(ost->muxing_queue, new_size);
> +            ret = av_fifo_realloc2(ms->muxing_queue, new_size);
>              if (ret < 0)
>                  exit_program(1);
>          }
> @@ -100,8 +114,8 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
>          if (!tmp_pkt)
>              exit_program(1);
>          av_packet_move_ref(tmp_pkt, pkt);
> -        ost->muxing_queue_data_size += tmp_pkt->size;
> -        av_fifo_generic_write(ost->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
> +        ms->muxing_queue_data_size += tmp_pkt->size;
> +        av_fifo_generic_write(ms->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), NULL);
>          return;
>      }
>  
> @@ -283,16 +297,17 @@ int of_check_init(OutputFile *of)
>  
>      /* flush the muxing queues */
>      for (i = 0; i < of->ctx->nb_streams; i++) {
> +        MuxStream     *ms = &of->mux->streams[i];
>          OutputStream *ost = output_streams[of->ost_index + i];
>  
>          /* try to improve muxing time_base (only possible if nothing has been written yet) */
> -        if (!av_fifo_size(ost->muxing_queue))
> +        if (!av_fifo_size(ms->muxing_queue))
>              ost->mux_timebase = ost->st->time_base;
>  
> -        while (av_fifo_size(ost->muxing_queue)) {
> +        while (av_fifo_size(ms->muxing_queue)) {
>              AVPacket *pkt;
> -            av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
> -            ost->muxing_queue_data_size -= pkt->size;
> +            av_fifo_generic_read(ms->muxing_queue, &pkt, sizeof(pkt), NULL);
> +            ms->muxing_queue_data_size -= pkt->size;
>              of_write_packet(of, pkt, ost, 1);
>              av_packet_free(&pkt);
>          }
> @@ -333,6 +348,31 @@ int of_write_trailer(OutputFile *of)
>      return 0;
>  }
>  
> +static void mux_free(Muxer **pmux, int nb_streams)
> +{
> +    Muxer *mux = *pmux;
> +
> +    if (!mux)
> +        return;
> +
> +    for (int i = 0; i < nb_streams; i++) {
> +        MuxStream *ms = &mux->streams[i];
> +
> +        if (!ms->muxing_queue)
> +            continue;
> +
> +        while (av_fifo_size(ms->muxing_queue)) {
> +            AVPacket *pkt;
> +            av_fifo_generic_read(ms->muxing_queue, &pkt, sizeof(pkt), NULL);
> +            av_packet_free(&pkt);
> +        }
> +        av_fifo_freep(&ms->muxing_queue);
> +    }
> +    av_freep(&mux->streams);
> +
> +    av_freep(pmux);
> +}
> +
>  void of_close(OutputFile **pof)
>  {
>      OutputFile *of = *pof;
> @@ -342,25 +382,42 @@ void of_close(OutputFile **pof)
>          return;
>  
>      s = of->ctx;
> +
> +    mux_free(&of->mux, s ? s->nb_streams : 0);
> +
>      if (s && s->oformat && !(s->oformat->flags & AVFMT_NOFILE))
>          avio_closep(&s->pb);
>      avformat_free_context(s);
>      av_dict_free(&of->opts);
>  
> -    av_freep(&of->mux);
> -
>      av_freep(pof);
>  }
>  
>  int of_muxer_init(OutputFile *of, int64_t limit_filesize)
>  {
>      Muxer *mux = av_mallocz(sizeof(*mux));
> +    int ret = 0;
>  
>      if (!mux)
>          return AVERROR(ENOMEM);
>  
> +    mux->streams = av_calloc(of->ctx->nb_streams, sizeof(*mux->streams));
> +    if (!mux->streams) {
> +        av_freep(&mux);
> +        return AVERROR(ENOMEM);
> +    }
> +
>      of->mux  = mux;
>  
> +    for (int i = 0; i < of->ctx->nb_streams; i++) {
> +        MuxStream *ms = &mux->streams[i];
> +        ms->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket));
> +        if (!ms->muxing_queue) {
> +            ret = AVERROR(ENOMEM);
> +            goto fail;
> +        }
> +    }
> +
>      mux->limit_filesize = limit_filesize;
>  
>      if (strcmp(of->format->name, "rtp"))
> @@ -368,12 +425,16 @@ int of_muxer_init(OutputFile *of, int64_t limit_filesize)
>  
>      /* write the header for files with no streams */
>      if (of->format->flags & AVFMT_NOSTREAMS && of->ctx->nb_streams == 0) {
> -        int ret = of_check_init(of);
> +        ret = of_check_init(of);
>          if (ret < 0)
> -            return ret;
> +            goto fail;
>      }
>  
> -    return 0;
> +fail:
> +    if (ret < 0)
> +        mux_free(&of->mux, of->ctx->nb_streams);
> +
> +    return ret;
>  }
>  
>  int of_finished(OutputFile *of)
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index ed3fd818d0..c7d1d21a37 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1613,8 +1613,6 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>      ost->max_muxing_queue_size = FFMIN(ost->max_muxing_queue_size, INT_MAX / sizeof(ost->pkt));
>      ost->max_muxing_queue_size *= sizeof(ost->pkt);
>  
> -    ost->muxing_queue_data_size = 0;
> -
>      ost->muxing_queue_data_threshold = 50*1024*1024;
>      MATCH_PER_STREAM_OPT(muxing_queue_data_threshold, i, ost->muxing_queue_data_threshold, oc, st);
>  
> @@ -1638,10 +1636,6 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>      }
>      ost->last_mux_dts = AV_NOPTS_VALUE;
>  
> -    ost->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket));
> -    if (!ost->muxing_queue)
> -        exit_program(1);
> -
>      return ost;
>  }
>  
> 

My objections to adding a separately allocated muxing context and to
this MuxStream have not changed. Both incur unnecessary allocations and
indirections and (in case of the latter) loops; the latter is also very
unnatural. The patch here actually shows it: You only use the muxer
context to get the MuxStream context corresponding to the OutputStream
you are interested in:

>      for (i = 0; i < of->ctx->nb_streams; i++) {
> +        MuxStream     *ms = &of->mux->streams[i];
>          OutputStream *ost = output_streams[of->ost_index + i];

>      AVStream *st = ost->st;
> +    MuxStream *ms = &of->mux->streams[st->index];
>      int ret;

Your aim of making sure what code can use/modify what parts can also be
fulfilled by comments.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

* Re: [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation Anton Khirnov
@ 2022-01-13 10:54   ` Andreas Rheinhardt
  2022-01-18 10:16   ` Anton Khirnov
  1 sibling, 0 replies; 36+ messages in thread
From: Andreas Rheinhardt @ 2022-01-13 10:54 UTC (permalink / raw)
  To: ffmpeg-devel

Anton Khirnov:
> do_video_stats() is only ever called for video.
> ---
>  fftools/ffmpeg_mux.c | 42 ++++++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> index 76d9d4b9c4..8a64661c9c 100644
> --- a/fftools/ffmpeg_mux.c
> +++ b/fftools/ffmpeg_mux.c
> @@ -72,31 +72,29 @@ static void do_video_stats(OutputStream *ost, int frame_size)
>      }
>  
>      enc = ost->enc_ctx;
> -    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
> -        frame_number = ost->st->nb_frames;
> -        if (vstats_version <= 1) {
> -            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
> -                    ost->quality / (float)FF_QP2LAMBDA);
> -        } else  {
> -            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
> -                    ost->quality / (float)FF_QP2LAMBDA);
> -        }
> +    frame_number = ost->st->nb_frames;
> +    if (vstats_version <= 1) {
> +        fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
> +                ost->quality / (float)FF_QP2LAMBDA);
> +    } else  {
> +        fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
> +                ost->quality / (float)FF_QP2LAMBDA);
> +    }
>  
> -        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
> -            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
> +    if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
> +        fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
>  
> -        fprintf(vstats_file,"f_size= %6d ", frame_size);
> -        /* compute pts value */
> -        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
> -        if (ti1 < 0.01)
> -            ti1 = 0.01;
> +    fprintf(vstats_file,"f_size= %6d ", frame_size);
> +    /* compute pts value */
> +    ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
> +    if (ti1 < 0.01)
> +        ti1 = 0.01;
>  
> -        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
> -        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
> -        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
> -               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
> -        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
> -    }
> +    bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
> +    avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
> +    fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
> +           (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
> +    fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
>  }
>  
>  static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
> 

LGTM to removing the check, but it would be better if you removed the
check before moving the code to ffmpeg_mux.c and then fixed the
indentation while moving the code to ffmpeg_mux.c.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

* Re: [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data Anton Khirnov
  2022-01-13 10:50   ` Andreas Rheinhardt
@ 2022-01-17 22:29   ` Anton Khirnov
  1 sibling, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-17 22:29 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Andreas Rheinhardt (2022-01-13 11:50:48)
> 
> My objections to adding a separately allocated muxing context and to
> this MuxStream have not changed. Both incur unnecessary allocations
> and indirections and (in case of the latter) loops;

I cannot imagine any remotely real situation where the cost of these
allocation and loops is anywhere close to being meaningful. So your
argument amounts to vastly premature optimization.

> the latter is also very unnatural. The patch here actually shows it:
> You only use the muxer context to get the MuxStream context
> corresponding to the OutputStream you are interested in:

I don't see how that shows any unnaturalness. As I said last time
already - the idea is to (eventually) split OutputStream into separate
encoding and muxing objects that can be used independently.

If you really insist I can wait until actually implementing that split,
but I'd rather avoid giant branches if possible.

> 
> >      for (i = 0; i < of->ctx->nb_streams; i++) {
> > +        MuxStream     *ms = &of->mux->streams[i];
> >          OutputStream *ost = output_streams[of->ost_index + i];
> 
> >      AVStream *st = ost->st;
> > +    MuxStream *ms = &of->mux->streams[st->index];
> >      int ret;
> 
> Your aim of making sure what code can use/modify what parts can also be
> fulfilled by comments.

In my experience that simply does not work. People either don't read the
comments at all or read them once and forget. Did you see what happened
to all those fields in AVStream marked as "libavformat private use only"?

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

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

* Re: [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation
  2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation Anton Khirnov
  2022-01-13 10:54   ` Andreas Rheinhardt
@ 2022-01-18 10:16   ` Anton Khirnov
  2022-01-18 10:18     ` Andreas Rheinhardt
  2022-01-18 10:25     ` Anton Khirnov
  1 sibling, 2 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-18 10:16 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Andreas Rheinhardt (2022-01-13 11:54:59)
> Anton Khirnov:
> > do_video_stats() is only ever called for video.
> > ---
> >  fftools/ffmpeg_mux.c | 42 ++++++++++++++++++++----------------------
> >  1 file changed, 20 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> > index 76d9d4b9c4..8a64661c9c 100644
> > --- a/fftools/ffmpeg_mux.c
> > +++ b/fftools/ffmpeg_mux.c
> > @@ -72,31 +72,29 @@ static void do_video_stats(OutputStream *ost, int frame_size)
> >      }
> >  
> >      enc = ost->enc_ctx;
> > -    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
> > -        frame_number = ost->st->nb_frames;
> > -        if (vstats_version <= 1) {
> > -            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
> > -                    ost->quality / (float)FF_QP2LAMBDA);
> > -        } else  {
> > -            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
> > -                    ost->quality / (float)FF_QP2LAMBDA);
> > -        }
> > +    frame_number = ost->st->nb_frames;
> > +    if (vstats_version <= 1) {
> > +        fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
> > +                ost->quality / (float)FF_QP2LAMBDA);
> > +    } else  {
> > +        fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
> > +                ost->quality / (float)FF_QP2LAMBDA);
> > +    }
> >  
> > -        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
> > -            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
> > +    if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
> > +        fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
> >  
> > -        fprintf(vstats_file,"f_size= %6d ", frame_size);
> > -        /* compute pts value */
> > -        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
> > -        if (ti1 < 0.01)
> > -            ti1 = 0.01;
> > +    fprintf(vstats_file,"f_size= %6d ", frame_size);
> > +    /* compute pts value */
> > +    ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
> > +    if (ti1 < 0.01)
> > +        ti1 = 0.01;
> >  
> > -        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
> > -        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
> > -        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
> > -               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
> > -        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
> > -    }
> > +    bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
> > +    avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
> > +    fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
> > +           (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
> > +    fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
> >  }
> >  
> >  static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
> > 
> 
> LGTM to removing the check, but it would be better if you removed the
> check before moving the code to ffmpeg_mux.c and then fixed the
> indentation while moving the code to ffmpeg_mux.c.

Better why? What's the point of essentially re-doing these commits and
dealing with rebase conflicts, just to get the same result in the end?

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

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

* Re: [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation
  2022-01-18 10:16   ` Anton Khirnov
@ 2022-01-18 10:18     ` Andreas Rheinhardt
  2022-01-18 10:25     ` Anton Khirnov
  1 sibling, 0 replies; 36+ messages in thread
From: Andreas Rheinhardt @ 2022-01-18 10:18 UTC (permalink / raw)
  To: ffmpeg-devel

Anton Khirnov:
> Quoting Andreas Rheinhardt (2022-01-13 11:54:59)
>> Anton Khirnov:
>>> do_video_stats() is only ever called for video.
>>> ---
>>>  fftools/ffmpeg_mux.c | 42 ++++++++++++++++++++----------------------
>>>  1 file changed, 20 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
>>> index 76d9d4b9c4..8a64661c9c 100644
>>> --- a/fftools/ffmpeg_mux.c
>>> +++ b/fftools/ffmpeg_mux.c
>>> @@ -72,31 +72,29 @@ static void do_video_stats(OutputStream *ost, int frame_size)
>>>      }
>>>  
>>>      enc = ost->enc_ctx;
>>> -    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
>>> -        frame_number = ost->st->nb_frames;
>>> -        if (vstats_version <= 1) {
>>> -            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
>>> -                    ost->quality / (float)FF_QP2LAMBDA);
>>> -        } else  {
>>> -            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
>>> -                    ost->quality / (float)FF_QP2LAMBDA);
>>> -        }
>>> +    frame_number = ost->st->nb_frames;
>>> +    if (vstats_version <= 1) {
>>> +        fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
>>> +                ost->quality / (float)FF_QP2LAMBDA);
>>> +    } else  {
>>> +        fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
>>> +                ost->quality / (float)FF_QP2LAMBDA);
>>> +    }
>>>  
>>> -        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
>>> -            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
>>> +    if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
>>> +        fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
>>>  
>>> -        fprintf(vstats_file,"f_size= %6d ", frame_size);
>>> -        /* compute pts value */
>>> -        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
>>> -        if (ti1 < 0.01)
>>> -            ti1 = 0.01;
>>> +    fprintf(vstats_file,"f_size= %6d ", frame_size);
>>> +    /* compute pts value */
>>> +    ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
>>> +    if (ti1 < 0.01)
>>> +        ti1 = 0.01;
>>>  
>>> -        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
>>> -        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
>>> -        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
>>> -               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
>>> -        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
>>> -    }
>>> +    bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
>>> +    avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
>>> +    fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
>>> +           (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
>>> +    fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
>>>  }
>>>  
>>>  static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
>>>
>>
>> LGTM to removing the check, but it would be better if you removed the
>> check before moving the code to ffmpeg_mux.c and then fixed the
>> indentation while moving the code to ffmpeg_mux.c.
> 
> Better why? What's the point of essentially re-doing these commits and
> dealing with rebase conflicts, just to get the same result in the end?
> 

A smaller diff.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

* Re: [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation
  2022-01-18 10:16   ` Anton Khirnov
  2022-01-18 10:18     ` Andreas Rheinhardt
@ 2022-01-18 10:25     ` Anton Khirnov
  2022-01-18 10:35       ` Andreas Rheinhardt
  2022-01-18 10:52       ` Anton Khirnov
  1 sibling, 2 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-18 10:25 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Andreas Rheinhardt (2022-01-18 11:18:24)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2022-01-13 11:54:59)
> >> Anton Khirnov:
> >>> do_video_stats() is only ever called for video.
> >>> ---
> >>>  fftools/ffmpeg_mux.c | 42 ++++++++++++++++++++----------------------
> >>>  1 file changed, 20 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> >>> index 76d9d4b9c4..8a64661c9c 100644
> >>> --- a/fftools/ffmpeg_mux.c
> >>> +++ b/fftools/ffmpeg_mux.c
> >>> @@ -72,31 +72,29 @@ static void do_video_stats(OutputStream *ost, int frame_size)
> >>>      }
> >>>  
> >>>      enc = ost->enc_ctx;
> >>> -    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
> >>> -        frame_number = ost->st->nb_frames;
> >>> -        if (vstats_version <= 1) {
> >>> -            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
> >>> -                    ost->quality / (float)FF_QP2LAMBDA);
> >>> -        } else  {
> >>> -            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
> >>> -                    ost->quality / (float)FF_QP2LAMBDA);
> >>> -        }
> >>> +    frame_number = ost->st->nb_frames;
> >>> +    if (vstats_version <= 1) {
> >>> +        fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
> >>> +                ost->quality / (float)FF_QP2LAMBDA);
> >>> +    } else  {
> >>> +        fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
> >>> +                ost->quality / (float)FF_QP2LAMBDA);
> >>> +    }
> >>>  
> >>> -        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
> >>> -            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
> >>> +    if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
> >>> +        fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
> >>>  
> >>> -        fprintf(vstats_file,"f_size= %6d ", frame_size);
> >>> -        /* compute pts value */
> >>> -        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
> >>> -        if (ti1 < 0.01)
> >>> -            ti1 = 0.01;
> >>> +    fprintf(vstats_file,"f_size= %6d ", frame_size);
> >>> +    /* compute pts value */
> >>> +    ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
> >>> +    if (ti1 < 0.01)
> >>> +        ti1 = 0.01;
> >>>  
> >>> -        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
> >>> -        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
> >>> -        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
> >>> -               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
> >>> -        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
> >>> -    }
> >>> +    bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
> >>> +    avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
> >>> +    fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
> >>> +           (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
> >>> +    fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
> >>>  }
> >>>  
> >>>  static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
> >>>
> >>
> >> LGTM to removing the check, but it would be better if you removed the
> >> check before moving the code to ffmpeg_mux.c and then fixed the
> >> indentation while moving the code to ffmpeg_mux.c.
> > 
> > Better why? What's the point of essentially re-doing these commits and
> > dealing with rebase conflicts, just to get the same result in the end?
> > 
> 
> A smaller diff.

Doesn't seem worth the trouble. git show -w is already just two lines.

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

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

* Re: [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation
  2022-01-18 10:25     ` Anton Khirnov
@ 2022-01-18 10:35       ` Andreas Rheinhardt
  2022-01-18 10:52       ` Anton Khirnov
  1 sibling, 0 replies; 36+ messages in thread
From: Andreas Rheinhardt @ 2022-01-18 10:35 UTC (permalink / raw)
  To: ffmpeg-devel

Anton Khirnov:
> Quoting Andreas Rheinhardt (2022-01-18 11:18:24)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2022-01-13 11:54:59)
>>>> Anton Khirnov:
>>>>> do_video_stats() is only ever called for video.
>>>>> ---
>>>>>  fftools/ffmpeg_mux.c | 42 ++++++++++++++++++++----------------------
>>>>>  1 file changed, 20 insertions(+), 22 deletions(-)
>>>>>
>>>>> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
>>>>> index 76d9d4b9c4..8a64661c9c 100644
>>>>> --- a/fftools/ffmpeg_mux.c
>>>>> +++ b/fftools/ffmpeg_mux.c
>>>>> @@ -72,31 +72,29 @@ static void do_video_stats(OutputStream *ost, int frame_size)
>>>>>      }
>>>>>  
>>>>>      enc = ost->enc_ctx;
>>>>> -    if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
>>>>> -        frame_number = ost->st->nb_frames;
>>>>> -        if (vstats_version <= 1) {
>>>>> -            fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
>>>>> -                    ost->quality / (float)FF_QP2LAMBDA);
>>>>> -        } else  {
>>>>> -            fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
>>>>> -                    ost->quality / (float)FF_QP2LAMBDA);
>>>>> -        }
>>>>> +    frame_number = ost->st->nb_frames;
>>>>> +    if (vstats_version <= 1) {
>>>>> +        fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
>>>>> +                ost->quality / (float)FF_QP2LAMBDA);
>>>>> +    } else  {
>>>>> +        fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
>>>>> +                ost->quality / (float)FF_QP2LAMBDA);
>>>>> +    }
>>>>>  
>>>>> -        if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
>>>>> -            fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
>>>>> +    if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
>>>>> +        fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
>>>>>  
>>>>> -        fprintf(vstats_file,"f_size= %6d ", frame_size);
>>>>> -        /* compute pts value */
>>>>> -        ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
>>>>> -        if (ti1 < 0.01)
>>>>> -            ti1 = 0.01;
>>>>> +    fprintf(vstats_file,"f_size= %6d ", frame_size);
>>>>> +    /* compute pts value */
>>>>> +    ti1 = av_stream_get_end_pts(ost->st) * av_q2d(ost->st->time_base);
>>>>> +    if (ti1 < 0.01)
>>>>> +        ti1 = 0.01;
>>>>>  
>>>>> -        bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
>>>>> -        avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
>>>>> -        fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
>>>>> -               (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
>>>>> -        fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
>>>>> -    }
>>>>> +    bitrate     = (frame_size * 8) / av_q2d(enc->time_base) / 1000.0;
>>>>> +    avg_bitrate = (double)(ost->data_size * 8) / ti1 / 1000.0;
>>>>> +    fprintf(vstats_file, "s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s ",
>>>>> +           (double)ost->data_size / 1024, ti1, bitrate, avg_bitrate);
>>>>> +    fprintf(vstats_file, "type= %c\n", av_get_picture_type_char(ost->pict_type));
>>>>>  }
>>>>>  
>>>>>  static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream, OSTFinished others)
>>>>>
>>>>
>>>> LGTM to removing the check, but it would be better if you removed the
>>>> check before moving the code to ffmpeg_mux.c and then fixed the
>>>> indentation while moving the code to ffmpeg_mux.c.
>>>
>>> Better why? What's the point of essentially re-doing these commits and
>>> dealing with rebase conflicts, just to get the same result in the end?
>>>
>>
>> A smaller diff.
> 
> Doesn't seem worth the trouble. git show -w is already just two lines.
> 

And if you swapped the patches it would be only two lines for an
ordinary git show; not because of git show functionality, but because
the patch only touches two lines.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

* Re: [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation
  2022-01-18 10:25     ` Anton Khirnov
  2022-01-18 10:35       ` Andreas Rheinhardt
@ 2022-01-18 10:52       ` Anton Khirnov
  1 sibling, 0 replies; 36+ messages in thread
From: Anton Khirnov @ 2022-01-18 10:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Andreas Rheinhardt (2022-01-18 11:35:13)
> And if you swapped the patches it would be only two lines for an
> ordinary git show; not because of git show functionality, but because
> the patch only touches two lines.

And I would have to spend extra time dealing with rebase conflicts. Not
wasting people's time should be a factor in reviews.

I wouldn't mind if there was a point to it, but optimizing diff size
doesn't seem particularly meaningful to me. Especially since that code
would get touched anyway, so it's not making things easier for git blame
either.

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

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

end of thread, other threads:[~2022-01-18 10:52 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11  9:58 [FFmpeg-devel] [PATCH 01/28] ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 02/28] ffmpeg: store the output file index in OutputFile Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 03/28] ffmpeg: move some muxing-related code into a separate file Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 04/28] ffmpeg: move writing the trailer to ffmpeg_mux.c Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 05/28] ffmpeg: move freeing the output file " Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 06/28] ffmpeg: store output format separately from the muxer context Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 07/28] ffmpeg_mux: add private " Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 08/28] ffmpeg: add a helper function to access output file size Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 09/28] ffmpeg: fix the type of limit_filesize Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 10/28] ffmpeg: refactor limiting output file size with -fs Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 11/28] ffmpeg: set want_sdp when initializing the muxer Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 12/28] ffmpeg: write the header for stream-less outputs " Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 13/28] ffmpeg: move closing the file into of_write_trailer() Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 14/28] ffmpeg: refactor the code checking for bitexact output Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 15/28] ffmpeg: access output file chapters through a wrapper Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 16/28] ffmpeg: do not log to the muxer context Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data Anton Khirnov
2022-01-13 10:50   ` Andreas Rheinhardt
2022-01-17 22:29   ` Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 18/28] ffmpeg: fix initial muxing queue size Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 19/28] ffmpeg_mux: split queuing packets into a separate function Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 20/28] ffmpeg_mux: split of_write_packet() Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 21/28] ffmpeg: move a comment to a more appropriate place Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 22/28] ffmpeg: move output file opts into private context Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 23/28] ffmpeg: move processing video stats to ffmpeg_mux Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 24/28] ffmpeg_mux: drop a useless check and reduce indentation Anton Khirnov
2022-01-13 10:54   ` Andreas Rheinhardt
2022-01-18 10:16   ` Anton Khirnov
2022-01-18 10:18     ` Andreas Rheinhardt
2022-01-18 10:25     ` Anton Khirnov
2022-01-18 10:35       ` Andreas Rheinhardt
2022-01-18 10:52       ` Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 25/28] ffmpeg_mux: stop using AVStream.nb_frames in do_video_stats() Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 26/28] ffmpeg_mux: stop using av_stream_get_end_pts() " Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 27/28] ffmpeg_mux: merge variable declaration and initialization Anton Khirnov
2022-01-11  9:58 ` [FFmpeg-devel] [PATCH 28/28] ffmpeg_mux: move processing AV_PKT_DATA_QUALITY_STATS to do_video_stats() Anton Khirnov

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

This inbox may be cloned and mirrored by anyone:

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

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

Example config snippet for mirrors.


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