joncampbell123 / dosbox-x

DOSBox-X fork of the DOSBox project
GNU General Public License v2.0
2.77k stars 381 forks source link

Media keys crash Dosbox-X #2976

Open zomgugoff opened 3 years ago

zomgugoff commented 3 years ago

Code of Conduct & Contributing Guidelines

Have you checked that no other similar bug report(s) already exists?

What operating system(s) this bug have occurred on?

Arch Linux

What version(s) of DOSBox-X have this bug?

0.83.18 SDL1

Describe the bug

The media keys on the laptop I'm using cause Dosbox-X to crash

Expected behavior

I would assume they would be ignored as they are not present in the mapper. Although, having them be mappable for miscellaneous functions would be nice.

Steps to reproduce the behaviour

Run dosbox-x Press any media keys (Play/Stop/Next/Previous)

Used configuration

The crash occurs with or without a config file

Emulator log

dosbox-x: sdl_mapper.cpp:1459: virtual bool CKeyBindGroup::CheckEvent(SDL_Event*): Assertion `key < keys' failed.
Aborted (core dumped)

Additional context

I wish it were as simple as not pressing those keys, but the keyboard layout of this laptop (Asus ROG Strix Scar 15) has them right up against the right side of the normal keys, so I accidentally hit them when trying to press backspace/enter/backslash/ or the arrow keys.

maron2000 commented 3 years ago

My keyboard don't have media keys, and I don't have a test environment running Arch Linux, But you can try ignoring the keys pressed by changing some code in sdl_mapper.cpp

//line 1459 of sdl_mapper.cpp
// assert(key < keys);  // current code
if (key < keys) key = SDLK_UNKNOWN; // test code
grapeli commented 3 years ago

I confirm. When XF86MonBrightnessDown is pressed, DOSBox-X (SDL1) crashes. backtrace.log

Allofich commented 3 years ago

Does the change suggested by maron2000 prevent the crash?

grapeli commented 3 years ago

@Allofich No. backtrace.log

maron2000 commented 3 years ago

@grapeli Thanks for testing. I didn't imagine that deleting the assertion and force the entered key as unknown leads to segmentation fault.

Bitterman94 commented 2 years ago

bug

Hello there! The issue with media keys is not fixed in v0.83.22 release. When using "videodriver=directx" and pressing "FN" key DOSBOX-X crashes with the error attached above.

Please release a hotfix.

If leaving videodriver field empty like "videodriver= " then it does not crash, but without "directx" driver the game performs considerably slow.

maron2000 commented 2 years ago

This assertion doesn't fail on my PC regardless of videodriver settings (default or directx). I tried Volume up & down, Mute, and Brightness up & down which are the media keys available on my keyboard.

I noticed that there was a typo in the previously proposed test fix. You may want to try it. line 1470 of sdl_mapper.cpp (as of commit 5d4fc4c37426f5135b321b45d76720fc8c2f1d29)

// assert(key < keys); // current code
if (key >= keys) key = SDLK_UNKNOWN; // test code

The following screenshot is taken using videodriver=directx and output=ddraw on 0.83.22 release (without the above mentioned patch). mediakey

Bitterman94 commented 2 years ago

I cannot compile & change code, please release a test build so I could check.

maron2000 commented 2 years ago

I usually don't distribute my private builds, but since I can't reproduce the issue myself, this is strictly for test purpose. Plz test it at your own risk. Release.zip Windows SDL1 x64 VS

The changes are as below.

        //assert(key < keys);
        if(key >= keys) {
            key = SDLK_UNKNOWN; // a test to avoid assertion failure (key < keys)
            LOG_MSG("assertion failed: key: %x [keysym.sym:%x keysym.scancode: %x]", key, event->key.keysym.sym, event->key.keysym.scancode);
        }
Bitterman94 commented 2 years ago

It worked nicely on my laptop with Win10. An Issue is gone.

Could you please do another (additional) build because I won't be able to launch your test build on my Windows XP SP3 x32 (32 bit) machine.

I guess you should do exactly the same but for x32, like this:

"Windows SDL1 x32 VS"

Bitterman94 commented 2 years ago

maron2000, could you please compose a build according to the info I mentioned above?

maron2000 commented 2 years ago

@Bitterman94 Nice to hear good news from you. The build should print logs instead of crashing when you press media keys, so plz share logs for various media keys here. Debug -> Show logging text or Show logging console

And, so here's the 32bit version. As same as x64 version, plz try it at your own risk. Release_win32.zip

P.S. I understand that you want to try ASAP, but I can't always respond promptly since I have to go to work.

Bitterman94 commented 2 years ago

DOSBox-X.log

Big thanks, it works correctly as x32 build should and all the code changes are in place!

An issue with FN Key & Media buttons is gone.

Please find attached a DOSBox-X.log

maron2000 commented 2 years ago

@Bitterman94 Thanks for testing, and sharing your log. According to the log there should be a way to eliminate the root cause, but still the fix is enough to avoid crashes. I'll send a PR later on.