-
-
Notifications
You must be signed in to change notification settings - Fork 300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: XMU Settings #1315
UI: XMU Settings #1315
Changes from 19 commits
96157f6
766409f
3f9b265
8e3296b
51fe070
21d467a
b1f9085
a9747a0
ac3b36d
485cb9c
bd1e773
bc80676
1e0595e
f397226
8423130
70881db
afe5146
5def7ad
f966217
3167c43
8889047
85b7071
32fe45a
a7f9905
e372b5a
498a44a
e74066e
97d37c0
93fc4a0
a8a9678
9d43196
51422f0
60c2362
4677199
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
pfiles = [ | ||
'controller_mask.png', | ||
'xmu_mask.png', | ||
'logo_sdf.png', | ||
'xemu_64x64.png', | ||
'abxy.ttf', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
#include "fatx.h" | ||
#include "qemu/bswap.h" | ||
|
||
#define FATX_SIGNATURE 0x58544146 | ||
|
||
// This is from libfatx | ||
#pragma pack(1) | ||
struct fatx_superblock { | ||
uint32_t signature; | ||
uint32_t volume_id; | ||
uint32_t sectors_per_cluster; | ||
uint32_t root_cluster; | ||
uint16_t unknown1; | ||
uint8_t padding[4078]; | ||
}; | ||
#pragma pack() | ||
|
||
bool create_fatx_image(const char* filename, unsigned int size) | ||
{ | ||
unsigned int empty_fat = cpu_to_le32(0xfffffff8); | ||
unsigned char zero = 0; | ||
|
||
FILE *fp = qemu_fopen(filename, "wb"); | ||
if (fp != NULL) | ||
{ | ||
struct fatx_superblock superblock; | ||
memset(&superblock, 0xff, sizeof(struct fatx_superblock)); | ||
|
||
superblock.signature = cpu_to_le32(FATX_SIGNATURE); | ||
superblock.sectors_per_cluster = cpu_to_le32(4); | ||
superblock.volume_id = (uint32_t)rand(); | ||
superblock.root_cluster = cpu_to_le32(1); | ||
superblock.unknown1 = 0; | ||
|
||
// Write the fatx superblock. | ||
fwrite(&superblock, sizeof(superblock), 1, fp); | ||
|
||
// Write the FAT | ||
fwrite(&empty_fat, sizeof(empty_fat), 1, fp); | ||
|
||
fseek(fp, size-sizeof(unsigned char), SEEK_SET); | ||
fwrite(&zero, sizeof(unsigned char), 1, fp); | ||
|
||
fflush(fp); | ||
fclose(fp); | ||
|
||
return true; | ||
} | ||
|
||
return false; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
#ifndef FATX_H | ||
#define FATX_H | ||
|
||
#include "qemu/osdep.h" | ||
|
||
#ifdef __cplusplus | ||
extern "C" { | ||
#endif | ||
|
||
bool create_fatx_image(const char* filename, unsigned int size); | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,8 @@ | |
#include "xemu-notifications.h" | ||
#include "xemu-settings.h" | ||
|
||
#include "sysemu/blockdev.h" | ||
|
||
// #define DEBUG_INPUT | ||
|
||
#ifdef DEBUG_INPUT | ||
|
@@ -94,8 +96,24 @@ static const char **port_index_to_settings_key_map[] = { | |
&g_config.input.bindings.port4, | ||
}; | ||
|
||
static int* peripheral_types_settings_map[4][2] = { | ||
{ &g_config.input.peripherals.port1.peripheral_type_0, &g_config.input.peripherals.port1.peripheral_type_1 }, | ||
{ &g_config.input.peripherals.port2.peripheral_type_0, &g_config.input.peripherals.port2.peripheral_type_1 }, | ||
{ &g_config.input.peripherals.port3.peripheral_type_0, &g_config.input.peripherals.port3.peripheral_type_1 }, | ||
{ &g_config.input.peripherals.port4.peripheral_type_0, &g_config.input.peripherals.port4.peripheral_type_1 } | ||
}; | ||
|
||
static const char **peripheral_params_settings_map[4][2] = { | ||
{ &g_config.input.peripherals.port1.peripheral_param_0, &g_config.input.peripherals.port1.peripheral_param_1 }, | ||
{ &g_config.input.peripherals.port2.peripheral_param_0, &g_config.input.peripherals.port2.peripheral_param_1 }, | ||
{ &g_config.input.peripherals.port3.peripheral_param_0, &g_config.input.peripherals.port3.peripheral_param_1 }, | ||
{ &g_config.input.peripherals.port4.peripheral_param_0, &g_config.input.peripherals.port4.peripheral_param_1 } | ||
}; | ||
|
||
static int sdl_kbd_scancode_map[25]; | ||
|
||
static const int port_map[4] = {3, 4, 1, 2}; | ||
|
||
void xemu_input_init(void) | ||
{ | ||
if (g_config.input.background_input_capture) { | ||
|
@@ -113,6 +131,10 @@ void xemu_input_init(void) | |
new_con->type = INPUT_DEVICE_SDL_KEYBOARD; | ||
new_con->name = "Keyboard"; | ||
new_con->bound = -1; | ||
new_con->peripheral_types[0] = PERIPHERAL_NONE; | ||
new_con->peripheral_types[1] = PERIPHERAL_NONE; | ||
new_con->peripherals[0] = NULL; | ||
new_con->peripherals[1] = NULL; | ||
|
||
sdl_kbd_scancode_map[0] = g_config.input.keyboard_controller_scancode_map.a; | ||
sdl_kbd_scancode_map[1] = g_config.input.keyboard_controller_scancode_map.b; | ||
|
@@ -178,6 +200,18 @@ int xemu_input_get_controller_default_bind_port(ControllerState *state, int star | |
return -1; | ||
} | ||
|
||
void xemu_save_peripheral_settings(int player_index, int peripheral_index, int peripheral_type, const char *peripheral_parameter) | ||
{ | ||
int *peripheral_type_ptr = peripheral_types_settings_map[player_index][peripheral_index]; | ||
const char **peripheral_param_ptr = peripheral_params_settings_map[player_index][peripheral_index]; | ||
|
||
assert(peripheral_type_ptr); | ||
assert(peripheral_param_ptr); | ||
|
||
*peripheral_type_ptr = peripheral_type; | ||
xemu_settings_set_string(peripheral_param_ptr, peripheral_parameter == NULL ? "" : peripheral_parameter); | ||
} | ||
|
||
void xemu_input_process_sdl_events(const SDL_Event *event) | ||
{ | ||
if (event->type == SDL_CONTROLLERDEVICEADDED) { | ||
|
@@ -202,6 +236,10 @@ void xemu_input_process_sdl_events(const SDL_Event *event) | |
new_con->sdl_joystick_id = SDL_JoystickInstanceID(new_con->sdl_joystick); | ||
new_con->sdl_joystick_guid = SDL_JoystickGetGUID(new_con->sdl_joystick); | ||
new_con->bound = -1; | ||
new_con->peripheral_types[0] = PERIPHERAL_NONE; | ||
new_con->peripheral_types[1] = PERIPHERAL_NONE; | ||
new_con->peripherals[0] = NULL; | ||
new_con->peripherals[1] = NULL; | ||
|
||
char guid_buf[35] = { 0 }; | ||
SDL_JoystickGetGUIDString(new_con->sdl_joystick_guid, guid_buf, sizeof(guid_buf)); | ||
|
@@ -255,6 +293,39 @@ void xemu_input_process_sdl_events(const SDL_Event *event) | |
char buf[128]; | ||
snprintf(buf, sizeof(buf), "Connected '%s' to port %d", new_con->name, port+1); | ||
xemu_queue_notification(buf); | ||
|
||
// Try to bind peripherals back to controller | ||
for(int i = 0; i < 2; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately binding here wont detect the keyboard binds. We can probably just refactor this code out and add a call in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I refactored the code into a new function called xemu_input_rebind_xmu(int port) but calling it from xemu_input_bind causes xemu to crash when switching between Keyboard and Xbox Controller. This is because unbinding the controller unplugs the XMUs, and trying to bind the XMUs again right after unplugging them causes an error because the file is still in use. Instead I call this function where the code was originally and also on line 180 just after it auto-binds the keyboard. |
||
enum peripheral_type peripheralType = (enum peripheral_type)(*peripheral_types_settings_map[port][i]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: style There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I renamed this variable to peripheral_type, though that might be confusing because it's the same as its type. |
||
|
||
// If peripheralType is out of range, change the settings for this controller and peripheral port to default | ||
if(peripheralType < PERIPHERAL_NONE || peripheralType >= PERIPHERAL_TYPE_COUNT) { | ||
xemu_save_peripheral_settings(port, i, PERIPHERAL_NONE, NULL); | ||
peripheralType = PERIPHERAL_NONE; | ||
} | ||
|
||
const char *param = *peripheral_params_settings_map[port][i]; | ||
|
||
if(peripheralType == PERIPHERAL_XMU) { | ||
if(param != NULL && strlen(param) > 0) { | ||
// This is an XMU and needs to be bound to this controller | ||
if(qemu_access(param, F_OK) == 0) { | ||
bound_controllers[port]->peripheral_types[i] = (enum peripheral_type)peripheralType; | ||
bound_controllers[port]->peripherals[i] = malloc(sizeof(XmuState)); | ||
memset(bound_controllers[port]->peripherals[i], 0, sizeof(XmuState)); | ||
xemu_input_bind_xmu(port, i, param); | ||
|
||
char *buf = g_strdup_printf("Connected XMU %s to port %d%c", param, port + 1, 'A' + i); | ||
xemu_queue_notification(buf); | ||
g_free(buf); | ||
} else { | ||
char *buf = g_strdup_printf("Unable to bind XMU at %s to port %d%c", param, port + 1, 'A' + i); | ||
xemu_queue_error_message(buf); | ||
g_free(buf); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} else if (event->type == SDL_CONTROLLERDEVICEREMOVED) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to unbind the XMU and free the peripherals in this path? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand. Are you asking in relation to the SDL_CONTROLLERDEVICEREMOVED event or the xemu_input_rebind_xmu call? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is in relation to the device removed event. When the device is removed, the controller state is freed, so any allocated peripherals will be leaked. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will unbind the XMU/peripherals when we call xemu_input_bind(iter->bind, NULL, 0) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I think I see what you mean. It's not the QEMU device that's being leaked, it's the memory for the peripheral state that's being leaked. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless I'm missing it though, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm beginning to think that maybe I should change that. I'm going through and fixing these memory leaks and running out of places where I'm not freeing the XmuState immediately after unbinding it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided to leave the freeing of the |
||
DPRINTF("Controller Removed: %d\n", event->cdevice.which); | ||
|
@@ -430,6 +501,11 @@ void xemu_input_bind(int index, ControllerState *state, int save) | |
if (bound_controllers[index]) { | ||
assert(bound_controllers[index]->device != NULL); | ||
Error *err = NULL; | ||
|
||
// Unbind any XMUs | ||
for(int i = 0; i < 2; i++) | ||
xemu_input_unbind_xmu(index, i); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to free the XMU/peripherals in this path? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if I understand what you're asking. When qdev_unplug is called in xemu_input_bind to free up the underlying QDEV device, it implicitly frees any peripherals connected to the hub. So, we could just set the peripherals[0]->dev and peripherals[1]->dev to NULL and peripheral_types[0] and peripheral_types[1] to PERIPHERAL_NONE. We can't bind them back again when we plug the new controller in though because qdev_unplug doesn't release the XMU image files immediately. This gave me an idea though. It might be useful if, instead of freeing the entire QEMU device when we swap controllers, we just reuse the same unerlying QEMU device and move it to the new ControllerState. This is what it would look like: If bound_controllers[index] is not null and state is not null, then we're going to be replacing an existing device with a new device. So, instead of freeing the existing device and creating a new one, we call a swap function that does the following:
Concerns: Any thoughts on this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initially I thought this might be another leak like the above case, but on second inspection it might not be since the underlying state isn't freed. From a technical standpoint I don't see an issue with your idea. From a UX standpoint, the current behavior when switching controllers is similar to switching on real hardware (i.e. unplugging one controller and plugging in another one). I don't suspect most users would care about preserving this, but it does add an extra step if this is wanted behavior. If you did want to attempt it, I would suggest sending it as a separate patch and getting Matt's opinion on the UX (leaving out the peripheral part of it and adjusting one or the other patch appropriately depending on which lands first). |
||
|
||
qdev_unplug((DeviceState *)bound_controllers[index]->device, &err); | ||
assert(err == NULL); | ||
|
||
|
@@ -461,7 +537,6 @@ void xemu_input_bind(int index, ControllerState *state, int save) | |
bound_controllers[index] = state; | ||
bound_controllers[index]->bound = index; | ||
|
||
const int port_map[4] = {3, 4, 1, 2}; | ||
char *tmp; | ||
|
||
// Create controller's internal USB hub. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my testing if you disconnect and reconnect a controller in the input UI the XMU is not rebound. Do we need to rebind peripherals here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how it behaves when we unplug a device and plug it back in with auto-bind enabled so we should probably do that here as well. If I remember correctly, this will cause Xemu to crash if the user selects a different controller with one already connected though because QEMU will not have finished releasing the XMU image after freeing the existing controller when it goes to rebind the XMU to the hub created for the new controller. If we want to do this then we might want to consider reusing the QEMU device that's already bound when changing from one controller to another instead freeing it and creating a new one. |
||
|
@@ -507,6 +582,118 @@ void xemu_input_bind(int index, ControllerState *state, int save) | |
} | ||
} | ||
|
||
void xemu_input_bind_xmu(int player_index, int peripheral_port_index, const char *filename) | ||
{ | ||
assert(player_index >= 0 && player_index < 4); | ||
assert(peripheral_port_index >= 0 && peripheral_port_index < 2); | ||
|
||
ControllerState *player = bound_controllers[player_index]; | ||
enum peripheral_type peripheralType = player->peripheral_types[peripheral_port_index]; | ||
if(peripheralType != PERIPHERAL_XMU) | ||
return; | ||
|
||
XmuState *xmu = (XmuState*)player->peripherals[peripheral_port_index]; | ||
|
||
// Unbind existing XMU | ||
if(xmu->dev != NULL) { | ||
xemu_input_unbind_xmu(player_index, peripheral_port_index); | ||
} | ||
|
||
if(filename == NULL) | ||
return; | ||
|
||
// Look for any other XMUs that are using this file, and unbind them | ||
for(int player_i = 0; player_i < 4; player_i++) { | ||
ControllerState *state = bound_controllers[player_i]; | ||
if(state != NULL) { | ||
for(int peripheral_i = 0; peripheral_i < 2; peripheral_i++) { | ||
if(state->peripheral_types[peripheral_i] == PERIPHERAL_XMU) { | ||
XmuState *xmu_i = (XmuState*)state->peripherals[peripheral_i]; | ||
assert(xmu_i); | ||
|
||
if(xmu_i->filename != NULL && strcmp(xmu_i->filename, filename) == 0) { | ||
char *buf = g_strdup_printf("This XMU is already mounted on player %d slot %c\r\n", player_i+1, 'A' + peripheral_i); | ||
xemu_queue_notification(buf); | ||
g_free(buf); | ||
return; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
xmu->filename = strdup(filename); | ||
faha223 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const int xmu_map[2] = {2, 3}; | ||
char *tmp; | ||
|
||
static int id_counter = 0; | ||
tmp = g_strdup_printf("xmu_%d", id_counter++); | ||
|
||
// Add the file as a drive | ||
QDict *qdict1 = qdict_new(); | ||
qdict_put_str(qdict1, "id", tmp); | ||
qdict_put_str(qdict1, "format", "raw"); | ||
qdict_put_str(qdict1, "file", filename); | ||
|
||
QemuOpts *drvopts = qemu_opts_from_qdict(qemu_find_opts("drive"), qdict1, &error_abort); | ||
|
||
DriveInfo *dinfo = drive_new(drvopts, 0, &error_abort); | ||
assert(dinfo); | ||
|
||
// Create the usb-storage device | ||
QDict *qdict2 = qdict_new(); | ||
|
||
// Specify device driver | ||
qdict_put_str(qdict2, "driver", "usb-storage"); | ||
|
||
// Specify device identifier | ||
qdict_put_str(qdict2, "drive", tmp); | ||
g_free(tmp); | ||
|
||
// Specify index/port | ||
tmp = g_strdup_printf("1.%d.%d", port_map[player_index], xmu_map[peripheral_port_index]); | ||
qdict_put_str(qdict2, "port", tmp); | ||
g_free(tmp); | ||
|
||
// Create the device | ||
QemuOpts *opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict2, &error_abort); | ||
|
||
DeviceState *dev = qdev_device_add(opts, &error_abort); | ||
assert(dev); | ||
|
||
xmu->dev = (void*)dev; | ||
|
||
// Unref for eventual cleanup | ||
qobject_unref(qdict1); | ||
qobject_unref(qdict2); | ||
|
||
xemu_save_peripheral_settings(player_index, peripheral_port_index, player->peripheral_types[peripheral_port_index], xmu->filename); | ||
} | ||
|
||
void xemu_input_unbind_xmu(int player_index, int peripheral_port_index) | ||
{ | ||
assert(player_index >= 0 && player_index < 4); | ||
assert(peripheral_port_index >= 0 && peripheral_port_index < 2); | ||
|
||
ControllerState *state = bound_controllers[player_index]; | ||
if(state->peripheral_types[peripheral_port_index] != PERIPHERAL_XMU) | ||
return; | ||
|
||
XmuState *xmu = (XmuState*)state->peripherals[peripheral_port_index]; | ||
if(xmu != NULL) | ||
{ | ||
if(xmu->dev != NULL) { | ||
qdev_unplug((DeviceState*)xmu->dev, &error_abort); | ||
object_unref(OBJECT(xmu->dev)); | ||
xmu->dev = NULL; | ||
} | ||
|
||
g_free((void*)xmu->filename); | ||
xmu->filename = NULL; | ||
} | ||
} | ||
|
||
void xemu_input_set_test_mode(int enabled) | ||
{ | ||
test_mode = enabled; | ||
|
@@ -515,4 +702,4 @@ void xemu_input_set_test_mode(int enabled) | |
int xemu_input_get_test_mode(void) | ||
{ | ||
return test_mode; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation depends on host byte order being little endian. Can you rework it so it will also work on BE hosts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed the code to use cpu_to_le32(u32) on the unsigned integers that were being written to the superblock. I think that will work but I don't have a BE machine to test on. It doesn't break this functionality on my machine.