Skip to content

linux-pipewire: Use dynamic pod builder #10459

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

Open
GeorgesStavracas wants to merge 1 commit into
base: master
Choose a base branch
from
Open

linux-pipewire: Use dynamic pod builder #10459

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 50 additions & 52 deletions plugins/linux-pipewire/pipewire.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <spa/debug/format.h>
#include <spa/debug/types.h>
#include <spa/param/video/type-info.h>
#include <spa/pod/dynamic.h>
#include <spa/utils/result.h>

//#define DEBUG_PIPEWIRE
Expand Down Expand Up @@ -299,10 +300,10 @@ static void swap_texture_red_blue(gs_texture_t *texture)
}

static inline struct spa_pod *build_format(obs_pipewire_stream *obs_pw_stream,
struct spa_pod_builder *b,
uint32_t format, uint64_t *modifiers,
size_t modifier_count)
{
struct spa_pod_dynamic_builder pod_builder;
struct spa_rectangle max_resolution = SPA_RECTANGLE(8192, 4320);
struct spa_rectangle min_resolution = SPA_RECTANGLE(1, 1);
struct spa_rectangle resolution;
Expand All @@ -311,6 +312,8 @@ static inline struct spa_pod *build_format(obs_pipewire_stream *obs_pw_stream,
struct spa_fraction min_framerate;
struct spa_fraction framerate;

spa_pod_dynamic_builder_init(&pod_builder, NULL, 0, 4096);
Copy link
Collaborator

@kkartaltepe kkartaltepe Mar 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This should be a lot smaller now. instead of 50 formats with 50 modifiers it just needs to hold the 50 modifiers. I assume it grows automatically, but we can just see what the size is on amd and use that I guess.

Copy link
Collaborator

@kkartaltepe kkartaltepe Mar 30, 2024

Choose a reason for hiding this comment

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

spa_pod_dynamic_builder_clean is referenced in other places. I cant find any docs on spa_pod_dynamic_builder (not sure why i hoped it would exist), but can you confirm that we dont need to clean up after init.


if (obs_pw_stream->framerate.set) {
framerate = obs_pw_stream->framerate.fraction;
min_framerate = obs_pw_stream->framerate.fraction;
Expand Down Expand Up @@ -338,63 +341,65 @@ static inline struct spa_pod *build_format(obs_pipewire_stream *obs_pw_stream,
* The object type is important because it defines the properties that are
* acceptable. The id gives more context about what the object is meant to
* contain. In this case we enumerate supported formats. */
spa_pod_builder_push_object(b, &format_frame, SPA_TYPE_OBJECT_Format,
spa_pod_builder_push_object(&pod_builder.b, &format_frame,
SPA_TYPE_OBJECT_Format,
SPA_PARAM_EnumFormat);
/* add media type and media subtype properties */
spa_pod_builder_add(b, SPA_FORMAT_mediaType,
spa_pod_builder_add(&pod_builder.b, SPA_FORMAT_mediaType,
SPA_POD_Id(SPA_MEDIA_TYPE_video), 0);
spa_pod_builder_add(b, SPA_FORMAT_mediaSubtype,
spa_pod_builder_add(&pod_builder.b, SPA_FORMAT_mediaSubtype,
SPA_POD_Id(SPA_MEDIA_SUBTYPE_raw), 0);

/* formats */
spa_pod_builder_add(b, SPA_FORMAT_VIDEO_format, SPA_POD_Id(format), 0);
spa_pod_builder_add(&pod_builder.b, SPA_FORMAT_VIDEO_format,
SPA_POD_Id(format), 0);

/* modifier */
if (modifier_count > 0) {
struct spa_pod_frame modifier_frame;

/* build an enumeration of modifiers */
spa_pod_builder_prop(b, SPA_FORMAT_VIDEO_modifier,
spa_pod_builder_prop(&pod_builder.b, SPA_FORMAT_VIDEO_modifier,
SPA_POD_PROP_FLAG_MANDATORY |
SPA_POD_PROP_FLAG_DONT_FIXATE);

spa_pod_builder_push_choice(b, &modifier_frame, SPA_CHOICE_Enum,
0);
spa_pod_builder_push_choice(&pod_builder.b, &modifier_frame,
SPA_CHOICE_Enum, 0);

/* The first element of choice pods is the preferred value. Here
* we arbitrarily pick the first modifier as the preferred one.
*/
spa_pod_builder_long(b, modifiers[0]);
spa_pod_builder_long(&pod_builder.b, modifiers[0]);

/* modifiers from an array */
for (uint32_t i = 0; i < modifier_count; i++)
spa_pod_builder_long(b, modifiers[i]);
spa_pod_builder_long(&pod_builder.b, modifiers[i]);

spa_pod_builder_pop(b, &modifier_frame);
spa_pod_builder_pop(&pod_builder.b, &modifier_frame);
}
/* add size and framerate ranges */
spa_pod_builder_add(
b, SPA_FORMAT_VIDEO_size,
&pod_builder.b, SPA_FORMAT_VIDEO_size,
SPA_POD_CHOICE_RANGE_Rectangle(&resolution, &min_resolution,
&max_resolution),
SPA_FORMAT_VIDEO_framerate,
SPA_POD_CHOICE_RANGE_Fraction(&framerate, &min_framerate,
&max_framerate),
0);
return spa_pod_builder_pop(b, &format_frame);
return spa_pod_builder_pop(&pod_builder.b, &format_frame);
}

static bool build_format_params(obs_pipewire_stream *obs_pw_stream,
struct spa_pod_builder *pod_builder,
const struct spa_pod ***param_list,
uint32_t *n_params)
GPtrArray **out_params)
{
obs_pipewire *obs_pw = obs_pw_stream->obs_pw;
uint32_t params_count = 0;
g_autoptr(GPtrArray) params = NULL;

assert(out_params != NULL);

const struct spa_pod **params;
params = bzalloc(2 * obs_pw_stream->format_info.num *
sizeof(struct spa_pod *));
params = g_ptr_array_new_full(2 * obs_pw_stream->format_info.num *
Copy link
Collaborator

@kkartaltepe kkartaltepe Mar 30, 2024

Choose a reason for hiding this comment

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

If we want to use a dynamic array we should use the obs DARRAY to keep consistency with the rest of obs.

Copy link
Collaborator

@tytan652 tytan652 Mar 30, 2024

Choose a reason for hiding this comment

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

DARRAY in this kind of code path (not cold) should be avoided.

Copy link
Collaborator

@kkartaltepe kkartaltepe Mar 30, 2024

Choose a reason for hiding this comment

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

First of all negotiation is cold. And second we know the size so there arnt any performance concerns to begin with. Its just a matter of using the api we all know or using the gptr api.

sizeof(struct spa_pod *),
free);

if (!params) {
blog(LOG_ERROR,
Expand All @@ -409,22 +414,27 @@ static bool build_format_params(obs_pipewire_stream *obs_pw_stream,
if (obs_pw_stream->format_info.array[i].modifiers.num == 0) {
continue;
}
params[params_count++] = build_format(
obs_pw_stream, pod_builder,
obs_pw_stream->format_info.array[i].spa_format,
obs_pw_stream->format_info.array[i].modifiers.array,
obs_pw_stream->format_info.array[i].modifiers.num);
g_ptr_array_add(
params,
build_format(
obs_pw_stream,
obs_pw_stream->format_info.array[i].spa_format,
obs_pw_stream->format_info.array[i]
.modifiers.array,
obs_pw_stream->format_info.array[i]
.modifiers.num));
}

build_shm:
for (size_t i = 0; i < obs_pw_stream->format_info.num; i++) {
params[params_count++] = build_format(
obs_pw_stream, pod_builder,
obs_pw_stream->format_info.array[i].spa_format, NULL,
0);
g_ptr_array_add(
params,
build_format(
obs_pw_stream,
obs_pw_stream->format_info.array[i].spa_format,
NULL, 0));
}
*param_list = params;
*n_params = params_count;
*out_params = g_steal_pointer(&params);
return true;
}

Expand Down Expand Up @@ -568,26 +578,22 @@ static void renegotiate_format(void *data, uint64_t expirations)
UNUSED_PARAMETER(expirations);
obs_pipewire_stream *obs_pw_stream = (obs_pipewire_stream *)data;
obs_pipewire *obs_pw = obs_pw_stream->obs_pw;
const struct spa_pod **params = NULL;
g_autoptr(GPtrArray) params = NULL;

blog(LOG_INFO, "[pipewire] Renegotiating stream");

pw_thread_loop_lock(obs_pw->thread_loop);

uint8_t params_buffer[4096];
struct spa_pod_builder pod_builder =
SPA_POD_BUILDER_INIT(params_buffer, sizeof(params_buffer));
uint32_t n_params;
if (!build_format_params(obs_pw_stream, &pod_builder, &params,
&n_params)) {
if (!build_format_params(obs_pw_stream, &params)) {
teardown_pipewire(obs_pw);
pw_thread_loop_unlock(obs_pw->thread_loop);
return;
}

pw_stream_update_params(obs_pw_stream->stream, params, n_params);
pw_stream_update_params(obs_pw_stream->stream,
(const struct spa_pod **)params->pdata,
params->len);
pw_thread_loop_unlock(obs_pw->thread_loop);
bfree(params);
}

/* ------------------------------------------------- */
Expand Down Expand Up @@ -1203,11 +1209,8 @@ obs_pipewire_stream *obs_pipewire_connect_stream(
obs_pipewire *obs_pw, obs_source_t *source, int pipewire_node,
const struct obs_pipwire_connect_stream_info *connect_info)
{
struct spa_pod_builder pod_builder;
const struct spa_pod **params = NULL;
g_autoptr(GPtrArray) params = NULL;
obs_pipewire_stream *obs_pw_stream;
uint32_t n_params;
uint8_t params_buffer[4096];

assert(connect_info != NULL);

Expand Down Expand Up @@ -1246,27 +1249,22 @@ obs_pipewire_stream *obs_pipewire_connect_stream(
blog(LOG_INFO, "[pipewire] Created stream %p", obs_pw_stream->stream);

/* Stream parameters */
pod_builder =
SPA_POD_BUILDER_INIT(params_buffer, sizeof(params_buffer));

obs_get_video_info(&obs_pw_stream->video_info);

if (!build_format_params(obs_pw_stream, &pod_builder, &params,
&n_params)) {
if (!build_format_params(obs_pw_stream, &params)) {
pw_thread_loop_unlock(obs_pw->thread_loop);
bfree(obs_pw_stream);
return NULL;
}

pw_stream_connect(
obs_pw_stream->stream, PW_DIRECTION_INPUT, pipewire_node,
PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS, params,
n_params);
PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS,
(const struct spa_pod **)params->pdata, params->len);

blog(LOG_INFO, "[pipewire] Playing stream %p", obs_pw_stream->stream);

pw_thread_loop_unlock(obs_pw->thread_loop);
bfree(params);

g_ptr_array_add(obs_pw->streams, obs_pw_stream);

Expand Down