[FFmpeg-devel] [PATCH] Add libavsequencer.

Måns Rullgård mans
Wed Aug 25 02:43:15 CEST 2010

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

> M?ns Rullg?rd a ?crit :
>> Sebastian Vater <cdgs.basty at googlemail.com> writes:
>>> M?ns Rullg?rd a ?crit :
>>>> Pretty please, do not commit _anything_ related to this until a full,
>>>> functional set of patches has been presented and reviewed by several
>>>> people.
>>> Hi Mans, I understand your point, but currently a) libavsequencer is
>>> disabled by default and b) is tagged as experimental.
>> That doesn't matter.  There is no point whatsoever in committing
>> something that may not survive even a cursory review once the rest is
>> presented.  In fact, doing so would be a VERY BAD THING.
> Yes, that's true! But avsequencer has been reviewed more than you might
> think at first (see ffmpeg-soc repository for our discussions regarding
> this).

That list is read by only a few people.  Something as intrusive as
this needs a broader review.

>>> If I remember correctly, libavfilter got a similar way to get into SVN.
>> Nothing from libavfilter was committed before it was functional,
>> albeit optional.  By the time the first parts were committed to main
>> svn, it had been under development for well over a year, and there had
>> been numerous lengthy discussions about various aspects relating to
>> it.  In contrast, libavsequencer (or whatever the name of the week is)
>> strikes me as a rush job being rammed into svn without semblance of
>> proper review.
> Hmm, certain parts are submitted here weeks ago,

Yes, a few fragments with no real functionality.  That is impossible
to review.

> there was some time to review them and to post complaints, but until
> now, no one, except Stephano regarding the nits commented on that,
> so maybe you missed it simply during that time?

I did not miss it.  I do, however, find it a waste of time to review
an incomplete set of patches.

>> >From what I saw looking at the tucomposer source earlier, I say it is
>> humanly impossible to turn it into ffmpeg-worthy code in the time
>> which has been available.  Your attempts at dodging reviews and
>> refusal to submit a minimal patch set reinforces this feeling of mine.
> Why do you look still at TuComposer's original source?

I looked at it a couple of months ago after you posted a link to it.
It was total garbage.

> Please look at my github regarding this, it CHANGED HEAVILY during
> that time! In all aspects: documentation, code and header stuff.

It is impossible to turn what I saw back then into anything even half
decent in the time which has passed since.

> 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.

> 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

>>> So I don't really see any big issues not commiting that part already.
>>> But maybe I'm missing sth. out?
>> You're missing the part where nobody has seen the minimal, functional
>> patch set which has been repeatedly requested.
> As said, some of the stuff already got reviewed in the ffmpeg-soc
> thread,

Then I will say again: this needs to be posted in full few, not only
on the soc list.

> there's still plenty of code left, of course.


> 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.

> but we should start at some point, and the way I got suggested it
> was to post minimal patches here which to get reviewed and
> integrated.

Reviewing the first step is impossible when the final goal is unknown.

> Please note, that I did a lot of private communication between Vitor,
> Stefano, Benjamin, Ronald and Michael. They also have a huge set of
> IFF-TCM1 files to see how the current player / demuxer / decoder works.

So why are you trying to push ahead against Ronald's request now?

> Anyway, please checkout my latest github and see for yourself.

That would be much easier if you provided a link.

> I'll have to submit the IFF-TCM1 test files to some central place,
> though.

That would be a start.

>> Again, I request that nothing be committed before a proper review
>> has taken place.
> Of course, everything should be tested fine and work properly before
> commited. So I prefer (as some dev's recommended me) to do that piece by
> piece, i.e. small and easy to review patches, if they work for it's own,
> they're ok.

Pieces cannot be reviewed in isolation.  The reviewer must have the
full picture in order to make an assessment of any single piece.

M?ns Rullg?rd
mans at mansr.com

More information about the ffmpeg-devel mailing list