Skip to content
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

Camera rework and small fixes #137

Merged
merged 30 commits into from
Nov 22, 2023
Merged

Camera rework and small fixes #137

merged 30 commits into from
Nov 22, 2023

Conversation

Meziu
Copy link
Member

@Meziu Meziu commented Oct 15, 2023

Big (and messy) PR incoming!

Changes related to the cam service:

  • Rework of many cam functions to fix weird behaviour or crashes and to streamline the service. Still WIP (Trimming still needs some research and the image calibration configs don't seem to work).
  • Removed the need for ALL redundant "image size" requests in function arguments.
  • Support for BothOutwardCam using a specific take_picture implementation.
  • The camera-image example now takes 3D images (and is abnormally slow thanks to CPU rendering).
  • The camera-image example doesn't need all the specific const values to work properly.

Some other minor changes that were made in the mean time:

  • Renamed white/blacklist functions to allow/blocklist.
  • Safely wrapped Console::set_window.
  • Properly documented GSPGPU events.
  • Added missing functionality for the Hid module, such as volume slider and accelerometer/gyroscope. After some more research (done for the changes) I noticed the service requires to be exclusively usable from one instance, using ServiceReference, as it modifies global states which are normally un-trackable.
  • Changed how ServiceReference checks for service usage. Poison handling is now a feature!
  • Fixed some panic! cases that are now handled as new Error types. (also renamed NdspError to just Error following the module namespaces changes)
  • Added warnings and changed functionality to avoid dangerous unsafe cases when using Gfx on the VRAM.
  • Translated a couple of calls to static inlines after the merge of Support static inline functions in ctru-sys #133.

P.S.
I'm sorry for mixing up the changes to the cam service and those related to TODO or other small fixes. I had initially envisioned this PR as a "todo fix" PR but as I worked on it I noticed that cam was going to need a lot of work.

@Meziu
Copy link
Member Author

Meziu commented Oct 15, 2023

It seems like nobody on github ever used stereo calibration before (not even devkitPro's own examples include it) nor image quality calibration. In my tests it seems like CAMU_SetStereoCameraCalibrationData has no effect, so I'm wondering how these functions are supposed to work. The same goes for trimming, of which I have understood only that it accepts images of the "standard" supported resolutions (like VGA and TOP_LCD) but that I still can't grasp in its entirety (does it accept all resolutions with an area multiple of 256?).

@Steveice10 Sorry for the tag, but since you've written the cam:u implementation for libctru, and have a lot of experience with FBI and cameras in general, would you mind sharing some of your knowledge with us? I've spent a couple of weeks reverse-engineering the cam service, but your insight would be greatly appreciated.

@Meziu
Copy link
Member Author

Meziu commented Oct 16, 2023

Thank you so much for the information! I didn't expect your quick response 😄.

* You need to use `cam:s` (I think `cam:c` also works?) for privileged access. All 3 cam services share the same handler but have state checks on what service needs to be used to access certain commands.

Interesting. This effectively means I'd need to re-implement all IPC calls to use my own cam:s handle, correct? In libctru everything uses cam:u, so that probably won't work. Also, would that change bring any trade-offs that you know of, or is the handle to use only up to the specific needs?

2. For trimming I don't think I ever played with it thoroughly enough beyond standard dimensions to know what exactly it will and won't accept.

No problem, I've already been experimenting with it enough. I should be able to find some conclusive evidence in no time, i hope...

I guess I should try to merge this PR more or less as-is and work on re-implementing everything over cam:s if possible later, since it would be way too disruptive right now.

Copy link
Member

@ian-h-chamberlain ian-h-chamberlain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't have a chance to deeply read the new camera code yet but mostly LGTM! Couple minor comments but otherwise all seems good

ctru-rs/examples/camera-image.rs Outdated Show resolved Hide resolved
ctru-rs/src/console.rs Outdated Show resolved Hide resolved
ctru-rs/src/console.rs Outdated Show resolved Hide resolved
ctru-rs/src/console.rs Show resolved Hide resolved
ctru-rs/src/lib.rs Outdated Show resolved Hide resolved
ctru-rs/src/services/hid.rs Outdated Show resolved Hide resolved
ctru-rs/src/services/hid.rs Outdated Show resolved Hide resolved
ctru-rs/src/services/hid.rs Show resolved Hide resolved
ctru-rs/src/services/ndsp/mod.rs Show resolved Hide resolved
ctru-rs/src/services/cam.rs Outdated Show resolved Hide resolved
@Meziu Meziu marked this pull request as ready for review November 19, 2023 18:02
@Meziu Meziu requested a review from a team as a code owner November 19, 2023 18:02
@Meziu Meziu requested a review from AzureMarker November 19, 2023 18:02
@Meziu
Copy link
Member Author

Meziu commented Nov 19, 2023

This PR is finally ready to merge.

ctru-rs/examples/movement.rs Outdated Show resolved Hide resolved
@Meziu Meziu merged commit 9406f5d into master Nov 22, 2023
4 checks passed
@Meziu Meziu deleted the camera-and-more branch November 22, 2023 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants