Open undisbeliever opened 4 weeks ago
Starting on my feedback in this section with these...
For EVOL, EFB and FIR, I recommend clamping to the signed limits (-128/127), like how volume and panning are also clamped to their respective limits.
For translating stereo to mono (I recommend storing the original values, as https://github.com/undisbeliever/terrific-audio-driver/issues/15#issuecomment-2445249018 has some commentary on the inversion settings and the amount of adjustments may vary)...
The next part I'll tackle feedback-wise is the EDL, since the EDL (and the ESA by proxy, though I think you're keeping that fixed based off of the largest EDL setting) is the most notorious one in terms of safely handling echo buffers: although it's quickly relocatable, the buffer size does not change until it has completed its previous loop. In addition, there are indeed some side effects from changing the EDL:
Now, if you want the EDL to go through a single loop of clearing the entire buffer (which also requires that EON and EFB DSP registers be temporarily zeroed out) before sounding off after changing, then the best move I can see is to let a timer do the job, keep echo writes on, and temporarily zero EVOL to prevent unintended sounds from coming out (perhaps this clear and wait process can also be optional, though at their own risk sound-wise).
The amount of time to wait should be 2old EDL
+ 2new EDL
timer 1 ticks (assuming timer 1 is set to 64). This is because the buffer has to finish first, then it has to execute one loop before restorations can be done with a new, clean buffer.
So, if you want to implement a waiting period after changing the EDL in order to allow for the buffer to clear before sounding off on it, then...
old EDL
+ 2new EDL
timer 1 ticks (assuming timer 1 is 64) for the buffer to clear. Echo writes will remain enabled during this time, as otherwise the buffer won't be cleared.The EDL can also be changed without a waiting period, though the side effect notes remain.
For adjust_fir_tap
, I'm leaning towards making the limit part optional, and this can be done within a single opcode slot. tap
is pre-shifted by 4, and perhaps the highest bit of the first byte can indicate whether there is a limit defined. Specifically, lttt0000
where...
l
indicates whether a limit is defined.ttt
is the FIR tap ID to adjust.Defining limits in other adjust-based opcodes will require a second slot be taken.
I'm thinking for the MML syntax it could be \ftap+ <0-7>,<0..127>,<0..127>
and \ftap- <0-7>,<0..128>,<-1..-128>
(the +/- sign is put at the beginning of the command to indicate that the FIR tap is being adjusted, not set, to avoid character conflicts with the sign, and the third parameter is the limit parameter, which I mentioned above as possibly being optional.
Yesterday I tried to code golf the set_stereo_echo_volume
and adjust_stereo_echo_volume
instructions and ran into issues dealing with signs.
Mainly issues with figuring out what to do when one channel's volume changes sign but the channel doesn't change (ie left goes negative, right stays positive). It gets even more complex when I add surround to the mono/stereo setting.
Here's a new idea. Echo volume is unsigned and would have an invert flag in a similar manner as the voice channels.
Advantages:
Disadvantages:
What do you think of these new instructions?
set_echo_invert_flags lr00000m
If stereo mode is surround, echo invert flags are unchanged.
If stereo mode is mono or stereo, the echo invert flags will be set to 0xff
if m-flag set, or 0
if m-flag clear.
set_echo_volume u8
Arguments masked by 0x7f
, limiting argument range to 0-127.
set_stereo_echo_volume u8 left, u8 right
0x7f
, limiting argument range to 0-127.(left+right)/2
)adjust_stereo_echo_volume i8 left, i8 right
adjust_echo_volume (left+right)/2
adjust_stereo_echo_volume i8 adjust
Whenever the echo volume changes the new EVOLL
and EVOLR
will be calculated using the l/r echo-invert-flags and echo-volume-left/echo-volume-right variables
Another alternative is to use echo volume + pan in the same format as voice channels. The main downside of echo pan is that the maximum centred EVOL is 64. I'm not sure if how big of a downside this would be (RainAmbienceWThunder.mml has an EchoVolume of 127).
I'm thinking for the MML syntax it could be \ftap+ <0-7>,<0..127>,<0..127> and \ftap- <0-7>,<0..128>,<-1..-128> That sounds like a good idea.
I can do the same with the echo volume and echo feedback, so \efb+ +12
assemble to adjust_echo_feedback +12
.
For adjust_fir_tap, I'm leaning towards making the limit part optional
Also a good idea and it can be easily done with shift and a branch.
I think the code would use less space if I make the opcode parameter order tap+limit-flag [, limit], offset
.
Thinking about this a bit more - using two different opcodes, adjust_fir_tap tap, offset
& adjust_fir_tap_limit limit, tap, offset
, would use the least amount of code space and I have 3 unused opcodes at the moment.
Either way, the bytecode assembly would still be adjust_fir_tap tap, offset [, limit]
.
I'll choose the first option for the echo volume: unsigned values with invert flags, since it also solves a problem with how to deal with the mono situation.
The second plus sign in \efb+ +12
would be redundant in this case, though then again, \efb- 12
would assemble to adjust_echo_feedback -12
. Basically, I conjured up the idea on the fly to avoid signed value conflicts (and it is indeed space sensitive, as without the space the end result is a conflict with signage on the numbers). It's also why when I noted the \ftap
adjustment syntax, I actually noted unsigned values for most of the parameters (for the limit parameters, I simply matched the sign to your specifications), because the sign is defined at the beginning of the MML syntax in question.
The second plus sign in
\efb+ +12
would be redundant in this case
That's a good point and something I hadn't considered. The parameter should be unsigned in this case.
The tokenizer requires a space after a \
MML command. So that would take care of separating \efb
, \efb+
and \efb-
commands.
I added the second plus because the MML and bytecode syntax for signed numbers (portamento velocity, adjust volume, adjust pan) requires a + or - sign (excluding song headers).
I can always relax the sign requirements for the set EFB and set FIR commands and allow \efb 12
to compile to set_echo_feedback 12
to match the song header.
Updated 16 November 2024
I think the MML echo commands should use the
\
syntax. This makes it more obvious to the song-author they are global commands and non-standard MML.[ ]
set_echo_invert_flags lr00000m
0xff
if m-flag set, or 0 if m-flag clear.EVOL_L
andEVOL_R
S-DSP registers, based on the echo volume and echo invert flags.\ei 0
,\ei
,\ei M
\ei L
\ei R
,\ei LR
in the same format as channel invert.[ ]
set_echo_volume u8
argument & 0x7f
set_stereo_echo_volume arg, arg
.EVOL_L
andEVOL_R
S-DSP registers, based on the echo invert flags.\evol <0..127>
[ ]
set_stereo_echo_volume u8, u8
0x7f
EVOL_L
andEVOL_R
S-DSP registers, based on the echo invert flags.\evol <left 0..127>,<right 0..127>
[ ]
adjust_echo_volume i8 adjust
EVOL_L
andEVOL_R
S-DSP registers, based on the echo invert flags.\evol +<1..127>
\evol -<1..128>
[ ]
adjust_stereo_echo_volume i8 left, i8 right
adjust_echo_volume (left+right)/2
EVOL_L
andEVOL_R
S-DSP registers, based on the echo invert flags.\evol <+-left>,<+-right>
[ ]
set_echo_feedback i8
EFB
S-DSP register\efb <-128..127>
[ ]
adjust_echo_feedback i8 amount
EFB
S-DSP register (with clamping).\efb+ <1..127>
&\efb- <1..128>
[ ]
set_fir i8 i8 i8 i8 i8 i8 i8 i8
\fir { i8 i8 i8 i8 i8 i8 i8 i8 }
[ ]
set_fir_tap u8 tap, i8 value
\ftap <0-7>,<-128..127>
tap
is pre-shifted by 4[ ]
adjust_fir_tap u8 tap, i8 adjust [, i8 limit]
\ftap+ <0-7>,<1..127>[,<limit>]
&\ftap- <0-7>,<1..128>[,<limit>]
tap
is pre-shifted by 4adjust_fir_tap
andadjust_fir_tap_limit
.[ ]
set_echo_delay u8
EDL
S-DSP register\edl <delay_in_milliseconds>
#MaxEchoLength
to the song headerESA
register would be based off#MaxEchoLength
. This ensuresESA
is only set once in song initialisation.set_echo_delay
cannot be >#MaxEchoLength
#MaxEchoLength
if it not declared in the header?set_echo_delay
.