Vitor Sessak a écrit :
> On 08/13/2010 10:28 PM, Sebastian Vater wrote:
>> 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?
>
> yes.Fixed. > >>>>>>>> >>>>>>>> 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. > > I mean that if the choice of 0x66 for AVSEQ_TRACK_EFFECT_CMD_FOO is > merely a convention, it should not be reinforced in the comment. Fixed. > >>>> 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, > > Every format uses the same convention? For the positive notes, this is the case, as far as I know. For the negative special note effects it differs. But most use negative values, too, if I remember correctly, so you know it's a special note by just checking for < 0. Anyway, I fixed this by removing the 1 to 12 assignments and just set NONE to 0, the compiler automatically always adds one in a subsequent enum, right? This is required, since the player uses a lut for calculating the target frequency which is base_freq * (octave - 4) * 2^((note - 1)/12). -- Best regards, :-) Basty/CDGS (-: _______________________________________________ FFmpeg-soc mailing list [email protected] https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
