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

Sebastian Vater cdgs.basty at googlemail.com
Sat Aug 7 21:44:31 CEST 2010


Vitor Sessak a écrit :
> On 07/13/2010 09:49 PM, Sebastian Vater wrote:
>> Vitor Sessak a écrit :
>>> On 07/11/2010 10:05 PM, Sebastian Vater wrote:
>>>
>>>> /*
>>>>   * AVSequencer order list and data management
>>>>   * Copyright (c) 2010 Sebastian
>>>> Vater<cdgs.basty-gM/Ye1E23mwN+BqQ9rBEUg at public.gmane.org>
>>>>   *
>>>>   * This file is part of FFmpeg.
>>>>   *
>>>>   * FFmpeg is free software; you can redistribute it and/or
>>>>   * modify it under the terms of the GNU Lesser General Public
>>>>   * License as published by the Free Software Foundation; either
>>>>   * version 2.1 of the License, or (at your option) any later version.
>>>>   *
>>>>   * FFmpeg is distributed in the hope that it will be useful,
>>>>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>>   * Lesser General Public License for more details.
>>>>   *
>>>>   * You should have received a copy of the GNU Lesser General Public
>>>>   * License along with FFmpeg; if not, write to the Free Software
>>>>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>>> 02110-1301 USA
>>>>   */
>>>>
>>>> #ifndef AVSEQUENCER_ORDER_H
>>>> #define AVSEQUENCER_ORDER_H
>>>>
>>>> #include "libavsequencer/track.h"
>>>>
>>>> /**
>>>>   * 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.
>>>
>>> Wouldn't it be better named AVSequencerChannelData?
>>
>> It is only channel based if you import from a tracker which separates
>> channels for each track, but most trackers don't do this, they have just
>> one entry for all the channels, since they only know synchronized
>> 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 {
>>>>      /** Array of pointers containing all order list data used by this
>>>>        channel.  */
>>>>      AVSequencerOrderData **order_data;
>>>
>>> Please replace in the comment "order list data" for a brief
>>> description of what it means. And BTW, is this an "order _list_ data"?
>>
>> Actually it is the data of the order list, order_data contains all order
>> list entries like AVSequencerTrackData contains all the rows of
>> AVSequencerTrack.
>
> Is it the list of the rows in chronological order? Is it the list of
> notes in alphabetical order? Also, if I understand well, it is an
> array of linked lists. Why?
>
>>>>      /** 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;
>>>> #define AVSEQ_ORDER_LIST_VOLUME 255
>>>>
>>>>      /** 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;
>>>> #define AVSEQ_ORDER_LIST_SUB_VOLUME 0
>>>>
>>>>      /** Stereo track panning level for this channel (defaults to
>>>>         -128 = central stereo track panning).  */
>>>>      int8_t track_panning;
>>>> #define AVSEQ_ORDER_LIST_TRACK_PAN  -128
>>>>
>>>>      /** 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;
>>>> #define AVSEQ_ORDER_LIST_TRACK_SUB_PAN  -128
>>>>
>>>>      /** Stereo panning level for this channel (defaults to
>>>>         -128 = central stereo panning).  */
>>>>      int8_t channel_panning;
>>>> #define AVSEQ_ORDER_LIST_PANNING    -128
>>>>
>>>>      /** 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;
>>>> #define AVSEQ_ORDER_LIST_SUB_PANNING    0
>>>
>>>>      /** Compatibility flags for playback. There are rare cases
>>>>         where order handling can not be mapped into internal
>>>>         playback engine and have to be handled specially. For
>>>>         each order list which needs this, this will define new
>>>>         flags which tag the player to handle it to that special
>>>>         way.  */
>>>>      uint8_t compat_flags;
>>>
>>> I suppose this is not unused ATM?
>>
>> Fixed.
>>
>>>
>>>>      /** 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;
>>>> #define AVSEQ_ORDER_LIST_FLAG_CHANNEL_SURROUND  0x01 ///<  Initial
>>>> channel surround instead of stereo panning
>>>> #define AVSEQ_ORDER_LIST_FLAG_TRACK_SURROUND    0x02 ///<  Initial
>>>> track surround instead of stereo panning
>>>> #define AVSEQ_ORDER_LIST_FLAG_MUTED             0x04 ///<  Initial
>>>> muted channel
>>>>
>>>> } AVSequencerOrderList;
>>>>
>>>> /**
>>>>   * Song order list data structure, this contains actual order list
>>>> data.
>>>>   * 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 AVSequencerOrderData {
>>>>      /** AVSequencerTrack pointer to track which should be played.  */
>>>>      AVSequencerTrack *track;
>>>
>>>>      /** Next order list data pointer if seeking forward one
>>>> frame.  */
>>>>      AVSequencerOrderData *next_pos;
>>>>
>>>>      /** Previous order list data pointer if seeking backward one
>>>>         frame.  */
>>>>      AVSequencerOrderData *prev_pos;
>>>
>>> These do not look to belong to the BSS.
>>
>> They are used to allow the composer to define alternative order
>> sequences in case of forward and backward seeking. These alternatives
>> can pre-initialize note data in that case. This is a very rarely used
>> feature, but avoids the problem that instruments are missing if you skip
>> one channel.
>>
>> Example:
>> We have a track playing a new instrument at row 60. If the user skips
>> this track before arriving row 60, it will not be played at all, thus
>> causing silence. Now consider the normal next row, would change data
>> here or even has a complete empty row (but expecting to play a long
>> looped instrument), in that case it would confuse the actual output.
>> These pointers allow the composer to initialize that in case.
>
> Is this about an human user seeking the file he is playing or a
> tracker formats that specify seeking commands?
>
>>>>      /** Tempo change or zero to skip tempo change.  */
>>>>      uint16_t tempo;
>>>
>>> Cryptic comment. This is the timestamp where the tempo change? An
>>> index to a list a tempo changing structures? The instrument number
>>> that has a different tempo?
>>
>> Fixed.
>>
>>>
>>>>      /** Played nesting level (GoSub command maximum nesting
>>>> depth).  */
>>>>      uint16_t played;
>>>
>>> Again, BSS?
>>
>> Would require duplicating each track information in the player and
>> therefore add a lot of unnecessary complexity.
>
> So that means that the player _writes_ to this struct? This seems not
> optimal design in a modularity point of view. IMHO, everything that is
> feeded to the player should be read-only and the player should only
> write in his own context structure.
>
>>>>      /** Track volume (this overrides settings in AVSequencerTrack).
>>>>         To enable this, the flag AVSEQ_ORDER_DATA_FLAG_SET_VOLUME
>>>>         must be set in flags.  */
>>>>      uint8_t volume;
>>>
>>> This is ugly, why not making it a signed int and summing it to
>>> AVSequencerTrack.volume no matter what?
>>
>> This again would seriously break compatibility with almost any tracker.
>> Trackers either set volume by track data XOR by order data, never both.
>> It is not a volume transpose but a set volume command.
>
> What is the difference between a track volume and a order data volume?
> If they are the same, since only one or the other is specified per
> file, why not using a single struct field for it, without caring where
> in the file it is defined (and the module loader should know where to
> look)?
>

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.

-- 

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: order.c_20100807.patch
Type: text/x-patch
Size: 2425 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100807/9b6c9ded/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: order.h_20100807.patch
Type: text/x-patch
Size: 8099 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100807/9b6c9ded/attachment-0001.bin>


More information about the FFmpeg-soc mailing list