laamaa / m8c

Cross-platform M8 tracker headless client
Other
380 stars 81 forks source link

using the dirty flag in render causes poor framerate on Ondroid Go devices #78

Closed maks closed 2 years ago

maks commented 2 years ago

Per https://discord.com/channels/709264126240620591/879940212690669578/982689334090158120 (@aquarat on discord) discovered that using the dirtyflag in https://github.com/laamaa/m8c/blob/main/render.c#L221 seems to cause very low framerate on Odroid Go (Advance,Super) devices running Ubuntu 18.04 distro from the vendor (Hardkernel).

I'm not sure why this would be the case, but for the time being, @laamaa would you be open to having a config flag to allow disabling the use of that if, maybe even a C preprocessor macro for now? defaulted to off but so anyone building for these devices could set it rather than maintaining a fork.

bloopclick commented 2 years ago

If that's the case, it's probably because of limited resolution on SDL_Delay(). In what way is the performance bad? Does the program run slowly or is it consuming too many resources?

The discord link is not working for me.

maks commented 2 years ago

Ah sorry about, I've included a screenshot of the discord mesg below.

So it seems that the rendering is very infrequent. What you see when it happens is that movement through the UI appears to skip button presses, but whats actually happening is the screen is redrawing only every few moves (secs?) rather the 30-60fps.

image

bloopclick commented 2 years ago

Hm.. I can't match up the reported behavior with the reported fix.

The way it should work, the process waits a while on SDL_Delay(), then (in some order) sends serial commands to the M8, which in turn (eventually) generates redraw commands from the M8. There's probably another SDL_Delay() round, then those commands are processed out of the serial buffer, which are immediately drawn onto the window texture, and the dirty flag is set. When that is done, if the dirty flag is set, the render screen function copies the window texture to the window.

The way I understand this to work (I wrote it) and the bug report don't match up well. Not sure what's happening here.

maks commented 2 years ago

Hmmmm I'm not sure if I follow how the code works per your explanation. From a quick look, to me it looks like the dirty flag is there to short-circuit drawing if nothing has changed drawing wise and in each place where drawing occurs you set the dirty flag at the end of the function. So my guess is that somehow updates are coming in by one of the places where the dirty flag is set, hence no drawing happens for that incoming event hence its not visible on screen until after a couple more events when finally the dirty flag is finally set and a redraw occurs. That would match the behaviour I'm seeing, where by removing the if condition on the dirty flag, the redraws happen unconditionally and so always show updates to the screen state based on incoming events.

bloopclick commented 2 years ago

When serial parsing occurs, all of the draw data is parsed into draw commands, and these functions are called immediately. Every draw command function sets dirty flag, so if good serial data is processed, this should be set. Once this is all done, the last thing is a call to render_screen(), which only does something if one of the draw functions was called. As best I can tell, if a serial draw command was parsed, it will be drawn to the screen that round.

Once that completes, the process waits SDL_Delay(conf.idle_ms) and then goes around again to read the serial buffer and read everything that collected during the delay.

maks commented 2 years ago

@bloopclick yes, thats the behaviour I was trying to explain in my previous comment. I can't definitely figure out whats causing this though either. I wonder if its a race condition setting the dirty flag variable? I notice you don't have any guards around it, but m8c uses quite a few threads, is it possible that the code that calls the draw functions runs on a different thread to the main thread that calls render_screen() ?

bloopclick commented 2 years ago

The only threads in m8c presently are internal to SDL and dirty is only touched by the main thread.

maks commented 2 years ago

Hmm that makes me even more confused as to what could be causing the behaviour I and the other Odroid user are seeing. I definitely see the drawing get out of sync with the actual device state after a few dozen button presses or so. On the song screen I can easily have the highlight row marker and actual highlighted pattern "cell" be out of sync for example. And doing a screen reset gets things back to be correctly displayed, so there's definitely something that causes the dirtry flag to not always be set correctly, but admit I can't see how that can be happening looking at render.c

smootalicious commented 2 years ago

maks, I can't reproduce out of sync issues with any regularity, but I can just often enough to debug. I ended up setting the return to 0 after an invalid packet in command.c and that forced a reset/render. Strangely that didn't always clear the issue so I added a 5 tic delay in reset_display in write.c - I am not confident enough that this is a thing to check it in/PR yet. More testing to do.

RealJTG commented 2 years ago

I'm also trying to run m8c on ODROID Go Advanced (black edition) without X11. I compiled SDL2 for ODROID Go with kmsdrm support (https://github.com/AreaScout/SDL, as described here https://gist.github.com/thilong/462e2555952a338943ac4ecba5c66e24).

$ odroid@goadvance:~/code/m8c$ ./m8c
librga:RGA_GET_VERSION:4.00,4.000000
ctx=0x559c3b4e70,ctx->rgaFd=3
Rga built version:version:+2017-09-28 10:12:42
INFO: Reading config /home/odroid/.local/share/m8c/config.ini
INFO: Writing config file to /home/odroid/.local/share/m8c/config.ini
INFO: Looking for USB serial devices.
INFO: Found M8 in /dev/ttyACM0.
INFO: Opening port.
rk-debug mali-ver=rsx-p0-1.0
INFO: Looking for game controllers
INFO: Trying to open game controller database from /home/odroid/.local/share/m8c/game
controllerdb.txt
INFO: Trying to open game controller database from /home/odroid/code/m8c/gamecontroll
erdb.txt
INFO: Found 157 game controller mappings
INFO: Controller 1: GO-Advance Gamepad (rev 1.1)
INFO: Enabling and resetting M8 display
INFO: Reset display
DEBUG: New DRM FB (70): 320x480, stride 1280 from BO 0x559c464c70
DEBUG: New DRM FB (71): 320x480, stride 1280 from BO 0x559c46a7b0

It seems to work, but I have the same problem. If the screen is not updated for a long time and a certain number of keystrokes has been performed, m8c starts to skip further keystrokes. When a key is pressed either nothing is updated on the screen, or "jumps over", or drawn in inconsistent state, when one part of the screen is from this keystroke and another from the previous:

https://user-images.githubusercontent.com/930399/177774220-0f827582-e312-44fb-815e-69a967fa82f3.mp4

I tried to trace packets coming from Teensy and match them with the draw calls. Here's "good" and "bad" call trace.

Good:

INFO: send_msg_controller 32 // press <Down>
INFO: draw_rectangle_command
INFO:     └ draw_rectangle  (22, 161) 9x11 #000000
INFO: draw_rectangle_command
INFO:     └ draw_rectangle  (30, 161) 9x11 #000000
INFO: draw_rectangle_command
INFO:     └ draw_rectangle  (22, 171) 17x11 #00FC00
INFO: draw_character_command
INFO:     └ draw_character  (24, 163) -
INFO: draw_character_command
INFO:     └ draw_character  (32, 163) -
INFO: draw_character_command
INFO:     └ draw_character  (24, 173) -
INFO: draw_character_command
INFO:     └ draw_character  (32, 173) -
INFO: render_screen(): dirty = 1
INFO: draw_character_command
INFO:     └ draw_character  (8, 163) A
INFO: draw_character_command
INFO:     └ draw_character  (8, 173) B
INFO: render_screen(): dirty = 1
INFO: send_msg_controller 0 // release <Down>

Bad:

INFO: send_msg_controller 32 // press <Down>
INFO: draw_rectangle_command
INFO:     └ draw_rectangle  (22, 161) 9x11 #000000
INFO: draw_rectangle_command
INFO:     └ draw_rectangle  (30, 161) 9x11 #000000
INFO: draw_rectangle_command
INFO:     └ draw_rectangle  (22, 171) 17x11 #00FC00
INFO: draw_character_command
INFO:     └ draw_character  (24, 163) -
INFO: draw_character_command
INFO:     └ draw_character  (32, 163) -
INFO: draw_character_command
INFO:     └ draw_character  (24, 173) -
INFO: draw_character_command
INFO:     └ draw_character  (32, 173) -
INFO: render_screen(): dirty = 1
INFO: draw_character_command
INFO:     └ draw_character  (8, 163) A
INFO: draw_character_command
INFO:     └ draw_character  (8, 173) B
INFO: render_screen(): dirty = 1
INFO: send_msg_controller 0 // release <Down>

Plot twist: they are the same. The same commands came from device, the same stuff has been called with the same arguments, although we got different screen state. Further digging led me to these observations:

https://user-images.githubusercontent.com/930399/177774928-3017810b-b744-4d2d-a857-97a934db30a9.mp4

So it also may be something with input handling, IDK, maybe render_screen() execution time increases for a some reason and this slows down input and everything is going out of sync (but there's a single thread wtf). I also checked handle_sdl_events() function, down to SDL_PollEvent(), the Internets says it may skip something and should be called like this while (SDL_PollEvent(&event)) { // do stuff }, but that doesn't seem to help either (?)

And one more thing: in playback mode sound starts crackling after a short time, but if you continuously tap a key - everything is ok.


Upd: seems to work as intended with SDL2 built from https://github.com/christianhaitian/rk3326_core_builds, but sound crackling is still here :(

smootalicious commented 2 years ago

Re: the key skipping - I don't understand the inner workings of SDL, but maybe there's some kind of buffering happening? Perhaps forcing a render (or dirty flag 1) after a certain number of keystrokes will help? No idea with the crackling - it's maddening. Something makes me thing that may be SDL related too. If you remove the dirty flag code does the sound still crackle?

RealJTG commented 2 years ago

If you remove the dirty flag code does the sound still crackle?

Yep, and there are some trails in syslog:

Jul 07 18:05:30 goadvance kernel: retire_capture_urb: 110 callbacks suppressed
Jul 07 18:05:36 goadvance kernel: retire_capture_urb: 118 callbacks suppressed
Jul 07 18:05:42 goadvance kernel: retire_capture_urb: 87 callbacks suppressed
Jul 07 18:05:48 goadvance kernel: retire_capture_urb: 104 callbacks suppressed
Jul 07 18:05:54 goadvance kernel: retire_capture_urb: 110 callbacks suppressed
Jul 07 18:06:00 goadvance kernel: retire_capture_urb: 96 callbacks suppressed

When you spam buttons or rotate analog stick "suppressed callbacks" drops from 100 to 1-2. IDK, this probably should be adressed separately from this issue.

smootalicious commented 2 years ago

Do you happen to know what version of SDL you had before? And the new version solves the key skipping issue? I agree the crackle noise is a different issue, maybe buffer over/underruns on the usb bus or soundcard

maks commented 2 years ago

@RealJTG great debugging and research on this! And thanks for the pointer to the alternative SDL2 for the Odroid, I'll try that on my OGS soon too. Yes the audio crackling would be a completely separate issue as m8c doesn't do anything with sound - are you using alsaloop, pulseaudio loopback or Jack? Also another thing that might be worth looking into is that I saw on Discord headless thread someone mentioned that the Rust based client rm8 now has audio handling support via SDL so perhaps that could be ported to m8c to make it easier to manage audio on handhelds vs needing to use one of the audio system loopbacks?

maks commented 2 years ago

btw @RealJTG I forgot to ask are you running the Ubuntu 18.04 image from Hardkernel on your Odroid Go Advance? or another distro like ArkOS?

RealJTG commented 2 years ago

@maks stock Ubuntu 18.04 from Hardkernel

are you using alsaloop, pulseaudio loopback or Jack

Pulseaudio loopback

I also compiled rm8 against SDL2 from AreaScout and christianhaitian this way:

RUSTFLAGS="-C link-args=-Wl,-rpath,/usr/local/lib" cargo build

and it has the same sound issues, so it's almost certainly not a problem with the code, but with underlying subsystems.

maks commented 2 years ago

Just wanted to report back that I compiled and installed the christianhaitian fork of SDL2 and it did indeed fix the issue frame skipping issue that I reported here originally. Though strangely it seemed to start re-occurring, though intermittently after I had been using m8c for 1-15min. I'll experiment some more but already this is much better so I think at least in regards to that this issue could be closed as it seems to have been due to a bug in the other SDL2 fork I initially used. Thanks again to @RealJTG for figuring this out.