[FFmpeg-devel] [RFC/PATCH 5/8] tidsp: add init/close

Reimar Döffinger Reimar.Doeffinger
Tue Sep 7 21:49:05 CEST 2010


On Tue, Sep 07, 2010 at 10:33:08PM +0300, Felipe Contreras wrote:
> On Tue, Sep 7, 2010 at 10:17 PM, Reimar D?ffinger
> <Reimar.Doeffinger at gmx.de> wrote:
> > A separate patch should contain exactly that amount of changes
> > that you can reason about and argue for independently, not
> > more and not less.
> 
> If possible yes, but not if it breaks the logic independence.
> 
> At least in git and linux I've seen many people mentioning "this would
> help further patches".

Usually those are design changes, where the "help further patches" is just
a shorthand for "our design was not future-safe enough, so we need to
replace it"

> By your logic a skeleton patch wouldn't make sense, because you cannot
> argue that such patch makes sense on its own.

Well, I think it not only makes no sense but is likely to be confusing
to anyone doing regression testing. Having intentionally broken code
in SVN is not so great.

> If you have a different policy, then I can merge all the changes in
> tidsp_mpeg4.c into one patch, although I think that would be slightly
> harder to review.

I haven't looked at it enough to know which is more reviewable, so
I don't intend to request such a change.
I consider it highly likely there's a better way to split it, however
I guess there are more important, higher-level changes necessary anyway
(most importantly how to handle the shared code), so no point quibbling
about that now.



More information about the ffmpeg-devel mailing list