[FFmpeg-soc] [soc] libavsequencer [PATCH 03/08] Order list public API header file.

[Sebastian Vater]

Vitor Sessak a écrit :
> On 08/13/2010 10:19 PM, Sebastian Vater wrote:
>> Vitor Sessak a écrit :
>>> On 08/07/2010 09:44 PM, Sebastian Vater wrote:
>>>
>>> [...]
>>>
>>
>
>>>> +    /** Next order list data pointer if seeking forward one
>>>> frame.  */
>>>> +    struct AVSequencerOrderData *next_pos;
>>>> +
>>>> +    /** Previous order list data pointer if seeking backward one
>>>> +       frame.  */
>>>> +    struct AVSequencerOrderData *prev_pos;
>>>> +
>>>> +    /** Number of row to jump to if forward seeking one frame.  */
>>>> +    uint16_t next_row;
>>>> +
>>>> +    /** Number of row to jump to if backward seeking one frame.  */
>>>> +    uint16_t prev_row;
>>>> +
>>>> +    /** Beginning row for this track. If this is a track
>>>> synchronization
>>>> +       point, the high byte is interpreted as the first track number
>>>> +       to be synchronized with and the low byte as the second track
>>>> +       number or for all channels when all 4 tracks are 0.  */
>>>> +    uint16_t first_row;
>>>> +
>>>> +    /** Last row for this track. If this is a track synchronization
>>>> +       point, the high byte is interpreted as the third track number
>>>> +       to be synchronized with and the low byte as the fourth track
>>>> +       number or for all channels when all 4 tracks are 0.
>>>> +       If last row is set to 65535 in non synchronization mode,
>>>> +       the last row is always taken from AVSequencerTrack.  */
>>>> +    uint16_t last_row;
>>>
>>>> +    /** Order list data playback flags. Some sequencers feature
>>>> +       special end markers or even different playback routes for
>>>> +       different playback modules (one-shot and repeat mode
>>>> +       playback), mark synchronization points or temporary
>>>> +       change volume), which has to be taken care specially
>>>> +       in the internal playback engine.  */
>>>> +    uint8_t flags;
>>>
>>> enum...
>>
>> Hmm, what you mean with this, it is already enum, right?
>>
>>>
>>>> +    /** Relative note transpose for full track. Allows playing
>>>> several
>>>> +       tracks some half-tones up/down.  */
>>>> +    int8_t transpose;
>>>
>>> Comment unclear. Is this a flag? What does it means if transpose ==
>>> -23?
>>
>> It means to adjust -23 halftones to each track data instrument / note
>> pair encountered. Let's say you have in the pattern:
>> C-5 01 .. ...
>
> So it is really missing in the comment to say this quantity is
> expressed in halftone units.

Ehm, this is in the comments: Allows playing several tracks some
half-tones up/down.

This actually also is the definition of the word transpose in music.

Anyway I fixed the comment a bit.

>
>> Since -24 would be 2 octaves back (i.e. C-3), -23 would playback as:
>> C#3 01 .. ...
>>
>> It's simply globally applied to the whole track which is used by this
>> order entry.
>>
>> This is, e.g. a feature heavily used by FutureComposer.
>>
>>>
>>>> +    /** Instrument transpose. All instruments will be relatively
>>>> +       mapped to this if this is non-zero.  */
>>>> +    int16_t instr_transpose;
>>>
>>> Why an int16_t for 0/1?
>>
>> This is not a 0/1,
>
> Again, missing unit. Maybe something like "Number of half-tones to
> transpose the instrument." or something like that.

This is number of instrument value to add relatively. Fixed!

>
>> this means adding instr_transpose to the instrument
>> value.
>> Considering the example above:
>> C-5 01 .. ...
>>
>> and instr_transpose = 4 then it will play:
>> C-5 05 .. ...
>>
>> i.e. instrument number 5 instead of 1.
>>
>> Again, this is heavily used by FutureComposer.
>>
>>>
>>>> +    /** Tempo change or zero to skip tempo change. A tempo value of
>>>> +       zero would be zero, since that would mean literally execute
>>>> +       unlimited rows and tracks in just one tick.  */
>>>> +    uint16_t tempo;
>>>
>>>> +    /** Played nesting level (GoSub command maximum nesting
>>>> depth).  */
>>>> +    uint16_t played;
>>>> +
>>>> +    /** Track volume (this overrides settings in AVSequencerTrack).
>>>> +       To enable this, the flag AVSEQ_ORDER_DATA_FLAG_SET_VOLUME
>>>> +       must be set in flags. This allows have a basic default track
>>>> +       volume by still allowing to override the track volume in case
>>>> +       the track is used multiple times, e.g. for creating
>>>> echoes.  */
>>>> +    uint8_t volume;
>>>> +
>>>> +    /** Track sub-volume. This is basically track volume
>>>> +       divided by 256, but the sub-volume doesn't account
>>>> +       into actual mixer output (this overrides
>>>> AVSequencerTrack).  */
>>>> +    uint8_t sub_volume;
>>>> +} AVSequencerOrderData;
>>>> +
>>>> +/** AVSequencerOrderList->flags bitfield.  */
>>>> +enum AVSequencerOrderListFlags {
>>>
>>>> +    AVSEQ_ORDER_LIST_FLAG_CHANNEL_SURROUND  = 0x01, ///<  Initial
>>>> channel surround instead of stereo panning
>>>> +    AVSEQ_ORDER_LIST_FLAG_TRACK_SURROUND    = 0x02, ///<  Initial
>>>> track surround instead of stereo panning
>>>> +    AVSEQ_ORDER_LIST_FLAG_MUTED             = 0x04, ///<  Initial
>>>> muted channel
>>>> +};
>>>
>>> Did you mean "initially"?
>>
>> More like:
>> Use initial ...
>
> indeed, "initial" is better.
>

Fixed.

-- 

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