IIDX 25/26 - Add the ability to leave a camera unassigned #245

Merged
JeffPaine4890 merged 15 commits from add-cam-skip into master 2023-04-10 22:44:52 +03:00
JeffPaine4890 commented 2023-04-09 05:19:49 +03:00 (Migrated from github.com)

By setting the custom camera device override in iidxhook.conf to SKIP, bemanitools will now leave that camera unassigned. This is particularly useful for leaving camera 1 unassigned and letting bemanitools automatically map camera 2; most people have their webcams pointed at their face, and camera 2 corresponds to the face cam ingame whereas camera 1 is the hand cam. Also if someone for some reason wanted to use the QR scanning function, that goes through camera 2, so someone with only one camera would have been completely unable to use that.

By setting the custom camera device override in iidxhook.conf to SKIP, bemanitools will now leave that camera unassigned. This is particularly useful for leaving camera 1 unassigned and letting bemanitools automatically map camera 2; most people have their webcams pointed at their face, and camera 2 corresponds to the face cam ingame whereas camera 1 is the hand cam. Also if someone for some reason wanted to use the QR scanning function, that goes through camera 2, so someone with only one camera would have been completely unable to use that.
willxinc (Migrated from github.com) reviewed 2023-04-09 05:19:49 +03:00
icex2 (Migrated from github.com) reviewed 2023-04-09 14:00:28 +03:00
icex2 (Migrated from github.com) commented 2023-04-09 14:00:28 +03:00

I am a bit on the fence for using the string SKIP here as the implied type for this field is a device path. Mixing it's purpose with a new type of feature switch makes this switch more complex and blurry to understand.

Instead, I suggest to add an explicit new option in config-cam that allows you to "disable" specific cameras (as far as I understand the idea of "unassigned" here and what it does for the end-user).

struct camhook_config_cam {
    bool disable_emu;
    size_t num_devices;
    char device_id[CAMHOOK_CONFIG_CAM_MAX][MAX_PATH];
    bool disable_cam;
};
I am a bit on the fence for using the string `SKIP` here as the implied type for this field is a device path. Mixing it's purpose with a new type of feature switch makes this switch more complex and blurry to understand. Instead, I suggest to add an explicit new option in [config-cam](https://github.com/djhackersdev/bemanitools/blob/e9d24f7f90d9e60e6a922ea969fb56b89a7d9914/src/main/camhook/config-cam.h#L13) that allows you to "disable" specific cameras (as far as I understand the idea of "unassigned" here and what it does for the end-user). ```c struct camhook_config_cam { bool disable_emu; size_t num_devices; char device_id[CAMHOOK_CONFIG_CAM_MAX][MAX_PATH]; bool disable_cam; }; ```
icex2 commented 2023-04-09 14:01:00 +03:00 (Migrated from github.com)

Also requsted review from @willxinc. I think he knows that part of the code better than I do.

Also requsted review from @willxinc. I think he knows that part of the code better than I do.
willxinc (Migrated from github.com) reviewed 2023-04-09 23:36:08 +03:00
willxinc (Migrated from github.com) commented 2023-04-09 23:36:08 +03:00

It would need to be an array, but that seems reasonable to me, would probably simplify the check later too.

It would need to be an array, but that seems reasonable to me, would probably simplify the check later too.
willxinc commented 2023-04-09 23:36:52 +03:00 (Migrated from github.com)

I agree with the bit from icex2 about not using "SKIP" otherwise sounds reasonable to me, if adding the config is too much of a lift, let me know, I can make the changes and submit a new PR for you

I agree with the bit from icex2 about not using "SKIP" otherwise sounds reasonable to me, if adding the config is too much of a lift, let me know, I can make the changes and submit a new PR for you
JeffPaine4890 commented 2023-04-09 23:52:51 +03:00 (Migrated from github.com)

I glanced at the code this morning and I think I can figure it out. Not sure if I'll have time tonight, if you want to do it in the meantime go ahead, otherwise I'll fix it up when I can.

I was thinking of something like adding cam.enable_camera1 and cam.enable_camera2 which would both default to true.

I glanced at the code this morning and I think I can figure it out. Not sure if I'll have time tonight, if you want to do it in the meantime go ahead, otherwise I'll fix it up when I can. I was thinking of something like adding cam.enable_camera1 and cam.enable_camera2 which would both default to true.
willxinc commented 2023-04-10 02:33:49 +03:00 (Migrated from github.com)

yep that sounds reasonable, main issue is the config keys, etc. since the config format doesn't support arrays, but if you check how device_id is handled, should be pretty clear

> yep that sounds reasonable, main issue is the config keys, etc. since the config format doesn't support arrays, but if you check how device_id is handled, should be pretty clear
JeffPaine4890 commented 2023-04-10 22:14:49 +03:00 (Migrated from github.com)

Okay, now the config files for iidx25-29 include a cam.disable_camera1 and cam.disable_camera2 option, which both default to false.

Also, while I was updating iidxhook-27.conf, I aligned the cam.disable_emu default with iidx28/29's default

Okay, now the config files for iidx25-29 include a cam.disable_camera1 and cam.disable_camera2 option, which both default to false. Also, while I was updating iidxhook-27.conf, I aligned the cam.disable_emu default with iidx28/29's default
icex2 (Migrated from github.com) reviewed 2023-04-10 22:39:37 +03:00
icex2 (Migrated from github.com) commented 2023-04-10 22:39:36 +03:00

I think this still needs to be removed

# Override camera device ID 2 detection (copy from device manager, do not escape) Leave blank to automatically detect
I think this still needs to be removed ```suggestion # Override camera device ID 2 detection (copy from device manager, do not escape) Leave blank to automatically detect ```
icex2 (Migrated from github.com) reviewed 2023-04-10 22:39:49 +03:00
icex2 (Migrated from github.com) commented 2023-04-10 22:39:48 +03:00

Dito

# Override camera device ID 1 detection (copy from device manager, do not escape) Leave blank to automatically detect
Dito ```suggestion # Override camera device ID 1 detection (copy from device manager, do not escape) Leave blank to automatically detect ```
icex2 (Migrated from github.com) reviewed 2023-04-10 22:40:03 +03:00
icex2 (Migrated from github.com) commented 2023-04-10 22:40:02 +03:00
# Override camera device ID 2 detection (copy from device manager, do not escape) Leave blank to automatically detect
```suggestion # Override camera device ID 2 detection (copy from device manager, do not escape) Leave blank to automatically detect ```
icex2 (Migrated from github.com) reviewed 2023-04-10 22:40:11 +03:00
icex2 (Migrated from github.com) commented 2023-04-10 22:40:10 +03:00
# Override camera device ID 1 detection (copy from device manager, do not escape) Leave blank to automatically detect
```suggestion # Override camera device ID 1 detection (copy from device manager, do not escape) Leave blank to automatically detect ```
icex2 commented 2023-04-10 22:40:59 +03:00 (Migrated from github.com)

Nice job. lgtm aside the few cleanups that I pointed out. Will approve and merge once that's done.

Nice job. lgtm aside the few cleanups that I pointed out. Will approve and merge once that's done.
JeffPaine4890 commented 2023-04-10 22:43:46 +03:00 (Migrated from github.com)

You're right I forgot to change that back >_>

You're right I forgot to change that back >_>
icex2 (Migrated from github.com) approved these changes 2023-04-10 22:43:56 +03:00
Sign in to join this conversation.