[FFmpeg-devel] [PATCH] Add libavsequencer.

[Vitor Sessak]

On 08/19/2010 11:44 PM, Sebastian Vater wrote:
> Vitor Sessak a ?crit :
>> On 08/19/2010 10:02 PM, Sebastian Vater wrote:
>>> Vitor Sessak a ?crit :
>>>> On 08/19/2010 12:04 AM, Ronald S. Bultje wrote:
>>>>> Hi,
>>>>>
>>>>> On Wed, Aug 18, 2010 at 5:58 PM, Stefano Sabatini
>>>>> <stefano.sabatini-lala at poste.it>    wrote:
>>>>>> So in the end I suggest this course of action: apply the lavseq
>>>>>> patch,
>>>>>> *or* if we want to add more stuff to it before to commit it, then
>>>>>> maybe we should focus on the BSS. Once that's ready, we can create
>>>>>> lavseq *and* immediatly commit the BSS definition.
>>>>>
>>>>> I don't want anything committed until I've seen all patches required
>>>>> for TCM playback posted to ffmpeg-devel so we can judge whether each
>>>>> component is actually necessary.
>>>>>
>>>>> I don't want duplicate code, I don't want overly complicated code and
>>>>> I don't want overdesigned code. I want exactly that code necessary for
>>>>> TCM playback. Nothing more. Those patches, we will review together,
>>>>> removing things that are unneeded/unused, removing code available
>>>>> already elsewhere in FFmpeg, and simplifying it to the best of our
>>>>> ability. Once that process is done, we can commit the individual
>>>>> parts.
>>>>
>>>> I've read the whole thread, and I really think this is a great idea.
>>>> Indeed, let's see a patch (as big as it needs to be, _but not bigger_)
>>>> to make playing TCM files work. That means that _every_ field in
>>>> _every_ structure should be:
>>>>
>>>> (a) written by some code in the patch
>>>> (b) read by some code in the patch
>>>>
>>>> Everything that is there for the future, for editors, for
>>>> visualization and for transcoding should be added _later_ (but maybe
>>>> it is a good idea to keep it in your tree). As I said, simpler patches
>>>> are approved faster, so let's make this patch as simple as possible
>>>> (and the simpler way by now is with no new library).
>>>
>>> To summarize that for the different parts of avseq:
>>> 1. The BSS:
>>> All fields are required (since TCM uses them all).
>>
>> Not really. There are a lot of "max_xxx" and "min_xxx" that could be
>> replaced by a constant without changing the player output.
>
> Nope these fields are in the IFF-TCM1 file, too. Remember, they're user
> editable and therefore IFF-TCM1 stores the values, too. Constants won't
> help here.
>
> IFF-TCM1 contains ALL fields which are now in the BSS, except the
> pointer stuff I moved already to player.h.

They contain this field, but if this field is just ignored and the code 
uses a constant, does playing fail? If the file contain data that is not 
strictly necessary for playing perfectly, this data should be ignored by 
the decoder in this first patch (ignored in the sense that it will not 
store it in any struct).

>>> 2. The player:
>>> Most fields are required (some commands are unimplemented right now, due
>>> to a lack of editor and properly and easy testing of it), I've marked
>>> today all (hope didn't miss anything out) these parts with a TODO in the
>>> effect execution path.
>>
>> Please do not add them in the patch (but you can keep them in your tree).
>
> Okey.
>
>>
>>> This mainly regards the hold / decay stuff which I added for the MED
>>> demuxer / decoder in TuComposer, which however, never got implemented
>>> because in the time I wanted to start it was the time where I already
>>> recognized that it made no sense to continue old TuComposer as
>>> Amiga-only.
>>>
>>> But MED demuxer / decoder will come into FFmpeg.
>>
>> One thing at a time.
>
> Okey^2.
>
>>
>>> 3) The mixer:
>>> Start with the low quality mixer only. The null mixer is not required
>>> here, since there's a) no seeking here and b) no hardware mixing support
>>> now.
>>>
>>> Also remove the multiple mixer support until we a) actually need null
>>> mixer or/and b) have a hardware mixer ready.
>>
>> Nice!
>
> Great to see we finally agree! :-)

:-)

>>> Besides this, I'll try to edit my wxTuComposer GUI in parallel, so we
>>> get the editor advanced as well as the FFmpeg stuff. The editor will
>>> allow us a) to test everything in the final state and b) to see if our
>>> current design really fits not only for transcoding / playback but also
>>> for editing. ;)
>>
>> I really recommend you do one thing at a time, ie, focus first on
>> getting the TCM player committed.
>
> No panic! Just wanted to show my plans regarding this, I just think an
> editor to able fully test the capabilities of avseq is important, too.
> Anyway, we're not on GSoC anymore now, all what we do now, is plain Open
> Source doing in free-time and just 4 fun (which btw was also during gsoc
> for me, was just hard to getting all study stuff together with ffmpeg
> sometimes, but study is at end now ;-)).

Of course, but note that we have been here for some time, and one thing 
I found out is that if you start too many projects at the same time they 
take waaaay too long to finish. And when things starting taking too long 
most people will lose your motivation and stop working on it.

>>> Please note, that wxTuComposer will be a independent project of FFmpeg,
>>> it will just be the first project from me that will use the new
>>> avsequencer API. ;-)
>>>
>>> It will probably not only a full editor, but more like a wxWidgets GUI
>>> class, which you can easily add to your own projects, i.e. create a
>>> AVSequencer tracker instance with just two lines of C++ code:
>>>
>>> wxTuComposer *tucomp = new wxTuComposer ();
>>> tucomp->Show ();
>>>
>>> The rest will be, as usual in wxWidgets just be event handling. ;-)
>>>
>>> Maybe there is some day, where you'll find this on wxCode page of
>>> wxWidgets. ;-)
>>> Maybe even I start this directly as wxCode project (just disregard that
>>> I would stick to SVN for that, don't really want to go back to SVN after
>>> working with git the last three months ;-))
>>>
>>>>
>>>>> Committing one part without knowing how the next part uses it is about
>>>>> the worst design nightmare possible.
>>>>
>>>> I agree also with this. The right way to review the BSS is seeing
>>>> where and how each field is used.
>>>
>>> Great!
>>>
>>> One last question here, since my initial patch regarding basic lavseq
>>> integration added a changelog entry (added libavsequencer). I would
>>> change that to sth. like: Added libavsequencer (formerly known as
>>> TuComposer).
>>>
>>> The point is here, that some people surely did heard of TuComposer in
>>> the last 5 years, and in case they search for it, they should find at
>>> least the FFmpeg page to know that's now named AVSequencer.
>>>
>>> I think a small notice regarding this in changelog or is enough. But
>>> maybe you have other opinions.
>>
>> Don't worry, when we get something functional in SVN we will mention
>> tuComposer in the changelog.
>
> Thanks very much! I will also have to update my website download links /
> informations to point to FFmpeg instead. ;-)
>
>>
>>> Just another point, Vitor. You surely remember our last discussions
>>> regarding BSS data structs in ffmpeg-soc. You know I was afraid that
>>> extending the fields (uint16_t to enum, int, etc.) was frightening me
>>> because of no more able to save that 100% to any file format.
>>>
>>> I just want to tell you here, that these afraids are actually pointless
>>> (I talked to Stefano already regarding this some days ago). Since
>>> IFF-TCM1, as I developed it, is easily extendable regarding that (how I
>>> should forget that, again 10l to me).
>>>
>>> To be short, just an example. Let's say we extend uint8_t flags of
>>> AVSequencerSong to enum AVSequencerSongFlags (which will be int then,
>>> thus adding usually more 24 bits).
>>>
>>> Storing these in IFF-TCM1 is NO problem! Just to some small magic like:
>>> first_8_bit_of_flags = get_byte(pb);
>>>
>>> Then read the remaining stuff as of old IFF-TCM1, then the first new one
>>> (let's say we extended to 32-bit):
>>> flags  = get_byte(pb)<<   8 | first_8_bit_of_flags;
>>> flags |= get_be16(pb)<<   16;
>>>
>>> That should do it. Since IFF-TCM1 also has a version / revision field in
>>> the MHDR iff tag, we should bump that up simply by one. ;-)
>>>
>>> Or to say it simple, forget all my concerns about this! No new format
>>> required, just some small extension to IFF-TCM1 demuxer / decoder and
>>> we're fine.
>>
>> Ok, but I don't really think that IFF-TCM1 should be a kind of DNA of
>> the BSS. They should be two different entities.
>
> Well it was DNA of TuComposer itself. It was the file format which
> should make sure, that everything ever will be added to it will also be
> immediately save/restorable. I'ld just like to keep that with
> AVSequencer, too.

It make sense, but trying to keep backward and forward compatibility 
while AVSequence is been intensely developed is a nightmare. I'd suggest 
you change the BSS with no regard of how TCM is and afterwards worry 
about saving TCM files (or else you will be stuck with all the internal 
structure of the original TuComposer with no improvement is possible). 
For the moment, it is better just to skip fields that the BSS do not 
contain now (if, obviously, they are not needed for correct playback).

> On the other hand, investing just another total new format, should be
> avoided as much as possible, esp. if we have a format already which does
> the job quite well. Maybe we could create, though, a mirror of it for
> little endian CPU's (RIFF-TCM1 or RIFF-AVS1), but well that are topics
> for the future.

Why? You know you cannot do (portably) things like

struct somestruc *s;
uint8_t *buf;
memcpy(buf, &s, sizeof(*s));
write_buffer_to_file(buf, sizeof(*s));

no?

> Since ami_stuff does port FFmpeg to Amiga platforms already, AVSequencer
> will become available for it, too, so I don't see any worries here (I
> would be a bit sad if it wouldn't be available for Amiga anymore, but
> thanks to ami_stuff this isn't the case).
>
> To close this post, I want to end with a lot of thanks:
> a) Stefano Sabatini for his excellent aid as a mentor, even in times he
> was busy at work
>
> b) You (Vitor Sessak) for causing me really brainstorming me during the
> BSS reviews

Thank you too for keeping your head cool with the feedback, no mater if 
you agreed with it or not.

-Vitor