Draft: Pipewire unified wrappers

[columbarius]

Description

Unify pipewire.c api in the linux-pipewire plugin

Motivation and Context

We want to create a basic pipewire wrapper to share code between multiple obs plugins. This MR should create the basis for discussions to create sth. useful for various video and audio plugins.

How Has This Been Tested?

Types of changes

• Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

• My code has been run through clang-format.
• I have read the contributing document.
• My code is not on the master branch.
• The code has been tested.
• All commit messages are properly formatted and commits squashed where appropriate.
• I have included updates to all appropriate documentation.

[columbarius]

Relevant MRs:
#5377
#5728
#6207

[columbarius]

CC: @GeorgesStavracas @dimtpap

[columbarius]

Moved comment from #5728 (comment)

We separate camera plugin and registry.

• camera-plugin: init_registry -> query_cameras -> user selects camera -> query_registry for fd and id -> create stream like screencast -> ... -> unref_registry
• camera-registry: global refcounted struct, which deals with the camera/device-portal. Is created on first init_registry call and destroyed if the refcount is 0. It queries the portal for a filedescriptor and opens the connection and tracks all announced cameras.

In principle, I think separating different moving parts into their own unit is good. My idea with #6645 is that all the code under pipewire.c is generic and can be used by all source types - even audio sources.

I don't know if it would be feasible or even possible, but I think you can already make this camera-registry thing completely generic, so it can be reused by the audio source at #6207. It'll probably need to be implemented in pipewire.c directly, perhaps as follows:

* Split the code that actually initializes PipeWire from `obs_pipewire_create` into a new `obs_pipewire_start`

* Add a new vfunc struct `obs_pipewire_device_callbacks` with the regular `device_added`, `device_removed`, `device_changed`, etc callback (please feel free to suggest better naming here)

* Add `obs_pipewire_set_device_callbacks()`

That way the camera portal code will be able to react to PipeWire adding and removing cameras, without having to deal with PipeWire itself.

What do you think?

I like the idea to build a registry wrapper with these callbacks. I'm not sure a obs_pipewire_set_device_callbacks() is feasable but an obs_pipewire_registry_create() with these callbacks as an argument (see stream_events here) will be.

An alternative would be to not lazyload the registry, but start it when obs is launched. This way we can only add the camera plugin, if obs is allowed to access the portal.
If we lazy load I would suggest to work asynchronous with callbacks from the registry to the plugin after it's initialized, but that's just some idea.

I think the best approach here is to do what I've done with the screencasting code, and completely separate camera portal code from PipeWire itself. What the plugin needs to do is to expose the pw-camera sources all the time, and only query the Device Access portal when the user actually adds a pw-camera source. If camera access is denied, we show an error message in the properties dialog.

I think we might talk about different things here: The screencast portal creates for each stream a new pipewire_fd together with the pipewire_id which the client can use to access only this single stream. Iirc the camera portal only grants access to a single pipewire_fd, which can be used to access pipewire on the host with permissions on the connection only to show the client cameras, etc. In this case the client is responsible to set up a registry to get the pipewire_id of each camera. Can't we just use the provided pipewire_fd together with the pipewire_id, obtained from the registry, and just create a new stream (including it's own pw_core, pw_thread_loop, etc.) with those?

Anyways: should I just continue to rebase the current state, or should I continue with my new proposal?

Considering what I've suggested above, feel free to continue with your new proposal

This way we could create one wrapper each for pipewire_stream, pipewire_registry (with the suggested callbacks) and pipewire_node (@dimtpap could you extract such a wrapper from your audio work?)

[columbarius]

Btw. creating pw_core and pw_thread_loop and such things should be done by static functions inside pipewire.c and shared between the wrapper APIs.

[columbarius]

Added registry stuff, nothing tested and just a sketch.

[columbarius]

Does anyone know how to do fa935cc in nice?

[dimtpap]

Does anyone know how to do fa935cc in nice?

Why not make it a regular struct and pass it to pw_core_add_listener

[GeorgesStavracas]

I think this PR needs some work, but ultimately what it tries to achieve is desired for the maintainability of the plugin. Technical details aside, I think the internal API here is a bit odd in some places. What I had in mind with device registry callbacks and whatnot was something like this:

struct obs_pipewire_device_callbacks {
	void (*device_added)(void *user_data, obs_pipewire_device *device);
	void (*device_removed)(void *user_data, obs_pipewire_device *device);
};

void obs_pipewire_add_device_callbacks(
	obs_pipewire_data *obs_pw, 
	const struct obs_pipewire_device_callbacks *device_callbacks);

void obs_pipewire_remove_device_callbacks(
	obs_pipewire_data *obs_pw, 
	const struct obs_pipewire_device_callbacks *device_callbacks);

To properly do this, we would need to split the initialization of obs_pipewire in 2 steps:

obs_pipewire_data * obs_pipewire_create(int pipewire_fd);

obs_pipewire_stream *
obs_pipewire_connect_stream(
	obs_pipewire_data *obs_pw,
	uint32_t stream_node,
	const char *stream_name,
	struct pw_properties *stream_properties);

The screencast portal code would look more or less like this:

	capture->obs_pw = obs_pipewire_create(pipewire_fd);
	capture->pipewire_stream = obs_pipewire_connect_stream(capture->obs_pw,
		capture->pipewire_node, "OBS Studio",
		pw_properties_new(PW_KEY_MEDIA_TYPE, "Video",
				  PW_KEY_MEDIA_CATEGORY, "Capture",
				  PW_KEY_MEDIA_ROLE, "Screen", NULL)
	obs_pipewire_stream_set_cursor_visible(capture->pipewire_stream,
					capture->cursor_visible);

The camera portal code would look more or less like this:

static const struct obs_pipewire_device_callbacks camera_device_callbacks = {
	camera_portal_device_added,
	camera_portal_device_remove,
};

...

	camera->obs_pw = obs_pipewire_create(pipewire_fd);
	obs_pipewire_add_device_callbacks(camera->obs_pw, &camera_device_callbacks);

... (every time someone selects a camera) ...

	capture->pipewire_stream = obs_pipewire_connect_stream(camera->obs_pw,
		device->pipewire_node, "OBS Studio",
		pw_properties_new(PW_KEY_MEDIA_TYPE, ...,
				  PW_KEY_MEDIA_CATEGORY, ...,
				  PW_KEY_MEDIA_ROLE, "Screen", NULL));

[columbarius]

Added the case of an output stream to pipewire.c in planing of the virtualcamera pluging.

[columbarius]

@GeorgesStavracas I'm pretty happy with your ideas! From my side it's ready for review.

[columbarius]

@GeorgesStavracas I'm not sure about the api design. On one hand I'd like to have a single function for obs_pipewire_stream_pause on the other hand audio won't need a _get_width function.

Would you prefere one api for video_in, video_out, audio_in or one unified one, created with a

struct {
    type,
    union {
        video_in,
        ...,
    }
}

Thing and internally we do a lot of switch case?

[columbarius]

I went this weekend on a totally different direction using a struct of function pointers holding the implementation of certain features for each backend which wants to implement the obs_pipewire_stream thing/interface. The results are quite nice.
Here is a branch containing rebased versions of #5377 and #5728.

@GeorgesStavracas Please take a glance at it, if we want to go with this kind of api (pipewire-internal.h)

@dimtpap Please look if extending the impl struct with audio functions would help you.

[wtay]

I rebased the branch and added some small fixes: https://github.com/wtay/obs-studio/tree/pipewire-unified-dev

[GeorgesStavracas]

This pull request has become too chaotic and unmanageable. I'll start breaking it in smaller chunks, and will handle it.

I think something went wrong with my rebase, half of the commits don't even build. I'll fix that too.

[columbarius]

@GeorgesStavracas please ping me when I should start rebasing this including fixes from @wtay onto #8153
Nit: you linked #5614 instead of this MR there.

Same is valid if a review from me is helpful.

[PatTheMav]

@GeorgesStavracas What's the intended plan for this PR? Is there a roadmap of changes that need to be merged first and does this PR still make sense in its current form (I guess the change itself is desired, just not the way it's implemented in the PR)?