[MPlayer-dev-eng] [PATCH] Audio support for AVISynth

[Reimar Döffinger]

Hello,
On Wed, Jul 26, 2006 at 12:13:37AM +0200, Gianluigi Tiesi wrote:
> On Tue, Jul 25, 2006 at 03:45:43PM +0200, Reimar Doeffinger wrote:
> > Thanks. Though I have to ask: do actually read the patches you submit,
> > because I still see some obvious cosmetics. If you don't, do it and
> > maybe you understand what my problem with cosmetic chnages is.
> I prefer to look at overall resulting file instead of looking
> at the patch.

Which makes it really hard for those reviewing. And also, how do you
check for regressions when you don't look at the changes?

> > You only added a few spaces here, please, if you move it to a seperate
> > patch everyone will have a much easier time to understand what this
> > patch actually does, what it changes and why it didn't work before (or
> > in other words, learn from it).
> as Alexander replied its another block of code, also this is an example
> where looking at the patch is not better than looking at the resulting
> file.

The idea of separating cosmetics is that both work well.

> > Why were these two lines moved up? Is this necessary for the audio
> > support to work or is this a seperate issue or just "cosmetic"?
> I cannot spend hours to force minimize diff :P

Which leaves us with the options of spending that time ourselves or just
saying "let's just apply whatever patch comes around and let it silently
rot whenever the original author disappears since no-one else can handle
the code".

> > > -    // TODO release_clip?
> > > +
> > [...]
> > > +            if (AVS->clip) AVS->avs_release_clip(AVS->clip);
> > 
> > seems unrelated to me -> seperate patch?

Applied separately. Please redo your patch against latest SVN.

> > > -    //mp_msg(MSGT_DEMUX, MSGL_V, "AVS: seek rel_seek_secs = %f - flags = %x\n", rel_seek_secs, flags);
> > > -    
> > 
> > unrelated and cosmetic?
> hmm yes but are you sure you want 240 patches 1 line each one?

Since you ask: yes. Though I've actually given up hope on getting what I
want and split some parts into separate patches myself and applied them.

Greetings,
Reimar Döffinger