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

Added webcam support for Linux #47967

Closed
wants to merge 1 commit into from

Conversation

Schmetzler
Copy link

@Schmetzler Schmetzler commented Apr 17, 2021

I added webcam support for Linux.
I used libv4l2 which is dynamically loaded if present. If it is not present the kernel default v4l2 library is used.
It should be able to grab images even without libv4l2 if the camera supports RGB output directly.
With libv4l2 the image is always grabbed as RGB8 image.

I also implemented a simple hotplug feature which just checks the /dev/video* files every second.

I tested it with the TestCameraServer Project from https://github.com/BastiaanOlij/godot3_test_projects.git
I could only test for TYPE_IO_MMAP as my camera only supports this, but I implemented also the other types using the information provided by https://kernel.readthedocs.io/en/latest/media/uapi/v4l/capture.c.html

@Schmetzler Schmetzler requested a review from a team as a code owner April 17, 2021 02:51
@Chaosus Chaosus added this to the 4.0 milestone Apr 17, 2021
@Schmetzler Schmetzler force-pushed the linux-add-webcam branch 6 times, most recently from afdafc8 to b71add4 Compare April 17, 2021 10:41
// get some info
name = device->name;
// I do not think that another position is possible on linux
position = CameraFeed::FEED_UNSPECIFIED;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unspecified I think is fine for a webcam, front/back really only applies to phones.

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

I'm not a linux guy so I can't really test whether this works as advertised but doing a quick code review it looked sensible.

Would like someone to look at this who knows more about linux, @hpvb maybe you can have a quick look?

do {
r = funcs->ioctl(fd, request, arg);
} while (-1 == r && (EINTR == errno || errno == EAGAIN));
return r;
Copy link
Contributor

Choose a reason for hiding this comment

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

yoda coding! love it but not really our style :)

Copy link
Author

Choose a reason for hiding this comment

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

TBH that was just how the example was implemented. I found a little weird too, but hey semantics are the same.^^ I can change this and make it more common if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll let @akien-mga be the judge of coding styles :)

I do think Yoda coding has merit, it does catch certain errors, but I don't think we've got it anywhere else in the source code.

Copy link
Member

@Calinou Calinou Apr 19, 2021

Choose a reason for hiding this comment

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

Indeed, we don't use Yoda conditions in Godot's codebase as far as I know.

Copy link
Author

Choose a reason for hiding this comment

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

So I removed the Yoda conditions to not confuse the codebase.

@Schmetzler
Copy link
Author

Ah as I want to mention. I would also like to push it to the godot 3.x branch, but therefore the Vector<uint8_t> must be changed to PoolVector<uint8_t>. But i guess it should be cherrypicked after this one is merged?

@BastiaanOlij
Copy link
Contributor

Ah as I want to mention. I would also like to push it to the godot 3.x branch, but therefore the Vector<uint8_t> must be changed to PoolVector<uint8_t>. But i guess it should be cherrypicked after this one is merged?

Yes that would be a cherry pick :)

@Ansraer
Copy link
Contributor

Ansraer commented May 10, 2021

Just gonna link the webcam support issue here. Should make it easier for me to adjust the issue when this gets merged: #46531

@Calinou Calinou mentioned this pull request Jul 15, 2021
georgewsinger added a commit to SimulaVR/godot that referenced this pull request Jul 15, 2021
@georgewsinger
Copy link

georgewsinger commented Jul 15, 2021

Does this code support setting the scene's Environment to be the webcam stream? I have tried to do this and am just getting a black sky.

Code source: I merged this code into our Godot 3.2 fork here:

Using this to run godot with a webcam located at /dev/video0 (verified to work outside of godot) and with

environment.set_background(BG_CAMERA_FEED)

set results in

libv4l2 found.
Registered camera UVC Camera (046d:0825) (/dev/video0) with id 1 position 0 at index 0
/dev/video1 is no video capture device.

but just results in a black ground; moreover, the webcam doesn't even turn on.

Tracing the source of the issue: I've discovered that the specific failure is located in RasterizerSceneGLES3::render_scene, where feed.is_valid() evaluates to false, causing the webcam texture to not get drawn in the background/Environment:

				if (feed.is_valid() && (feed->get_base_width() > 0) && (feed->get_base_height() > 0)) {
					// copy our camera feed to our background

					glDisable(GL_BLEND);
					glDepthMask(GL_FALSE);
					glDisable(GL_DEPTH_TEST);
					glDisable(GL_CULL_FACE);
// ...

@georgewsinger
Copy link

After some fidgeting I got this to work, but only from gdb: https://www.youtube.com/watch?v=KhTs9uriXDQ

bool CameraFeedX11::activate_feed() {
	// activate streaming if not already
	if (!device->streaming) {
		if (!device->check_device()) {
			return false;
		}
		if (!device->request_buffers()) {
			device->close();
			return false;
		}
		if (!device->start_streaming(this)) {
			device->cleanup_buffers();
			device->close();
			return false;
		}
	};

	return true;
};

If I manually step through this via gdb (n over and over through this function), then the webcam starts. If I don't use gdb to step through like this, then it fails. There must be some sort of async issue going where pauses are necessary to get it to work.

@Calinou
Copy link
Member

Calinou commented Jul 16, 2021

If I manually step through this via gdb (n over and over through this function), then the webcam starts. If I don't use gdb to step through like this, then it fails. There must be some sort of async issue going where pauses are necessary to get it to work.

Does it work if you call OS::get_singleton()->delay_msec(100) every now and then in that function (with possibly larger or smaller values)?

@georgewsinger
Copy link

georgewsinger commented Jul 16, 2021

If I manually step through this via gdb (n over and over through this function), then the webcam starts. If I don't use gdb to step through like this, then it fails. There must be some sort of async issue going where pauses are necessary to get it to work.

Does it work if you call OS::get_singleton()->delay_msec(100) every now and then in that function (with possibly larger or smaller values)?

Yes, changing the function to the following makes now it work without having to manually step through it w/GDB:

bool CameraFeedX11::activate_feed() {
	// activate streaming if not already
	if (!device->streaming) {
		OS::get_singleton()->delay_usec(100); //<-- here
		if (!device->check_device()) {
			return false;
		}
		OS::get_singleton()->delay_usec(100); //<-- here
		if (!device->request_buffers()) {
			device->close();
			return false;
		}
		OS::get_singleton()->delay_usec(100); //<-- here
		if (!device->start_streaming(this)) {
			device->cleanup_buffers();
			device->close();
			return false;
		}
	};

	return true;
};

@georgewsinger
Copy link

	fd = funcs->open(dev_name.c_str(), O_RDWR | O_NONBLOCK, 0);

O_NONBLOCK is used in the calls to funcs->open, which is probably what's causing the async issue here.

@georgewsinger
Copy link

georgewsinger commented Jul 17, 2021

The webcam image is also left-right inverted when shown in the Environment, which is fixed by a call to img->flip_x() in V4l2_Device::get_image.

@RMKD
Copy link

RMKD commented Jul 24, 2021

Hey all - excited to see this. I'm trying to reproduce with a build from the fork. I get both a usb web cam and CV1 external cameras detected with libv4l2 in the log and can get them via CameraServer.feeds() but can't figure out how to activate or retrieve the texture in gdscript. Do I need to do something in native?

@MrJustreborn
Copy link

Will this work with a v4l2loopback-device? (eg Screencapture/OBS)

@Schmetzler
Copy link
Author

@georgewsinger

The webcam image is also left-right inverted when shown in the Environment, which is fixed by a call to img->flip_x() in V4l2_Device::get_image.

I don't think this should be done in the module, because sometimes it depends on the webcam what kind of picture it sends. In my opinion such things should be done in the shader.

	fd = funcs->open(dev_name.c_str(), O_RDWR | O_NONBLOCK, 0);

O_NONBLOCK is used in the calls to funcs->open, which is probably what's causing the async issue here.

To solve this async issue I guess your proposal with the little delay is best. What do the others think. I do not think that making the open call blocking is a good idea

@MrJustreborn

Will this work with a v4l2loopback-device? (eg Screencapture/OBS)

Yes it works also with a v4l2loopback device (I tested this)

@RMKD
Copy link

RMKD commented Aug 6, 2021

I'm still missing something in terms of how to get a basic test scene running. It seems like CameraServer should be returning feeds as CameraX11 objects, but invoking functions on the returned object yields a 'non-existent function' error. Any suggestions for setup or scene links?

Also, should instance be instantiate with the current head of master as the compiler suggests? Possibly there is a breaking change made after this PR was authored?

[ 65%] modules/camera/camera_x11.cpp: In member function 'void V4l2_Device::get_image(Ref<CameraFeed>, uint8_t*)':
modules/camera/camera_x11.cpp:556:13: error: 'class Ref<Image>' has no member named 'instance'; did you mean 'instantiate'?
  556 |         img.instance();
      |             ^~~~~~~~
      |             instantiate
[ 65%] Compiling ==> editor/rename_dialog.cpp
modules/camera/camera_x11.cpp: In member function 'void CameraX11::update_feeds()':
modules/camera/camera_x11.cpp:690:41: error: 'class Ref<CameraFeedX11>' has no member named 'instance'; did you mean 'instantiate'?
  690 |                                 newfeed.instance();
      |                                         ^~~~~~~~
      |                                         instantiate
scons: *** [modules/camera/camera_x11.linuxbsd.tools.64.o] Error 1
scons: building terminated because of errors.

@Calinou
Copy link
Member

Calinou commented Aug 6, 2021

Also, should instance be instantiate with the current head of master as the compiler suggests?

Indeed, instance() was renamed to instantiate() in master. A lot of things are being renamed in 4.0 for clarity, see #16863.

@RMKD
Copy link

RMKD commented Aug 8, 2021

Updating the .cpp to use instiantiate() compiles just fine (thx for the detailed change list). If i include GDCLASS(CameraFeedX11, CameraFeed); in the header file the server and feeds are recognized as the X11 class types... but they still don't have the expected methods from CameraFeed like get_name() - I notice some of the other modules include code to explicitly bind methods along with the GDCLASS clause:

//.h
protected:
	static void _bind_methods();
//.cpp
void GLTFTexture::_bind_methods() {
	ClassDB::bind_method(D_METHOD("get_src_image"), &GLTFTexture::get_src_image);
	ClassDB::bind_method(D_METHOD("set_src_image", "src_image"), &GLTFTexture::set_src_image);

I'm a little out of my depth on the c++ front so making some guesses - is something like the above required to invoke inherited class functions from gdscript in v4?

@RMKD
Copy link

RMKD commented Aug 9, 2021

well, this explains a lot: https://github.com/godotengine/godot/blob/master/servers/camera/camera_feed.cpp#L36-L37 (turns out manually binding the methods in camera_x11.cpp does help... because the parent class currently disables them in master)

@RMKD
Copy link

RMKD commented Aug 12, 2021

From what I can piece together, the circumvented CameraFeed code is impacted by both the refactoring of VisualServer (to RenderingServer and maybe some other places?) and the noted Vulkan refactoring. The required porting appears to include:

  • RenderingServer->texture_create()
  • RenderingServer->texture_allocate()
  • RenderingServer->texture_set_data()

Is there a thread of work covering this or would it just be an isolated PR?
Are rendering_device or renderer_storage the right places to look for replacements?

@bruvzg

This comment has been minimized.

@bruvzg
Copy link
Member

bruvzg commented Aug 12, 2021

PR for CameraFeed fix - #51581

@RMKD
Copy link

RMKD commented Aug 14, 2021

Confirmed to work via the lib4vl2 method on linux in-editor (thx for the demo scene in the PR) - but the texture doesn't render in a played scene, even with a script to cycle through and set_active(true) so I'll see if I can sort out what's going on there.

@bruvzg
Copy link
Member

bruvzg commented Aug 14, 2021

Confirmed to work via the lib4vl2 method on linux in-editor (thx for the demo scene in the PR) - but the texture doesn't render in a played scene, even with a script to cycle through and set_active(true) so I'll see if I can sort out what's going on there.

It seems to work only 3D scene is opened, closed and reopened in editor, or if it's assigned to 2D sprite (in this case it's working in running project, but only after resizing the window).

Probably something in the CameraTerxture/Feed/Server is initialized before RenderingServer is ready, but I'm not sure what's exactly wrong.

@pkowal1982
Copy link
Contributor

pkowal1982 commented Sep 13, 2021

I was also implementing linux camera support, so maybe we could combine best of both solutions?
#53666

@CrispyPin
Copy link
Contributor

What testing is needed for this to be merged?

What is required for this to be backported to 3.x?

@Schmetzler
Copy link
Author

Schmetzler commented Oct 12, 2021

@CrispyPin

See my note above:

Ah as I want to mention. I would also like to push it to the godot 3.x branch, but therefore the Vector<uint8_t> must be changed to PoolVector<uint8_t>. But i guess it should be cherrypicked after this one is merged?

And according to this you also have to change void V4l2_Device::get_image(Ref<CameraFeed> feed, uint8_t *buffer) in camera_x11.cpp around line 572.

from:

uint8_t *w = img_data.ptrw();
// TODO: Buffer is 1024 Byte longer?
memcpy(w, buffer, width * height * 3);

to:

PoolVector<uint8_t>::Write w = img_data.write();
// TODO: Buffer is 1024 Byte longer?
memcpy(w.ptr(), buffer, width*height*3);

This should be enough to backport it to godot 3.x

@bnolan
Copy link

bnolan commented Feb 13, 2022

Will this be merged into 3.5?

@Calinou
Copy link
Member

Calinou commented Feb 13, 2022

Will this be merged into 3.5?

This needs to be merged in master first, and a 3.x version of this pull request should be opened (and merged). To ensure feature parity, we only merge new features in 3.x if their master counterpart was merged first (unless the feature isn't relevant for master in the first place).

Note that there is another pull request implementing the same feature: #53666
I'm undecided on which one is the "better" implementation.

If you want to help get this merged, you can test it locally on your own device and make sure it works as expected 🙂

@akien-mga
Copy link
Member

Superseded by #53666. Thanks for the contribution nonetheless!

@akien-mga akien-mga closed this Sep 25, 2024
@akien-mga akien-mga removed this from the 4.x milestone Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.