[FFmpeg-soc] [soc] libavsequencer [PATCH 04/08] Track / pattern handling public API header file.

[Sebastian Vater]

Vitor Sessak a écrit :
> On 08/14/2010 12:01 AM, Sebastian Vater wrote:
>> Aurelien Jacobs a écrit :
>>> On Fri, Aug 13, 2010 at 10:28:55PM +0200, Sebastian Vater wrote:
>>>
>>>> Vitor Sessak a écrit :
>>>>
>>>>> On 08/07/2010 09:46 PM, Sebastian Vater wrote:
>>>>>
>>>>>> Vitor Sessak a écrit :
>>>>>>
>>>>>>> On 07/13/2010 10:57 PM, Sebastian Vater wrote:
>>>>>>>
>>>>>>>> Vitor Sessak a écrit :
>>>>>>>>
>>>>>>>>> On 07/13/2010 09:35 PM, Sebastian Vater wrote:
>>>>>>>>>
>>>>>>>>>> Vitor Sessak a écrit :
>>>>>>>>>>
>>>>>>>>>>> On 07/11/2010 10:07 PM, Sebastian Vater wrote:
>>>>>>>>>>>
>>>>>>>>>>>> typedef struct AVSequencerTrackData {
>>>>>>>>>>>>
>>>>> [...]
>>>>>
>>>>>
>>>>>> +/**
>>>>>> + * Song track effect structure, This structure is actually for
>>>>>> one row
>>>>>> + * and therefore actually pointed as an array with the amount of
>>>>>> + * rows of the whole track.
>>>>>> + * New fields can be added to the end with minor version bumps.
>>>>>> + * Removal, reordering and changes to existing fields require a
>>>>>> major
>>>>>> + * version bump.
>>>>>> + */
>>>>>> +typedef struct AVSequencerTrackEffect {
>>>>>> +    /** Effect command byte.  */
>>>>>> +    uint8_t command;
>>>>>>
>>>>> enum...
>>>>>
>>>> Again where's the problem? enums are all above.
>>>>
>>>
>>> The problem is that it seems you intended 'command' to be an enum, but
>>> you declared it as an uint8_t instead.
>>> You have to declare it as an enum to allow proper type checking (among
>>> other things).
>>> So for this one, what you want is:
>>>      enum AVSequencerTrackEffectCommand command;
>>>
>>> The same apply to all the enum remarks that you didn't understood.
>>>
>>
>> Ahh ok, thank you very much, I missed that point. But there are cases,
>> where the enum types should be really uint8_t and uint16_t...how to
>> solve that?
>
> In which case you can not replace any "int8_t" or "int16_t" of the BSS
> by a single "int" or even "int64_t"? Everything in the BSS should be
> internal stuff, so if you read 8 bits, you can store it internally in
> any integer type you wish that fits eight bits.

That's true. But let's consider effects, we have not only bit 7
allocated for the packer, but also the synth sound assembly stuff.
Having negative value for synth sound assembly code results the normal
effect if doing NOT on it, this effectively limits to 7-bit range.

Changing this, will not only require changing uint8_t to int but a lot
of other internal logic also. Anyway, I see that this is a good idea to
extend, but this has to be done carefully.

Just saying here, changing uint8_t to int isn't enough. We have also
find a solution for, how to handle compression of 16-/32-bit commands,
as well as introducing 16-/32-bit synth sound instructions (please note
here that negative synth sound instruction code (which is 8-bit) means
normal track effect).

Anyway, I have plans to extend synth sound instruction set, I have
already a idea pack to add a true stack to synth sound assembly language
code. But this is all in the future and should be done when the basic
stuff works, which means at least full playback of all TCM files.

>
> What happens if a new format shows up that decides to have a set of
> (2^64) possible commands? Will you have to change completely all your
> code just to change command from a uint8_t to a uint64_t?

Uhm, if this really happens, I become a monk and move to the next cave
and remain there for the rest of life.

No, just kidding!

All trackers I seen don't even use close to the 7-bit range, TuComposer
just becomes close to this, because it a) merges all commands and b) it
defines a lot of new on its own.

You have to know that practically all trackers represent effects to the
user either simply in the A-Z range (this being S3M/IT) which limits to
26 commands, or 0-15 (in hex 0-F represented by MOD/XM).

That's why I thought a full 7-bit range is more than enough. Although I
have practically allocated most of them for new features, there is still
enough place to addition even in this range.

If we have to add a new command, we have to change the playback engine
anyway, and thus break compatibility. In the same point we can extend
the command depth, if that's necessary.

The API functions itself are always at least uint32_t in the call
parameters, so there will not be external breakage about this.

If we ever need extension to 16-bit of commands or like that, we can do
this harmlessly since API calls are still 32-bit (ok if we need 64-bit
commands, but as said, then I'll become a monk ^^).

-- 

Best regards,
                   :-) Basty/CDGS (-: