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 {
>
> [...]
>
>>>>>>> if (instrument == previous_instrument) {...}
>>>>>>
>>>>>> Please note that instrument number 0 also differs whether you
>>>>>> play the
>>>>>> track in once mode (pattern play mode) and the instrument number was
>>>>>> initialized in the previous order. All trackers don't output
>>>>>> sound in
>>>>>> that case, changing this would make this part incompatible.
>>>>>
>>>>> That's another good point, so I'm fine with instrument==0 meaning
>>>>> last
>>>>> instrument. But if the behavior of setting instrument==0 is different
>>>>> of the behavior of setting instrument==last_instrument, it should be
>>>>> documented somewhere.
>>>>
>>>>>> TrackData is the octave, note, effect and effect data for each row,
>>>>>> Track itself defines the structure containing it, i.e. number of
>>>>>> rows,
>>>>>> maybe track name, etc.
>>>>>
>>>>> Maybe AVSequencerRowData then?
>>>>
>>>> Nice name, will think on it!
>
> So?AVSequencerTrackRow? Just to make the hierarchy clearer...agree on this? > >>>>>> >>>>>> This would require extending using 0x80-0xFF command effect which >>>>>> would >>>>>> conflict with synth sound handling (see there at the synth sound >>>>>> instructions). >>>>> >>>>> Ok, let me pose the question differently. Suppose we have an effect >>>>> foo. In MOD, effect foo is command 0x66 while in XM it is command >>>>> 0x55. What is the value of AVSEQ_TRACK_EFFECT_CMD_FOO? Or such thing >>>>> never happens? >>>> >>>> This happens very frequently, esp. between MOD/XM<=> S3M/IT. >>>> But this is part of the demuxer to convert the origins... >>> >>> Sure, but then doing as you do: >>> >>> /** 0x66 - Do foo to stuff */ >>> #define AVSEQ_TRACK_EFFECT_CMD_FOO 0x66 >>> >>> is confusing and misleading in comparison with >>> >>> /** Do foo to stuff */ >>> #define AVSEQ_TRACK_EFFECT_CMD_FOO 0x66 > > So? Sorry, don't understand here, what you mean. > > [...] > >> >> diff --git a/libavsequencer/track.h b/libavsequencer/track.h >> new file mode 100755 >> index 0000000..cf131af >> --- /dev/null >> +++ b/libavsequencer/track.h >> @@ -0,0 +1,1636 @@ >> +/* >> + * AVSequencer track and pattern management >> + * Copyright (c) 2010 Sebastian Vater <[email protected]> >> + * >> + * 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_TRACK_H >> +#define AVSEQUENCER_TRACK_H >> + >> +#include "libavutil/log.h" >> +#include "libavformat/avformat.h" >> + > > >> +/** >> + * 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. > >> + /** Effect command data word. */ >> + uint16_t data; >> +} AVSequencerTrackEffect; >> + >> +/** AVSequencerTrackEffect->note values. */ >> +enum AVSequencerTrackDataNote { >> + /** --- */ >> + AVSEQ_TRACK_DATA_NOTE_NONE = 0, >> + >> + /** C-n */ >> + AVSEQ_TRACK_DATA_NOTE_C = 1, >> + >> + /** C#n = Dbn */ >> + AVSEQ_TRACK_DATA_NOTE_C_SHARP = 2, >> + >> + /** D-n */ >> + AVSEQ_TRACK_DATA_NOTE_D = 3, >> + >> + /** D#n = Ebn */ >> + AVSEQ_TRACK_DATA_NOTE_D_SHARP = 4, >> + >> + /** E-n */ >> + AVSEQ_TRACK_DATA_NOTE_E = 5, >> + >> + /** F-n */ >> + AVSEQ_TRACK_DATA_NOTE_F = 6, >> + >> + /** F#n = Gbn */ >> + AVSEQ_TRACK_DATA_NOTE_F_SHARP = 7, >> + >> + /** G-n */ >> + AVSEQ_TRACK_DATA_NOTE_G = 8, >> + >> + /** G#n = Abn */ >> + AVSEQ_TRACK_DATA_NOTE_G_SHARP = 9, >> + >> + /** A-n */ >> + AVSEQ_TRACK_DATA_NOTE_A = 10, >> + >> + /** A#n = Bbn */ >> + AVSEQ_TRACK_DATA_NOTE_A_SHARP = 11, >> + >> + /** B-n = H-n */ >> + AVSEQ_TRACK_DATA_NOTE_B = 12, >> + >> + /** ^^^ = note kill */ >> + AVSEQ_TRACK_DATA_NOTE_KILL = -1, >> + >> + /** ^^- = note off */ >> + AVSEQ_TRACK_DATA_NOTE_OFF = -2, >> + >> + /** === = keyoff note */ >> + AVSEQ_TRACK_DATA_NOTE_KEYOFF = -3, >> + >> + /** -|- = hold delay */ >> + AVSEQ_TRACK_DATA_NOTE_HOLD_DELAY = -4, >> + >> + /** -\- = note fade */ >> + AVSEQ_TRACK_DATA_NOTE_FADE = -5, >> + >> + /** END = pattern end marker */ >> + AVSEQ_TRACK_DATA_NOTE_END = -16, >> +}; > > Why hardcoding values? Why not just use: > > /** AVSequencerTrackEffect->note values. */ > enum AVSequencerTrackDataNote { > /** --- */ > AVSEQ_TRACK_DATA_NOTE_NONE, > > /** C-n */ > AVSEQ_TRACK_DATA_NOTE_C, > > /** C#n = Dbn */ > AVSEQ_TRACK_DATA_NOTE_C_SHARP, > > [...] > } > > And let the compiler assign whatever numbers it seems fit? Would require every demuxer/muxer to convert these values if they don't match, also current player engine checks for < 0 to see if it's a special note. Anway, for notes between 1 and 12 (C-5 and B-5, e.g.), this is a great idea! Will fix this! > >> + >> +/** >> + * Song track data 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 AVSequencerTrackData { >> + /** Array (of size effects) of pointers containing all effects >> + used by this track. */ >> + AVSequencerTrackEffect **effects_data; >> + >> + /** Number of effects used by this track. */ >> + uint16_t effects; >> + >> + /** Which octave the note is played upon, if note is a positive >> + value (defaults to 4). Allowed values are in the [0:9] range, >> + since the keyboard definition table has 120 entries (10 octave >> + range * 12 notes per octave), also expect trouble with most >> + trackers if values outside this range are used. */ >> + uint8_t octave; >> + > >> + /** Note to be played (see defines below, n is octave number). */ > > which defines? Oops, will fix this, too! > > >> + int8_t note; > > enum? Again what you mean with that? > >> + /** Number of instrument to be played or 0 to take previous >> one. */ >> + uint16_t instrument; >> +} AVSequencerTrackData; >> + >> +/** AVSequencerTrack->compat_flags bitfield. */ >> +enum AVSequencerTrackCompatFlags { >> + AVSEQ_TRACK_COMPAT_FLAG_SAMPLE_OFFSET = 0x01, ///< Sample >> offset beyond end of sample will be ignored (IT compatibility) >> + AVSEQ_TRACK_COMPAT_FLAG_TONE_PORTA = 0x02, ///< Share >> tone portamento memory with portamentoes and unlock tone portamento >> samples and adjusts frequency to: new_freq = freq * new_rate / >> old_rate. If an instrument number is given the envelope will be >> retriggered (IT compatibility). >> + AVSEQ_TRACK_COMPAT_FLAG_SLIDES = 0x04, ///< >> Portamentos of same type share the same memory (e.g. porta up/fine >> porta up) >> + AVSEQ_TRACK_COMPAT_FLAG_VOLUME_SLIDES = 0x08, ///< All >> except portamento slides share the same memory (e.g. volume/panning >> slides) >> + AVSEQ_TRACK_COMPAT_FLAG_OP_SLIDES = 0x10, ///< >> Oppositional portamento directions don't share the same memory (e.g. >> porta up and porta down) >> + AVSEQ_TRACK_COMPAT_FLAG_OP_VOLUME_SLIDES = 0x20, ///< >> Oppositional non-portamento slide directions don't share the same memory >> + AVSEQ_TRACK_COMPAT_FLAG_VOLUME_PITCH = 0x40, ///< Volume >> & pitch slides share same memory (S3M compatibility) >> +}; >> + >> +/** AVSequencerTrack->flags bitfield. */ >> +enum AVSequencerTrackFlags { >> + AVSEQ_TRACK_FLAG_USE_TIMING = 0x01, ///< Use track >> timing fields >> + AVSEQ_TRACK_FLAG_SPD_TIMING = 0x02, ///< SPD speed >> timing instead of BpM >> + AVSEQ_TRACK_FLAG_PANNING = 0x04, ///< Use track >> panning and sub-panning fields >> + AVSEQ_TRACK_FLAG_SURROUND = 0x08, ///< Use track >> surround panning >> + AVSEQ_TRACK_FLAG_REVERSE = 0x10, ///< Playback of >> track in backward direction >> +}; >> + >> +/** >> + * Song track structure. >> + * 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 AVSequencerTrack { >> + /** >> + * information on struct for av_log >> + * - set by avseq_alloc_context >> + */ >> + const AVClass *av_class; >> + >> + /** Metadata information: Original track file name, track >> + title, track message, track artist, track album, >> + track begin and finish date of composition and comment. */ >> + AVMetadata *metadata; >> + >> + /** AVSequencerTrackData pointer to array of track data. */ >> + AVSequencerTrackData *data; > > >> + /** Last valid row of track (defaults to 63 = 0x3F) which >> + is the default for most trackers (64 rows per pattern). */ >> + uint16_t last_row; > > Again, is this needed to play a track? Of course, how should the player (or the BSS) know at which row a track ends, it's like module->song_list vs. module->songs indicating total number of songs. > >> + /** Track global volume (defaults to 255 = no volume scaling). */ >> + uint8_t volume; >> + >> + /** Sub-volume level for this track. This is basically track >> + global volume divided by 256, but the sub-volume doesn't >> + account into actual mixer output (defaults 0). */ >> + uint8_t sub_volume; >> + >> + /** Stereo panning level for this track (defaults to >> + -128 = central stereo panning). */ >> + int8_t 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 sub_panning; >> + >> + /** Track transpose. Add this value to octave * 12 + note to >> + get the final octave / note played (defaults to 0). */ >> + int8_t transpose; > >> + /** Compatibility flags for playback. There are rare cases >> + where track handling can not be mapped into internal >> + playback engine and have to be handled specially. For >> + each sub-song which needs this, this will define new >> + flags which tag the player to handle it to that special >> + way. */ >> + uint8_t compat_flags; > > enum? Again, what you mean with this, enums are there, so... > >> + /** Track playback flags. Some sequencers feature >> + surround panning or allow initial reverse playback, >> + different timing methods which have all to be taken >> + care specially in the internal playback engine. */ >> + uint8_t flags; > > enum? And again once a more time... -- Best regards, :-) Basty/CDGS (-: _______________________________________________ FFmpeg-soc mailing list [email protected] https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
