[FFmpeg-devel] [PATCH] Add libavsequencer.

[Måns Rullgård]

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

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

Exactly.

> 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