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

[Michael Niedermayer]

On Tue, Apr 23, 2019 at 11:55:59PM +0200, Michael Niedermayer wrote:
> On Tue, Apr 23, 2019 at 07:16:07AM +0000, Andreas Håkon wrote:
> > 
> > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> > On Tuesday, 23 de April de 2019 1:07, Michael Niedermayer <michael at niedermayer.cc> wrote:
> > 
> > > On Mon, Apr 22, 2019 at 11:42:57AM +0000, Andreas Håkon wrote:
> > >
> > >
> > > > Please, revise the code! I hope you understand it when you look at my
> > > > descriptions.
> > >
> > > I would prefer to have some testcase that shows a problem
> > > what you write basically sounds like
> > > there are 2 similar pieces of code but they differ, so you change
> > > one assuming the other is correct. Maybe the change is correct but
> > > the explanation is insufficient
> > >
> > 
> > Hi Michael,
> > 
> > I agree that a testcase is preferable. However, I can use the Mathematical Demonstration
> > by reduction to the absurd:
> > 
> > - The only difference between block A and B (the two "if () ... else if () ...") is that the
> > first one is executed when a filter graph is running.
> > - In the second block (block B) the "ost->inputs_done = 1;" is executed prior to return.
> > - In the first block (bloc A) is not executed.
> > 
> > Why is then executed in the Block B?
> > 
> > If the only difference is the filter graph, and nothing related to inputs, then only one
> > of these two can be true:
> > 
> > 1) Or the Block B doesn't needs the code "ost->inputs_done = 1;"
> > 2) Or the Block A needs it too.
> > 
> > And I don't think the option 1 is true.
> > 
> > So by reduction to the absurd is demonstrated.
> > Regards.
> 
> nothing of what you write here has any resemblance of a Mathematical proof,
> not that it matters but you claimed that. 
> A mathematical proof requires clear definitions and also requires the proposition
> of what one proofs to be well existing at least.
> 
> I think a discussion of mathematical proofs will not help this matter 
> 
> I think a more accurate description of the situation is
> 1. we both see that there are 2 pieces of code that differ in a way that looks
>    odd
> 2. we both do not know why.
> 3. you are convinced that remooving this difference in some random way that
>    looks sensible is a good idea while i think its not a good idea to make
>    such change without better understanding first.

some analysis of the change of this patch:

The code part that is changed in the patch first checks

        if (!ost->initialized) {

and if true runs

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

init_output_stream() on failure exits the program, on success
it does 
      
      ost->initialized = 1;
      
Thus we know ost->initialized must be non zero after this
and there is no code that resets it to 0 that i can see

and the only use (read) case of ost->inputs_done is
      
      if (!ost->initialized && !ost->inputs_done)

so the code you want to add is dead code and will have no effect.

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190424/b03e9c9a/attachment.sig>