[FFmpeg-devel] policy on "necro-bumping" patches

Ronald S. Bultje rsbultje at gmail.com
Wed Sep 16 00:36:49 CEST 2015


Hi,

On Tue, Sep 15, 2015 at 6:05 PM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Tue, Sep 15, 2015 at 05:35:53PM -0400, Ganesh Ajjanagadde wrote:
> > On Tue, Sep 15, 2015 at 3:03 PM, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> > > On Tue, Sep 15, 2015 at 02:00:27PM -0400, Ganesh Ajjanagadde wrote:
> > >> On Tue, Sep 15, 2015 at 1:47 PM, Michael Niedermayer <
> michaelni at gmx.at> wrote:
> > >> > On Tue, Sep 15, 2015 at 12:47:35PM -0400, Ganesh Ajjanagadde wrote:
> > >> >> On Tue, Sep 15, 2015 at 10:54 AM, Michael Niedermayer <
> michaelni at gmx.at> wrote:
> > >> >> > On Tue, Sep 15, 2015 at 08:48:33AM -0400, Ganesh Ajjanagadde
> wrote:
> > >> >> >> On Tue, Sep 15, 2015 at 6:54 AM, Ronald S. Bultje <
> rsbultje at gmail.com> wrote:
> > >> >> >> > Hi Ganesh,
> > >> >> >> >
> > >> >> >> > On Mon, Sep 14, 2015 at 10:27 PM, Ganesh Ajjanagadde <
> gajjanag at mit.edu>
> > >> >> >> > wrote:
> > >> >> >> >
> > >> >> >> >> Hi all,
> > >> >> >> >>
> > >> >> >> >> What is ffmpeg's policy on "necro-bumping" old patches? Or
> more
> > >> >> >> >> precisely, what is the policy of requesting a patch to be
> merged where
> > >> >> >> >> all objections raised have been addressed via
> discussion/updated
> > >> >> >> >> patches, and which have not been merged in over 2 weeks due
> to unknown
> > >> >> >> >> reasons?
> > >> >> >> >>
> > >> >> >> >> In particular, there are 2 patchsets I would like to get
> merged:
> > >> >> >> >> 1. This I consider an important patch, simply because it
> solves a trac
> > >> >> >> >> ticket labelled as "important":
> https://trac.ffmpeg.org/ticket/2964,
> > >> >> >> >> which also contains links to the patches. A lot of
> discussion went on
> > >> >> >> >> around it on the mailing lists, and it is supported strongly
> by
> > >> >> >> >> Nicolas and me. Michael seemed initially hesitant but later
> became
> > >> >> >> >> convinced of (at least one of the set's) utility, and one of
> the
> > >> >> >> >> patches was applied. The only objection I recall was from
> Hendrik,
> > >> >> >> >> which was addressed by Nicolas in a follow-up.
> > >> >> >> >>
> > >> >> >> >> 2. This I consider much more trivial, but in this case there
> are no
> > >> >> >> >> remaining objections. However, I still consider it important
> enough
> > >> >> >> >> for a request to re-examine, as I am doing here. The
> patchset is more
> > >> >> >> >> recent,
> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-August/177794.html
> > >> >> >> >> and
> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-September/178700.html.
> > >> >> >> >
> > >> >> >> >
> > >> >> >> > Trivial patches can be merged after 24-48 hours if there's no
> objections
> > >> >> >> > outstanding. For more elaborate patches, poke anyone for
> review if you feel
> > >> >> >> > it would be helpful.
> > >> >> >> >
> > >> >> >> > In both cases, having push access yourself will hurry this
> along (i.e. you
> > >> >> >> > really should get push access), but in this case I will push
> later today.
> > >> >> >> > If you don't want push access, poke one of us on IRC to do
> the push for
> > >> >> >> > you, or bump the original email with a "poke" or "ping".
> > >> >> >>
> > >> >> >> Thanks. Patches for 2) needs work, and I will be posting it
> soon.
> > >> >> >
> > >> >> >
> > >> >> >> Patch for 1) should be ok (it was reviewed by Nicolas, and
> Michael
> > >> >> >> seems ok with it like I mentioned).
> > >> >> >
> > >> >> > there where a few patches, iam not exactly sure which are left
> and
> > >> >> > what effects they have
> > >> >> > What i objected to and still object to is to cause the terminal
> to
> > >> >> > be messed up in the most common default configuration in linux
> > >> >> > (that is with bash) when ffmpeg crashes (either in a
> naturally/naively
> > >> >> > written script or from the command line)
> > >> >> > Iam not sure th last patches still cause this or not
> > >> >>
> > >> >> Don't know what you exactly mean by a naturally/naively written
> > >> >> script, and an example would be very helpful. I can say for
> certainty
> > >> >
> > >> > naturally written == a script that does not contain a explicit
> > >> > workaround for this issue
> > >> > One would only add a workaround once one has been hit by a bug or
> knows
> > >> > about it and searched and found a workaround.
> > >> > The users time and convenience matters, we should not inconveience
> > >> > many people by this.
> > >> >
> > >> >
> > >> >> that fate and its associated scripts will not suffer from this
> issue,
> > >> >> simply because of the -nostdin flag that was added in one of the
> > >> >> patches (which has been applied). The question that remains is
> whether
> > >> >> to apply:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-July/176481.html
> > >> >> or not.
> > >> >>
> > >> >
> > >> >> The point Nicolas raised is that no matter what we do, there exist
> > >> >> quite reasonable ffmpeg invocations that can mess up the terminal
> > >> >> (patch or no patch).
> > >> >
> > >> > this is true, but reasonable is not the same as "used alot in
> practice"
> > >> > something can be reasonable and used alot or it can be reasonable
> and
> > >> > used very rarely
> > >> >
> > >> >
> > >> >> All the patch does is remove a heuristic that
> > >> >> does not even work in all cases (which is impossible unless shell
> > >> >> configuration is changed with just 1-2 lines). It also improves
> > >> >> usability by fixing #2964.
> > >> >
> > >> > no heuristic works in all cases, thats the nature of heuristics
> > >> > for example our file format probing is all heuristics, not every
> > >> > file starting with "RIFFAVI" is a avi file  ...
> > >> >
> > >> > if the heuristic doesnt work well enough, then it should be improved
> > >> > if that is possible
> > >> >
> > >> > if the bug is belived to be else where (shell, config whatever)
> > >> > it should be fixed there so that future default setups do not need
> > >> > this heuristic anymore
> > >> > The heuristic should be kept as long as the alternative
> inconveiences
> > >> > more people
> > >> > I assume that the heuristic inconveiences fewer than the alternative
> > >> > of nothing. Is there a reason to belive that this is not the case ?
> > >>
> > >> Well, assuming the issue of Ticket 2964 is not regarded as terribly
> > >> incovenient...
> > >> If you consider a 1-2 line addition to bashrc or zshrc as incovenient
> > >> for a user (which we can document in wiki/faq, or wherever you want),
> > >> and you feel the inconvenience of the issue of 2964 is less than that
> > >> of some messed up terminals (non fate scripts), then yes, I agree with
> > >> you. I am not convinced by this myself, so I think we should see what
> > >> others think about this
> > >
> > >> (I think a formal poll is ridiculous for
> > >> something this tiny).
> > >
> > > what we really would need to know is how many users are affected by
> > > each of the 2 options, the choice should be made so as to
> > > inconveience fewer.
> >
> > Such a thing is impossible to determine with any reasonable assurance
> > - I presume FFmpeg is used in a variety of places. Who knows, distro
> > maintainers in a few years may finally wake up when enough users
> > complain to them about terminal trashing (from a variety of programs,
> > e.g cat on binaries, gpg without --armor, etc) that they add the 1-2
> > lines to the bashrc/zshrc that they ship.
> >
> > >
> > > also, the goal should be a better heuristic if at all possible
> > > before weighting which inconveience is less
> >
> > I do not necessarily agree with this: a "better heuristic" is
> > essentially a hack built on an existing hack - sometimes "less is
> > more", and the simple, clean solution (and which is also "correct",
> > since terminal cleanup is shell's responsibility and NOT FFmpeg's; try
> > e.g cat /bin/ls) is the current patch.
>
> "cat /bin/ls" will send the binary ls to the terminal and some of it
> can get interpreted as various control chars. This behaviour is
> correct and not a bug. One may very well intentionally do a
> cat file_with_control_chars
> to affect the state intentionally
>
> If an application OTOH crashes then something should cleanup after it
> that can be the shell


The shell wouldn't know the difference. It has to be an atexit() mechanism
in the application cleaning up after itself. This isn't specific to the
shell state changing - it applies more generally imo.

Ronald


More information about the ffmpeg-devel mailing list