[FFmpeg-devel] [PATCH] libavformat: fix inputs initialization in mpegts muxer with filters

[Andreas Håkon]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, 19 de April de 2019 17:08, Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Fri, Apr 19, 2019 at 08:23:35AM +0000, Andreas Håkon via ffmpeg-devel wrote:
>
> > From 936740731c17a9757aa093bdb35d9772df1e64a8 Mon Sep 17 00:00:00 2001
> > From: Andreas Hakon andreas.hakon at protonmail.com
> > Date: Fri, 19 Apr 2019 09:17:32 +0100
> > Subject: [PATCH] libavformat: input init mpegts with filters v2
> > This patch solves the initialization of the inputs when using filters
> > (a graph filter) with the mpegts muxer.
> > This bug seems to be generated by a simple forgetting to copy. The same code is
> > repeated two times, but only in one case the variable inputs_done is initialized.
>
> > Testcase:
> > $ ffmpeg -f mpegts -i mpeg.ts \
> > -filter_complex "[i:100]fps=fps=25[out]" \
> > -map "[out]" -c:v libx264 -f mpegts out.ts
>
> i see no difference in the output (same md5)
> with what input file does this show a difference?


Hi Michael,

Let me to explain the problem inside the code. Here the relevant part of the
file "/fftools/ffmpeg.c" (line #4606):

---------------------8<----------------------
    if (ost->filter && ost->filter->graph->graph) {
        if (!ost->initialized) {
            char error[1024] = {0};
            ret = init_output_stream(ost, error, sizeof(error));
            if (ret < 0) {
                av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n",
                       ost->file_index, ost->index, error);
                exit_program(1);
            }
        }
        if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
            return ret;
        if (!ist)
            return 0;
    } else if (ost->filter) {
        int i;
        for (i = 0; i < ost->filter->graph->nb_inputs; i++) {
            InputFilter *ifilter = ost->filter->graph->inputs[i];
            if (!ifilter->ist->got_output && !input_files[ifilter->ist->file_index]->eof_reached) {
                ist = ifilter->ist;
                break;
            }
        }
        if (!ist) {
            ost->inputs_done = 1;
            return 0;
        }
    } else {
        av_assert0(ost->source_index >= 0);
        ist = input_streams[ost->source_index];
    }
---------------------8<----------------------

Please, note the first block of the "if (ost->filter && ost->filter->graph->graph)".
This block normally returns with "if (!ist) return 0;".

But the second block (the one "else if (ost->filter)") returns with
"if (!ist) {ost->inputs_done = 1; return 0;"

What the second block likes to protect is when the input stream is completed.
However, the first block doesn't do it!

One difference is that the first block uses the transcode_from_filter() function.
But that not makes difference, as the second block iterates over the inputs, and
the function transcode_from_filter() does it too.

Futhermore, the "ost->inputs_done" is used only in the function "choose_output()"
(at line #3882 of /fftools/ffmpeg.c):

---------------------8<----------------------
static OutputStream *choose_output(void)
{
    int i;
    int64_t opts_min = INT64_MAX;
    OutputStream *ost_min = NULL;

    for (i = 0; i < nb_output_streams; i++) {
        OutputStream *ost = output_streams[i];
        int64_t opts = ost->st->cur_dts == AV_NOPTS_VALUE ? INT64_MIN :
                       av_rescale_q(ost->st->cur_dts, ost->st->time_base,
                                    AV_TIME_BASE_Q);
        if (ost->st->cur_dts == AV_NOPTS_VALUE)
            av_log(NULL, AV_LOG_DEBUG,
                "cur_dts is invalid st:%d (%d) [init:%d i_done:%d finish:%d] (this is harmless if it occurs once at the start per stream)\n",
                ost->st->index, ost->st->id, ost->initialized, ost->inputs_done, ost->finished);

        if (!ost->initialized && !ost->inputs_done)
            return ost;

        if (!ost->finished && opts < opts_min) {
            opts_min = opts;
            ost_min  = ost->unavailable ? NULL : ost;
        }
    }
    return ost_min;
}
------------8<----------------------

So, the case with troubles is when the output stream is selected because
is not initilized and no inputs are completed. And without this patch
it's possible that inputs can be completed but not marked as done.

I assume that the error is a simple incorrect "copy&paste" of the original
code, as the second block sets the "ost->inputs_done = 1;" but not the
first one. And the difference between both blocks is that the first
is when a filter graph is running.

Please, revise the code! I hope you understand it when you look at my
descriptions.

Regards,
A.H.

---