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

[Sebastian Vater]

Vitor Sessak a écrit :
> On 08/07/2010 09:44 PM, Sebastian Vater wrote:
>
> [...]
>
>> Hi I have excellent news!
>>
>> libavsequencer now flawlessly integrates into FFmpeg, just check out my
>> latest git. Please do a git pull --rebase, Stefano had problems without
>> using it.
>>
>> Here are the order.[ch] part of the BSS to review.
>>
>> This version compiles perfectly.
>
>>
>> diff --git a/libavsequencer/order.h b/libavsequencer/order.h
>> new file mode 100644
>> index 0000000..70bcf0b
>> --- /dev/null
>> +++ b/libavsequencer/order.h
>> @@ -0,0 +1,193 @@
>
> [...]
>
>> +typedef struct AVSequencerOrderData {
>> +    /**
>> +     * information on struct for av_log
>> +     * - set by avseq_alloc_context
>> +     */
>> +    const AVClass *av_class;
>> +
>> +    /** Metadata information: Original order entry file name, order
>> +     *  entry name, artist and comment.  */
>> +    AVMetadata *metadata;
>
>> +    /** AVSequencerTrack pointer to track which should be played.  */
>> +    AVSequencerTrack *track;
>
> If there is one and only one AVSequencerTrack per
> AVSequencerOrderData, why not having it directly as part of the
> struct, i.e.:
>
>     /** AVSequencerTrack pointer to track which should be played.  */
>     AVSequencerTrack track;
>
> ?
>

Would be possible though, but remember, the track stuff is supposed to
be editable and it's easier to just to replace the pointer instead of
copying the data all over it.

Also it requires way less memory usage, please note that number of order
data has to be multiplied by number of channels, i.e. for a 64 channel
sub-song we have 64 * (order_list->orders) entries, which can quite a
lot (a lot of modules have order_list->orders beyond 100).

>> +    /** 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 .. ...

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

>
>> +/**
>> + * Song order list structure, This structure is actually for one
>> channel
>> + * and therefore actually pointed as an array with size of number of
>> + * host channels.
>> + * 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 AVSequencerOrderList {
>> +    /**
>> +     * information on struct for av_log
>> +     * - set by avseq_alloc_context
>> +     */
>> +    const AVClass *av_class;
>> +
>> +    /** Metadata information: Original order list file name, order
>> +     *  list name, artist and comment.  */
>> +    AVMetadata *metadata;
>> +
>> +    /** Array (of size orders) of pointers containing all order list
>> +       data used by this channel.  */
>> +    AVSequencerOrderData **order_data;
>> +
>> +    /** Number of order list data used for this channel.  */
>> +    uint16_t orders;
>> +
>> +    /** Number of order list data entries to use for this channel.  */
>> +    uint16_t length;
>> +
>> +    /** Repeat start order list data number for this channel.  */
>> +    uint16_t rep_start;
>> +
>> +    /** Volume level for this channel (defaults to 255).  */
>> +    uint8_t volume;
>> +
>> +    /** Sub-volume level for this channel. This is basically channel
>> +       volume divided by 256, but the sub-volume doesn't account
>> +       into actual mixer output (defaults 0).  */
>> +    uint8_t sub_volume;
>> +
>> +    /** Stereo track panning level for this channel (defaults to
>> +       -128 = central stereo track panning).  */
>> +    int8_t track_panning;
>> +
>> +    /** Stereo track sub-panning level for this channel. This is
>> +       basically track panning divided by 256, but the sub-panning
>> +       doesn't account into actual mixer output (defaults 0).  */
>> +    uint8_t track_sub_panning;
>> +
>> +    /** Stereo panning level for this channel (defaults to
>> +       -128 = central stereo panning).  */
>> +    int8_t channel_panning;
>> +
>> +    /** Stereo sub-panning level for this channel. This is
>> +       basically channel panning divided by 256, but the sub-panning
>> +       doesn't account into actual mixer output (defaults 0).  */
>> +    uint8_t channel_sub_panning;
>
>> +    /** Order list playback flags. Some sequencers feature
>> +       surround panning or allow initial muting. which has to
>> +       be taken care specially in the internal playback engine.
>> +       Also sequencers differ in how they handle slides.  */
>> +    uint8_t flags;
>
> enum....

Again what you mean with enum here?

-- 

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