TriggerFx sometimes gets stuck on a note

Topics related to the API, programming discussions & questions, coding tips, bugs, etc. should go here.
User avatar
Artcfox
Posts: 1382
Joined: Thu Jun 04, 2015 5:35 pm
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by Artcfox »

uze6666 wrote:I've done the fix and while at it some refactoring, optimizations and cleanup. Though it seems I introduced a regression regarding the PCM channel some time ago. I will fix it before I commit everything back.
Awesome! Things are still running great for me with that fix in place, and I've even switched my focus back to working on gameplay.
uze6666 wrote:One other thing I noticed is the code generated in TriggerFx,TriggerCommon is far from efficient specially when there is pgm_read_byte macros. For some reasons the compiler will not use LPM with displacement to efficiently address structrure members. Well, on my version of GCC at least. If will test on 4.9.2. If I still get the same, I may just rewrite those function is assembler. Since they are called a lot of calls to those functions, that could add up to a lot of cpu savings, specially when music is playing. And it will assure the compiler wont reorganize things.
That's weird. I really don't want to sidetrack you, but if you do end up using a newer version of GCC then you may want to verify if using the __flash qualifier, rather than the PROGMEM macro, generates better code. As a bonus, you no longer need to use any special pgm_read_* macros when accessing the variables:

Code: Select all

     #ifdef __FLASH
     const __flash int var = 1;
     
     int read_var (void)
     {
         return var;
     }
     #else
     #include <avr/pgmspace.h> /* From AVR-LibC */
     
     const int var PROGMEM = 1;
     
     int read_var (void)
     {
         return (int) pgm_read_word (&var);
     }
     #endif /* __FLASH */
For more information see: https://gcc.gnu.org/onlinedocs/gcc/Name ... paces.html

Disclaimer: I have not used the __flash qualifier myself, I only know about its existence.

With that said, the thought of getting a lot of CPU cycles back from a full assembly implementation makes my mouth water. (I have not yet programmed an AVR in assembly before; my experience is limited to examining the generated code only.)
User avatar
D3thAdd3r
Posts: 3221
Joined: Wed Apr 29, 2009 10:00 am
Location: Minneapolis, United States

Re: TriggerFx sometimes gets stuck on a note

Post by D3thAdd3r »

Lots of interesting info on this. Now that it's pointed out, it makes total sense on the flag order, though I never considered that in all the times I've walked through that code. I can barely believe I never had this issue on a glitchy raycaster that always misses vsync, playing music, at 4 frames a second :lol: Glad you are back to gameplay now, good work guys.
User avatar
uze6666
Site Admin
Posts: 4801
Joined: Tue Aug 12, 2008 9:13 pm
Location: Montreal, Canada
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by uze6666 »

D3thAdd3r wrote:Lots of interesting info on this. Now that it's pointed out, it makes total sense on the flag order, though I never considered that in all the times I've walked through that code. I can barely believe I never had this issue on a glitchy raycaster that always misses vsync, playing music, at 4 frames a second :lol: Glad you are back to gameplay now, good work guys.
Yeah can't beleive it either, but I guess with music playing on all channels we never noticed since new notes comes all the time.
User avatar
uze6666
Site Admin
Posts: 4801
Joined: Tue Aug 12, 2008 9:13 pm
Location: Montreal, Canada
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by uze6666 »

I have fixed the regression on the PCM channel and pushed to the master branch all fixes, optimizations and cleanup regarding the sound glitch. Can you pull and confirm all works well?
User avatar
Artcfox
Posts: 1382
Joined: Thu Jun 04, 2015 5:35 pm
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by Artcfox »

uze6666 wrote:I have fixed the regression on the PCM channel and pushed to the master branch all fixes, optimizations and cleanup regarding the sound glitch. Can you pull and confirm all works well?
It looks like you pushed the PCM fix, but not the sound glitch fix.

Edit: I may have discovered another sound-related bug in PatchCommand11() and PatchCommand12().

I added -Wextra to my CFLAGS, and saw a bunch of -Wunused-parameter warnings in uzeboxSoundEngine.c, and as I was fixing them I realized that PatchCommand11() and PatchCommand12() assign to:

Code: Select all

tracks->slideSpeed
rather than:

Code: Select all

track->slideSpeed
which smells a bit fishy to me, since tracks is defined as an array:

Code: Select all

struct TrackStruct tracks[CHANNELS];
and I don't believe the current code is setting the slideSpeed of the correct track. I'm curious what others think.
User avatar
uze6666
Site Admin
Posts: 4801
Joined: Tue Aug 12, 2008 9:13 pm
Location: Montreal, Canada
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by uze6666 »

Are you using -Wall compile flag? These warning are normal in the sense that it is normal for these generic dereferenced function to not use certain of the parameters depending on the required functionality. Anyway, good catches. trackNo should not be there anymore and "tracks" should be track. I'll fix those.
It looks like you pushed the PCM fix, but not the sound glitch fix.
:? I'll check this out.
User avatar
Artcfox
Posts: 1382
Joined: Thu Jun 04, 2015 5:35 pm
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by Artcfox »

uze6666 wrote:Are you using -Wall compile flag?
Yes, I'm using:

Code: Select all

-Wall -Wextra -Winline
uze6666 wrote:These warning are normal in the sense that it is normal for these generic dereferenced function to not use certain of the parameters depending on the required functionality.
Right, but in the cases where the parameter is intentionally unused, that warning can be suppressed by putting:

Code: Select all

(void)unusedVariableName;
inside the body of the PatchCommand* functions, which was what I was in the process of doing when I caught those bugs.
uze6666 wrote:Anyway, good catches. trackNo should not be there anymore and "tracks" should be track. I'll fix those.
Thanks! I would also argue for adding:

Code: Select all

-Wextra
to the standard set of CFLAGS, since that's what helped me find those bugs.
User avatar
Artcfox
Posts: 1382
Joined: Thu Jun 04, 2015 5:35 pm
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by Artcfox »

uze6666 wrote:
Artcfox wrote:It looks like you pushed the PCM fix, but not the sound glitch fix.
:? I'll check this out.
Did you ever figure out what happened to those other changes?
User avatar
uze6666
Site Admin
Posts: 4801
Joined: Tue Aug 12, 2008 9:13 pm
Location: Montreal, Canada
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by uze6666 »

Artcfox wrote:
uze6666 wrote:
Artcfox wrote:It looks like you pushed the PCM fix, but not the sound glitch fix.
:? I'll check this out.
Did you ever figure out what happened to those other changes?
I really don't know. It seem I screwed up with git at some point and lost my changes between all those branch switching. No big deal in this case though, the fix is easy, :)
User avatar
uze6666
Site Admin
Posts: 4801
Joined: Tue Aug 12, 2008 9:13 pm
Location: Montreal, Canada
Contact:

Re: TriggerFx sometimes gets stuck on a note

Post by uze6666 »

Take two. Fixed and pushed. Let me know if all is fine.
Post Reply