[FFmpeg-devel] [PATCH] Enable pixdesc.c compilation

[Stefano Sabatini]

On date Wednesday 2009-11-18 01:56:53 +0100, Michael Niedermayer encoded:
> On Sun, Nov 15, 2009 at 06:07:17PM +0100, Stefano Sabatini wrote:
> > On date Sunday 2009-11-08 04:24:33 +0100, Michael Niedermayer encoded:
> > > On Sun, Nov 08, 2009 at 01:01:55AM +0100, Stefano Sabatini wrote:
> > [...]
> > > > One of the requirement I have for pixdescs is that it should make
> > > > possible to avoid particular cases handling in the generic functions
> > > > which deal with pixel formats.
> > > > 
> > > 
> > > > In other words the pixdesc definition should be expressive enough to
> > > > avoid case PIX_FMT_FOO: branches.
> > > > 
> > > > In order to prove that, my plan is to use it in imgconvert, as we
> > > > replace the old functions we should see if that's good enough, and
> > > > extend it in the meaningwhile.
> > > 
> > > hmm
> > > 
> > > > 
> > > > According to this heuristic, once all the PixFmtInfo functionality
> > > > will be replaced then the API should be ready for public exposure.
> > > 
> > > just keep in mind that i will reject every hunk that makes code more
> > > complex.
> > > (thats especially true for any attempt to replace a 2 line switch/case
> > >  by 2 pages of obfuscated code)
> > 
> > Please consider the attached example, the resulting code is 1/3 of the
> > original one, and doesn't need to be extended when new formats are
> > added.
> 
> the attached example adds 72 lines it removes none so its not less code.
> it is code duplication and more code.
> Please stop sending such messy patches, they waste both of our time.
> 
> yes sure i could do the extra work and fix your patch before trying to review
> it but a submited change should show the change, this does not. it does NOT
> contain the removed code because you "forgot" to remove it.
> As it is theres no way this could be reviewed let alone tested in libav*
> A change needs to show the old and new to be reviewable.
> 
> Before submiting more patches please make sure every individual patch
> conforms to:
> 1. NO unreachable code (except when it is seperat and has a clear API)
> 2. NO code duplication
> 3. NO function ever must behave different from how it is documented.
>    If you change a functions behavior change the docs as well.
> If you want to split a patch and by doing so violate one of above DONT split
> it only makes the patches worse

I'm aware of all that stuff, but the patch was intended as a proof of
concept rather than a patch for review, I explicitely named it an
"example", and it was intended just to show how it would look the
code, especially considering that you showed some doubts about the
overall idea, sorry if I didn't make this more clear.

I didn't removed the old code to make possible to run the test (which
tests the behavior of the new code against the old one).

Regards.
-- 
FFmpeg = Fundamentalist and Faithful Multimedia Prodigious Egregious Goblin