clangen / musikcube

a cross-platform, terminal-based music player, audio engine, metadata indexer, and server in c++
https://musikcube.com
BSD 3-Clause "New" or "Revised" License
4.17k stars 294 forks source link

ncurses artifacts when playing (new) track #368

Closed the-eater closed 3 years ago

the-eater commented 4 years ago

When changing tracks sometimes the ncurses interface seems to break result into this

adgadgadg

The easiest way to reproduce I have found now is by spamming enter (start track) and then navigating to a different album

but this also happens regularly when just playing an album

clangen commented 4 years ago

Interesting, I haven't seen this before. What platform are you running on, and which terminal emulator? From the sound of it, it can happen for any track, not just some specific ones? My knee-jerk reaction is that is due to some sort of corrupted metadata, but it sounds like that's not the case?

the-eater commented 4 years ago

VoidLinux / x86_64 happens on both my laptop and desktop, happens in kitty as well as st (kitty however freezes for like a minute when it happens)

seems to be just randomly happening, no specific track, e.g. I can start spamming play track on any track and this issue will happen. and while just playing albums it will just happen at random moments while the player starts the new track.

All metadata as far as I am aware is fine and valid. (downloaded flacs from Bandcamp)

since it happens occasionally but not consistently I'm guessing it is some race condition around ncurses. (which sounds like a pain to debug)

lmk if you want me to run any weird patches or debug options.

On 30 August 2020 22:44:12 CEST, casey langen notifications@github.com wrote:

Interesting, I haven't seen this before. What platform are you running on, and which terminal emulator? From the sound of it, it can happen for any track, not just some specific ones? My knee-jerk reaction is that is due to some sort of corrupted metadata, but it sounds like that's not the case?

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/clangen/musikcube/issues/368#issuecomment-683468459

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

sorjgdojbodjhdo commented 3 years ago

Hey,

I'm running through the same issue here on Debian using the .deb release. In my case, it happens every time I use the software. My library is composed of a lot of CD rips in FLAC. What could be the root of the issue ? Thanks.

clangen commented 3 years ago

I'm still unable to reproduce the issue on any operating system I use -- all seems fine on Windows, macOS, Debian, Ubuntu and Arch. I get the feeling we're doing some sort of UI update from a background thread, which is corrupting things.

@sorjgdojbodjhdo do you have any specific steps to follow, different from other comments in this thread?

I'd really like to get this fixed for the next release, if possible. :)

clangen commented 3 years ago

Another question -- which terminal emulator(s) are you guys using? Maybe the bug is specific to certain ones.

the-eater commented 3 years ago

So far the terminal emulators that this happens on are:

on vte I get to see 0;musikcube so might be around the title setting part??

clangen commented 3 years ago

on vte I get to see 0;musikcube so might be around the title setting part??

Ooh, good idea. That feature can be disabled in advanced settings by setting the DisableWindowTitleUpdates value to true. If you disable that does the problem go away?

the-eater commented 3 years ago

it's already turned off

the-eater commented 3 years ago

https://github.com/clangen/musikcube/blob/c93d66a4ba995220bac52058d20d43eb24205807/src/musikcube/app/layout/MainLayout.cpp#L336 ha, so I actually can't turn it off huh :D

the-eater commented 3 years ago

considering how title setting is done by circumventing ncurses, it wouldn't surprise me if that is the exact reason

https://github.com/clangen/musikcube/blob/c93d66a4ba995220bac52058d20d43eb24205807/src/musikcube/cursespp/App.cpp#L280

give me a sec to build musikcube without title setting :)

the-eater commented 3 years ago

So here's a few factors that I think play a role in this:

EDIT:

btw quick returning from window update works, so I sent a PR :)

clangen commented 3 years ago

Ooh, awesome, nice detective work -- merged the PR! :)

Hmm, let me try to see if I can figure out why the TUI freezes when playing files over sshfs. In general, all I/O in musikcube happens asynchronously, otherwise remote playback would be really janky. There must be a code path for files mounted on the local filesystem that tries to do something synchronously.

clangen commented 3 years ago

I also pushed a change to disable title updates on Unix platforms by default for all new users, since this seems to be inherently problematic. Windows builds are unaffected because they don't write a magic escape sequence to stdout to do the updating, and instead use standard Windows APIs. https://github.com/clangen/musikcube/commit/5b1f07c953251c048d760d2f5345b662db2db6b4

clangen commented 3 years ago

@the-eater I spent some time trying to repro the TUI freezing you mentioned but was unable to do so. I setup an sshfs mount over a super slow network but it didn't seem to freeze the UI. What output driver are you using? PulseAudio?

the-eater commented 3 years ago

using PulseAudio yes. something occurred to me, might it be lastfm?

sorjgdojbodjhdo commented 3 years ago

I'm using the terminal included on Pop os. i installed the app with the deb release, added some folder to play music from and had the artifacts. I could include a photo but this is exactly as shown in the initial post.

clangen commented 3 years ago

@the-eater hmm, good thought -- but I gave lastfm a try across a couple platforms here and everything seems to be asynchronous. A couple things that may help me track it down:

clangen commented 3 years ago

@sorjgdojbodjhdo thanks! @the-eater figured out the bug and it'll be fixed in the next release!

clangen commented 3 years ago

Added in 0.96.3. https://github.com/clangen/musikcube/releases/tag/0.96.3

@sorjgdojbodjhdo make sure you turn off the title updating feature as described in this thread; new users will have it disabled automatically, but existing users will have to turn it off manually.

sorjgdojbodjhdo commented 3 years ago

Screenshot from 2020-12-29 14-08-14

Using the latest release and having disabled the title updating feature, since yesterday, I'm still seeing similar artifacts. I've attached a screenshot. I'm using the built in terminal on Pop OS and installed musikcube using the focal deb release. If I resize the window, the artifacts go away if that helps understanding the issue.

clangen commented 3 years ago

@sorjgdojbodjhdo this looks like a slightly different issue -- it looks like musikcube is misinterpreting some unicode escape sequence in the tags and treating double wide characters as singles. Would it be possible for you to send me a copy of the highlighted track, and perhaps the one below it as well? It will be hard for me to debug this problem without access to the metadata.

sorjgdojbodjhdo commented 3 years ago

Sure, I will do this. Do you have your email address somewhere or are you willing to provide me with a throwaway one for me to send you a download link?

clangen commented 3 years ago

Sure, you can just send them to my personal email address -- casey [dot] langen [at] gmail [dot] com. I'll take a look at the metadata to see if I can fix the issue, then delete the files.

If sending the files themselves makes you uncomfortable that's no big deal -- you can use something like Mp3tag on Windows or Kid3 on other platforms to export the tag to a text document, then send those instead. I should be able to just take the metadata and apply it to a different audio file and see if I can reproduce the issue that way.

sorjgdojbodjhdo commented 3 years ago

@clangen I sent them. It sure was quick to only extract the metadata.