[FFmpeg-devel] [PATCH] Add libavsequencer.

Måns Rullgård mans
Wed Aug 25 13:33:02 CEST 2010


Sebastian Vater <cdgs.basty at googlemail.com> writes:

> M?ns Rullg?rd a ?crit :
>>   
>>>>> Also, some parts already have been reviewed (see ffmpeg-soc mainly for
>>>>> that), so why you are concerning me, that I'm dodging reviews?
>>>>>         
>>>> A while ago you proposed that, to expedite the process, only a part of
>>>> a huge blob of code be reviewed, and the remainder accepted on faith.
>>>> I call that dodging review.
>>>>       
>>> I think you mean the part where we were talking about the mixer stuff.
>>>
>>> The point here is that the mixing functions are to 99% very similar,
>>> i.e. 16-bit mixing in differs in int16_t vs. int8_t for 8-bit mixing and
>>> shifting. It's a total of 23k lines which is pretty large.
>>>
>>> My idea here was, that just 2-3 of these functions are reviewed, then we
>>> discuss of creating #define macros for these and therefore get rid of
>>> 20k lines, making the whole stuff much more easilier to maintain and
>>> also to review, saving us all huge amounts of time.
>>>
>>> So you simply misunderstood that part, sorry for this!
>>>     
>>
>> I would appreciate if you dropped that belittling attitude.  You speak
>> as though to a small child who believes he has seen an elf.  Most
>> people, little children included, take offence at being addressed in
>> such a manner.
>>   
>
> Could you clarify here, what you exactly mean? I really don't understand.
> Maybe with some examples showing the issue on a strong emphasis and what
> you expected and got? Thanks in advance!

"you simply misunderstood that", "you must have missed that", "it is
quite complex, really", etc, etc.

>>>>> Regarding the minimal patch...this is just what you have in front of
>>>>> you! A minimal patch, which does simply nothing else than adding the
>>>>> library...or did you mean sth. else here?
>>>>>         
>>>> That patch does NOTHING useful.  Ronald asked you repeatedly to submit
>>>> a full set of patches allowing SOMETHING to be done with the simplest
>>>> file format.  You continue to (rather clumsily) attempt to evade this
>>>> request.
>>>>       
>>> I already told them that I want to do first a port which has the same
>>> compatibility as TuComposer regarding playback,
>>>     
>>
>> Not in my svn.
>>   
>
> What's the problem you see here with this approach? After all, this is
> quite complex, and small changes can have side effects (sometimes not
> easy to track by audible means).

It is a huge piece of unfinished code.  It doesn't belong there.

> As said, I want to use that as a base of a automatic regression test only.

You can run tests on your own copy.

>>> which is quite close now. The point is, once we review it on a
>>> larger scale here and also the whole stuff, we'll probably do some
>>> API changes, internal code logic changes.
>>
>> If API changes are expected, it clearly is not ready for mainline.
>
> I just changed the API again, yesterday. There were only smaller
> changes, but ...

See...

>>> Anyway, why do you think that I'm against reviewing it? After all, the
>>> review process makes a) code quality better b) improves readibility
>>> (documentation and code) and c) fixes bugs. So again, why do you think I
>>> try to avoid this?
>>
>> How else should I interpret your refusal to send reviewable patches?
>>   
>
> Some files still change too frequently and too much in a short period,
> i.e. sending patches has for some parts a pretty high probability that
> once review is done (which can quite a bit of time) the code does not
> match the current progress anymore.

Then clearly, the code is not yet ready for review.

> I am still thinking of the best way to post the patches actually, i.e.
> which parts should it all contain and which not? I got suggested to
> remove anything from the patches which is currently unreachable, etc.
> Figuring that out, is despite the large structures not always that easy.

Maybe once it's actually ready to be reviewed this will be easier.

>>>>> Since all that stuff is large,
>>>>>         
>>>> That is a problem.  You, however, appear to be quite proud of the
>>>> bloat you have produced.  That is not a good sign.
>>>>       
>>> Why do you think it being bloat?
>>
>> An excess of 20k lines for a simple audio mixer is bloat, whichever
>> way you look at it.
>>   
>
> Yes, solving that with define's as you showed me with the IFF-ILBM
> decoder optimization lut's, would be way nicer, for the mixer I strongly
> was preferring speed over size.

Even with separate code for 8-bit, 16-bit, 32-bit, and float, 20k
lines seems very much.  20k is plenty to write an entire operating
system.

-- 
M?ns Rullg?rd
mans at mansr.com



More information about the ffmpeg-devel mailing list