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.

Sign up for GitHub

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

Jump to bottom

GB Camera support #564

Merged
merged 3 commits into from
May 22, 2018
Merged

GB Camera support #564

merged 3 commits into from
May 22, 2018

Conversation

bsmiles32
Copy link
Member

m64p_gbcam

Following discussion with @LuigiBlood in PR #446, this PR adds GB camera support.
See commit message for more information.

NOTE: I haven't updated the msvc files yet, I'd like to get some feedback first.

@AntonioND: big thanks for sharing your RE work on the GB camera !

@LuigiBlood
Copy link
Contributor

LuigiBlood commented May 11, 2018 •
edited
Loading

Oh wait you really went that far to support a camera as well. I didn't even do that.
You've used the same base as I did too :)

@bsmiles32
Copy link
Member Author

Yep, full GB camera support (as far as documented by @AntonioND) with live capture.
I know the quality of the GB camera is not great, but I see that as a first step... I have greater ambitions for the video input backend :) If you have some info on the A/V capture cart we could look into that as well

@LuigiBlood
Copy link
Contributor

This is as far as I got about the Capture Cartridge:
https://github.com/LuigiBlood/64dd/wiki/Capture-Cartridge

I don't understand the video portion a single bit. It's very confusing and each game handles it slightly differently.

@bsmiles32
Copy link
Member Author

Updated travis-ci file.

@bsmiles32
Copy link
Member Author

@LuigiBlood Thanks for the link, will look into it

Jj0YzL5nvJ
Jj0YzL5nvJ suggested changes May 12, 2018
View reviewed changes
Copy link
Contributor

@Jj0YzL5nvJ Jj0YzL5nvJ left a comment

Choose a reason for hiding this comment

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

Obsolete URL...?

src/backends/api/video_backend.h Outdated
@@ -0,0 +1,44 @@
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* Mupen64plus - video_backend.h *
* Mupen64Plus homepage: http://code.google.com/p/mupen64plus/ *
Copy link
Contributor

Choose a reason for hiding this comment

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

Obsolete URL...

@AntonioND
Copy link

Thanks for telling me, @bsmiles32! It's nice to see that other people find that documentation useful. :)

@bsmiles32 bsmiles32 mentioned this pull request May 14, 2018
Open
@bsmiles32
Copy link
Member Author

@Jj0YzL5nvJ Obsolete URL fixed. Thanks.

@bsmiles32
Copy link
Member Author

Few things to consider before considering merging this :

  • I've put the opencv specific configuration parameter in the Core section, but since it's backend specific it may be better to put it in a opencv video backend specific section
  • OpenCV is quite a big dependency that I don't want to force upon our users, maybe we should allow to make it optional at compile time (or completely disable camera support). Better, we should be able to enable available backends at compile-time, and for enabled backends be able to select which one to use at run-time. This not just for the video input backends, but for all backends when multiple implementation are available.
  • Only 1 camera can be used for all emulated gb cameras (up to 4 because one per game controller).
  • I went with OpenCV because it was easy to implement in a cross plateform way video capture, but if you happen to know a simpler cross-plateform video capture library I'm all ears :)

@richard42
Copy link
Member

I don't like having such a big dependency for something that most of our users will never use, so a compile-time make flag is probably necessary. You're using this for cross-platform capability of video capture with whatever cameras happen to be attached to the computer?

@bsmiles32
Copy link
Member Author

I don't like having such a big dependency for something that most of our users will never use, so a compile-time make flag is probably necessary.

Agreed.

You're using this for cross-platform capability of video capture with whatever cameras happen to be attached to the computer?

Yes. Note that the OpenCV capture API allow to use webcams but also video/image files transparently (not tested but advertised on their documentation). Internally it wraps different backends depending on the plateform it is run.
Beside the capture stuff I also use some functions to resize the captured image to the desired resolution (and to display images for debug purpose, but that's disabled by default). But I don't use any "image processing" OpenCV stuff.
The ideal solution would be to work with SDL upstream to have them integrate a cross-plateform video capture API, so we can benefit from that without all the OpenCV burden. But way too much work for a rarely used feature... Or find a much lighter library, or do an optimized windows build with only video capture and image resizing enabled so we don't bring the full opencv burden.

@bsmiles32
Copy link
Member Author

  • Updated with compile-time make flag (OPENCV=1 with Makefile). Not done for VS2013 because editing by hand msvc files is really asking for troubles, so some other Window dev will have to port this compile-time {en,dis}ablement. [For now, OpenCV is required when using VisualStudio].
  • Fixed a bug with savestates (forgot to rename {M64282FP -> POCKET_CAM}_REGS_COUNT value).

richard42
richard42 reviewed May 20, 2018
View reviewed changes
projects/unix/Makefile Outdated
@@ -646,7 +666,7 @@ targets:
@echo " WARNFLAGS=flag == compiler warning levels (default: -Wall)"
@echo " PIC=(1|0) == Force enable/disable of position independent code"
@echo " OSD=(1|0) == Enable/disable build of OpenGL On-screen display"
@echo " NEW_DYNAREC=1 == Replace dynamic recompiler with Ari64's experimental dynarec"
Copy link
Member

Choose a reason for hiding this comment

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

was this an accident? I think we should keep this line for NEW_DYNAREC in the makefile

@bsmiles32
Copy link
Member Author

@richard42 Yep, silly mistake on my part. Updated with Makefile line re-added.

@bsmiles32
Copy link
Member Author

Disabling opencv stuff in MSVC so that Appveyor has a chance to build it.

bsmiles32 added 2 commits May 21, 2018 23:32
@bsmiles32
Add support for GameBoy Camera
63c3684 
All the reverse-engineering work comes from AntonioND [1].
A new video backend API has been added to grab video images.
By default, a dummy backend is provided.
However, an OpenCV based backend is also provided (if enabled at
compile-time with OPENCV=1 in Makefile).
Other implementation should be possible (GStreamer for instance ?) in
the future.

With the OpenCV backend, the video device selection can be done using
the Core parameter:

[Core]
GbCameraVideoDevice=<my_device>

Where <my_device> can be either an integer which represent the device
number (0 for default) or a string which specify the video device path.

Tested with 64DD Mario Talent Studio (Japan), a transfer pak plugged
in the first controller with a Japanese GameBoy camera. Also since the
core currently requires a cart ROM (even if should strictly be required)
I used Perfect Dark (Japan) to allow using the Transfer Pak. This is a
core/ui limitation not related to this PR.

[1] https://github.com/AntonioND/gbcam-rev-engineer
@bsmiles32
Disable OpenCV on Windows by default.
49ac71a 
Revert this once win32-deps repo gets updated with OpenCV 3.0.
@richard42 richard42 merged commit f47b06e into mupen64plus:master May 22, 2018
@bsmiles32 bsmiles32 deleted the gbcam branch February 1, 2021 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richard42 richard42 richard42 left review comments

@Jj0YzL5nvJ Jj0YzL5nvJ Jj0YzL5nvJ requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bsmiles32 @LuigiBlood @AntonioND @richard42 @Jj0YzL5nvJ