On Tue, Sep 15, 2015 at 6:36 PM, Ronald S. Bultje <rsbul...@gmail.com> wrote: > Hi, > > On Tue, Sep 15, 2015 at 6:05 PM, Michael Niedermayer <michae...@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 <michae...@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 < >> michae...@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 < >> michae...@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 < >> rsbul...@gmail.com> wrote: >> > >> >> >> > Hi Ganesh, >> > >> >> >> > >> > >> >> >> > On Mon, Sep 14, 2015 at 10:27 PM, Ganesh Ajjanagadde < >> gajja...@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.
In fact, we already have signal handlers for essentially all catchable signals that do the tty stuff. This is part of the reason why I ask for a concrete, practical usage scenario (not artificial) where after applying the patch, terminal trashing occurs in scripts. > > Ronald > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel