[FFmpeg-devel] [PATCH v3 0/7] Merge lazy filter initialization in ffmpeg CLI

[wm4]

On Thu, 2 Mar 2017 17:12:04 +0100
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Thu, Mar 02, 2017 at 02:37:09PM +0100, wm4 wrote:
> > On Thu, 2 Mar 2017 14:03:18 +0100
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >   
> > > On Thu, Mar 02, 2017 at 09:53:02AM +0100, wm4 wrote:  
> > > > These patches merge the previously skipped Libav commits, which made
> > > > avconv lazily initialize libavfilter graphs. This means the filters
> > > > are initialized with the actual output format, instead of whatever
> > > > libavformat reports.
> > > > 
> > > > It's a prerequisite to making hardware decoding support saner, as
> > > > hardware decoders will output a different pixfmt than the software
> > > > format reported by libavformat. This can be seen on ffmpeg_qsv.c
> > > > and ffmpeg_cuvid.c, which don't lose any functionality, even though
> > > > half of the code is removed.
> > > > 
> > > > There are some differences in how ffmpeg.c and avconv.c filter-flow
> > > > works. Also, avconv.c doesn't have sub2video. Relatively intrusive
> > > > changes were required.
> > > > 
> > > > I will push this tomorrow, except if critical failures are found.    
> > > 
> > > I think the patchset improved in terms of regressions but there are
> > > still several
> > >   
> [...]
> >  
> > > 
> > > also this code crashes with some private files:
> > > ==7506== Process terminating with default action of signal 11 (SIGSEGV)
> > > ==7506==  Access not within mapped region at address 0x8
> > > ==7506==    at 0x471529: av_buffersink_get_frame_rate (buffersink.c:193)
> > > ==7506==    by 0x435B54: init_output_stream_encode (ffmpeg.c:3217)
> > > ==7506==    by 0x4364A8: init_output_stream (ffmpeg.c:3351)
> > > ==7506==    by 0x42E4DB: reap_filters (ffmpeg.c:1428)
> > > ==7506==    by 0x43AA44: transcode_step (ffmpeg.c:4452)
> > > ==7506==    by 0x43AB28: transcode (ffmpeg.c:4496)
> > > ==7506==    by 0x43B2FD: main (ffmpeg.c:4701)  
> > 
> > I don't know if you're shitting me on purpose. I can't fix what I can't
> > reproduce, and you never gave me access to those files. Fix it yourself
> > if you think it's important. Seriously, what is this.  
> 
> I ofered access to this file to someone wanting to work on this
> previously
> "If this backtrace is not sufficient i can share the file privatly
>  with someone who wants to work on fixing this"
> 
> Noone asked for the file, if you want to work on this and keep the file
> private, i can share it with you.
> 
> Its not nice from you to ignore my ofer and then attack me pretending
> there was no such offer

OK, I missed that part. Sorry. I guess I overlooked it when I tried to
collect all the new test cases and tried to find the required samples
(for which you only provided a path on your local disk, and I had to
look them up from the ticker number in that path). So I apologize, but
I'm still grumpy.

Anyway, _please_ don't report bugs to me in the future without providing
a direct sample link (sure, you can do it privately if necessary).

In the mean time, I somehow expected you'd provide me a sample for the
case above (in private), since you know that I obviously want and don't
have it, but you didn't yet. Why make me ask.

Btw., I've tested every single case you pointed out and which I could
test.

> 
> >   
> > > This one produces a silent audio channel as previous patchsets did too
> > > ./ffmpeg -i ~/tickets/1726/Mono.thd out.wav  
> > 
> > While libavformat signals a mono channel configuration, the decoder
> > actually outputs stereo, with one channel muted. You can reproduce this
> > with current master ffmpeg too by adding "-ac 2". If this is a bug,
> > then it's merely an old bug that is made more obvious by this patch,
> > rather than introducing it.
> > 
> > I don't know why you could not determine this yourself. It would have
> > been easy to check.  
> 
> Ive a huge amount of things to do, and analyzing differences is alot
> of work, i stated that previously yet you keep attacking me, and i
> tried to do some light analysis on some things to help a bit but could
> not on all

This is not the first time you're implying that your time is somehow
more valuable than mine.

Anyway, I realize I might have misinterpreted your efforts. If the
failing examples you point out don't need to be fixed and are not an
absolute requirement for the patches to get a passing mark, but are
only provided by you as some sort of "help", then that's totally fine
with me.

I have to say this might not have the effect you want.

For example, getting handed a failure case _after_ you're done with the
work is not helpful. And I'm talking about cases that could be
triggered long ago, not just with new changes in a new iteration of the
patch set. It's a bit like throwing out a new bucket of dirt after
you've just cleaned the street, just because the house obviously needs
to be clean too. It's terrible timing.

Also, even if your intention is only to point out potential problems,
rather than analyzing each potential issue yourself, it's not
necessarily helpful to bombard someone with a number of potential false
positives. There are a lot of things to look out for, and having to
deal with all these things adds an extra load.

And this is not just about me and this specific patch set. I've seen it
with other people trying to merge something too. Also keep in mind that
we're about 500-1000 commits behind in the merges, that people want the
merges, that merges tend to cause collateral damages, that the merger
can't fix _all_ issues that might arise, that it's not necessarily a
good idea to make them fix all issues at once (tends to cause dirty
hacks to be added just to get over with it), that they have limited
time and effort, and that you might want to put a little effort into
bug reports too, even if you only want to point out potential issues.

Of course it would be different if someone were to add a new feature.
But porting someone else's work from a very-similar but different fork
is... different. In the first place, the merged work doesn't have to be
perfect. Why should the merged have to perfect it? Otherwise we might
want to discuss how exactly merges should be done. They never required
much (or any) reviews before, and suddenly it's different? (Let's be
honest, these merges are a shit show and a bit of a blight on the
project, but they're also necessary.)

In this case it was mostly not so good timing, but you have to admit
that coming up with subtle transcoding differences of fuzzed files is
almost surely bullshit. Most of the failures were corner cases. I might
have overreacted because I interpreted it so that you meant every issue
you pointed out was a blocker. Yeah, I apologize, but I'm still a bit
grumpy. I appreciate that you want to do some quality control and that
you want to help out, but I perceived it a bit different.

So my advice to you is: ignore issues that are probably obscure and not
immediate problems, and for issues that should be fixed make quality bug
reports. Don't let the merger who's just trying to get things done do
it.

> 
> 
> >   
> > > 
> > > This one looses the first displayed subtitle (the green "help")
> > > ./ffmpeg -i ~/tickets/153/bbc_small.ts -filter_complex '[0:v][0:s]overlay' -qscale 2 -t 3 test.avi  
> > 
> > The first time you mentioned this. Why didn't you do so a week ago?
> > When I complained that you come up with a new case punctually at
> > exactly the time when I want to merge the patches.  
> 
> Well, either its new or i missed it previously.
> you seem to assume i have a clean list of cases and withhold some,
> thats not the case. There is no clean list to begin with and as you
> asked me to spend time to investigate things a bit, i try to do so and
> so i try to discard cases that appear to be duplicates
> If iam wrong on one then theres a new case with the next patchset

I asked you that if you come up with failing cases, whether you could
please not provide them to me at the last second.

> Also id like to again ask for the attacks against me to stop.
> I spend huge amounts of time in FFmpeg trying to
> help but i get attacked on IRC and the ML. You realize that i just
> reported some issues in a patchset ...
> Its really strange, it feels like many people try really hard to push
> me out of the project. And the really "hillarious" part is if i leave,

It's really strange. I try to do some work for the project, but every
time I try to get it pushed, someone comes up with a few "new" failures
that weren't mentioned before and delays the patches again. It's like
someone is trying to push me out of the project.

Regarding attacks, it did feel a bit like that these last minute
reports were supposed to deliberately delay everything.

> everyone of them will pretend its a surprise and they didnt see it
> comming.

Well, you already announced that you'd leave the project a few months
ago or so, but nothing happened. As an (apparently involuntary) de-facto
project leader who refuses to fix the leadership associated problems of
the project, it's of course not strange that you get attacked once in a
while. Even if you deny being a project leader, you hold a tad too many
central key positions.

I realize you probably just want to write code and not deal with these
issues. But on the other hand it seems you're stuck in this position
whether or not you or we want it. The best idea I can come up is to
nominate a new project leader, but I don't know who could do that _and_
keep the project together.

Currently, we as a project can't make decisions, and nobody even knows
the damn rules we're supposed to play by. If it's just me, could
someone please set me straight?

I'm still going to push the patches tomorrow.