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

[Vitor Sessak]

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;

?

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

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

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

> +    /** 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"?

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

> +
> +#endif /* AVSEQUENCER_ORDER_H */