From 9e9ac22e82d553208e5a3e835ec4901089cbea6b Mon Sep 17 00:00:00 2001 From: Lucas Schwiderski Date: Mon, 23 May 2022 16:32:04 +0200 Subject: [PATCH] fix(context): Fix setting volume The volume object needs to be kept in memory until the operation has finished. --- src/lua_libpulse_glib/callback.c | 7 +- src/lua_libpulse_glib/introspection.c | 124 +++++++++++++++++++------- 2 files changed, 98 insertions(+), 33 deletions(-) diff --git a/src/lua_libpulse_glib/callback.c b/src/lua_libpulse_glib/callback.c index 108a1d8..24a726a 100644 --- a/src/lua_libpulse_glib/callback.c +++ b/src/lua_libpulse_glib/callback.c @@ -44,11 +44,16 @@ void free_lua_callback(simple_callback_data* data) { free(data); } - +// When preparing a thread for this callback, the function must be at index `1`. +// Values on the stack after that will be ignored. This allows adding userdata and other values +// for the purpose of memory management, to keep them alive until the callback has been called. void success_callback(pa_context* c, int success, void* userdata) { simple_callback_data* data = (simple_callback_data*) userdata; lua_State* L = data->L; + // Copy the callback function to a position from where it can be called. + // There may be other values on the stack for memory management. + lua_pushvalue(L, 1); lua_pushnil(L); lua_pushboolean(L, success); lua_call(L, 2, 0); diff --git a/src/lua_libpulse_glib/introspection.c b/src/lua_libpulse_glib/introspection.c index 33d13f6..d404dd9 100644 --- a/src/lua_libpulse_glib/introspection.c +++ b/src/lua_libpulse_glib/introspection.c @@ -195,9 +195,23 @@ int context_set_sink_volume_by_name(lua_State* L) { simple_callback_data* data = prepare_lua_callback(L, 4); const char* name = luaL_checkstring(L, 2); - pa_cvolume* volume = volume_from_lua(L, 3); - pa_operation* op = pa_context_set_sink_volume_by_name(ctx->context, name, volume, success_callback, data); + if (lua_istable(L, 3)) { + pa_cvolume* volume = volume_from_lua(L, 3); + volume_to_lua(L, volume); + pa_xfree((void*) volume); + } else { + lua_pushvalue(L, 3); + } + + volume_t* vol = luaL_checkudata(L, -1, LUA_PA_VOLUME); + + // Move the volume userdata to the callback's thread. It needs to be kept alive + // until the callback has run. + lua_xmove(L, data->L, 1); + + pa_operation* op = pa_context_set_sink_volume_by_name(ctx->context, name, &vol->inner, success_callback, data); + if (op == NULL) { int error = pa_context_errno(ctx->context); lua_pushvalue(L, 2); @@ -205,17 +219,13 @@ int context_set_sink_volume_by_name(lua_State* L) { lua_call(L, 1, 0); } - // TODO: Is this too early? - // It's probably fine when the operation failed, but in the other case, this might have to be moved to the callback. - // But once this is userdata, I can simply put it onto the callback's thread stack and have Lua garbage collect it. - pa_xfree((void*) volume); - return 0; } int context_set_sink_volume_by_index(lua_State* L) { lua_pa_context* ctx = luaL_checkudata(L, 1, LUA_PA_CONTEXT); + lua_Integer index = luaL_checkinteger(L, 2); if (index < 1) { return luaL_error(L, "Sink index out of bounds. Got %d", index); @@ -229,10 +239,23 @@ int context_set_sink_volume_by_index(lua_State* L) { } simple_callback_data* data = prepare_lua_callback(L, 4); - pa_cvolume* volume = volume_from_lua(L, 3); + + if (lua_istable(L, 3)) { + pa_cvolume* volume = volume_from_lua(L, 3); + volume_to_lua(L, volume); + pa_xfree((void*) volume); + } else { + lua_pushvalue(L, 3); + } + + volume_t* vol = luaL_checkudata(L, -1, LUA_PA_VOLUME); + + // Move the volume userdata to the callback's thread. It needs to be kept alive + // until the callback has run. + lua_xmove(L, data->L, 1); pa_operation* op = - pa_context_set_sink_volume_by_index(ctx->context, (uint32_t) index - 1, volume, success_callback, data); + pa_context_set_sink_volume_by_index(ctx->context, (uint32_t) index - 1, &vol->inner, success_callback, data); if (op == NULL) { int error = pa_context_errno(ctx->context); lua_pushvalue(L, 2); @@ -240,11 +263,6 @@ int context_set_sink_volume_by_index(lua_State* L) { lua_call(L, 1, 0); } - // TODO: Is this too early? - // It's probably fine when the operation failed, but in the other case, this might have to be moved to the callback. - // But once this is userdata, I can simply put it onto the callback's thread stack and have Lua garbage collect it. - pa_xfree((void*) volume); - return 0; } @@ -569,9 +587,22 @@ int context_set_source_volume_by_name(lua_State* L) { simple_callback_data* data = prepare_lua_callback(L, 4); const char* name = luaL_checkstring(L, 2); - pa_cvolume* volume = volume_from_lua(L, 3); - pa_operation* op = pa_context_set_source_volume_by_name(ctx->context, name, volume, success_callback, data); + if (lua_istable(L, 3)) { + pa_cvolume* volume = volume_from_lua(L, 3); + volume_to_lua(L, volume); + pa_xfree((void*) volume); + } else { + lua_pushvalue(L, 3); + } + + volume_t* vol = luaL_checkudata(L, -1, LUA_PA_VOLUME); + + // Move the volume userdata to the callback's thread. It needs to be kept alive + // until the callback has run. + lua_xmove(L, data->L, 1); + + pa_operation* op = pa_context_set_source_volume_by_name(ctx->context, name, &vol->inner, success_callback, data); if (op == NULL) { int error = pa_context_errno(ctx->context); lua_pushvalue(L, 2); @@ -579,11 +610,6 @@ int context_set_source_volume_by_name(lua_State* L) { lua_call(L, 1, 0); } - // TODO: Is this too early? - // It's probably fine when the operation failed, but in the other case, this might have to be moved to the callback. - // But once this is userdata, I can simply put it onto the callback's thread stack and have Lua garbage collect it. - pa_xfree((void*) volume); - return 0; } @@ -603,10 +629,23 @@ int context_set_source_volume_by_index(lua_State* L) { } simple_callback_data* data = prepare_lua_callback(L, 4); - pa_cvolume* volume = volume_from_lua(L, 3); + + if (lua_istable(L, 3)) { + pa_cvolume* volume = volume_from_lua(L, 3); + volume_to_lua(L, volume); + pa_xfree((void*) volume); + } else { + lua_pushvalue(L, 3); + } + + volume_t* vol = luaL_checkudata(L, -1, LUA_PA_VOLUME); + + // Move the volume userdata to the callback's thread. It needs to be kept alive + // until the callback has run. + lua_xmove(L, data->L, 1); pa_operation* op = - pa_context_set_source_volume_by_index(ctx->context, (uint32_t) index - 1, volume, success_callback, data); + pa_context_set_source_volume_by_index(ctx->context, (uint32_t) index - 1, &vol->inner, success_callback, data); if (op == NULL) { int error = pa_context_errno(ctx->context); lua_pushvalue(L, 2); @@ -614,11 +653,6 @@ int context_set_source_volume_by_index(lua_State* L) { lua_call(L, 1, 0); } - // TODO: Is this too early? - // It's probably fine when the operation failed, but in the other case, this might have to be moved to the callback. - // But once this is userdata, I can simply put it onto the callback's thread stack and have Lua garbage collect it. - pa_xfree((void*) volume); - return 0; } @@ -983,12 +1017,25 @@ int context_set_sink_input_volume(lua_State* L) { if (index < 1) { return luaL_error(L, "Sink input index out of bounds. Got %d", index); } - pa_cvolume* volume = volume_from_lua(L, 3); simple_callback_data* data = prepare_lua_callback(L, 4); + if (lua_istable(L, 3)) { + pa_cvolume* volume = volume_from_lua(L, 3); + volume_to_lua(L, volume); + pa_xfree((void*) volume); + } else { + lua_pushvalue(L, 3); + } + + volume_t* vol = luaL_checkudata(L, -1, LUA_PA_VOLUME); + + // Move the volume userdata to the callback's thread. It needs to be kept alive + // until the callback has run. + lua_xmove(L, data->L, 1); + pa_operation* op = - pa_context_set_sink_input_volume(ctx->context, (uint32_t) index - 1, volume, success_callback, data); + pa_context_set_sink_input_volume(ctx->context, (uint32_t) index - 1, &vol->inner, success_callback, data); if (op == NULL) { int error = pa_context_errno(ctx->context); lua_pushvalue(L, 2); @@ -1243,12 +1290,25 @@ int context_set_source_output_volume(lua_State* L) { if (index < 1) { return luaL_error(L, "Source output index out of bounds. Got %d", index); } - pa_cvolume* volume = volume_from_lua(L, 3); simple_callback_data* data = prepare_lua_callback(L, 4); + if (lua_istable(L, 3)) { + pa_cvolume* volume = volume_from_lua(L, 3); + volume_to_lua(L, volume); + pa_xfree((void*) volume); + } else { + lua_pushvalue(L, 3); + } + + volume_t* vol = luaL_checkudata(L, -1, LUA_PA_VOLUME); + + // Move the volume userdata to the callback's thread. It needs to be kept alive + // until the callback has run. + lua_xmove(L, data->L, 1); + pa_operation* op = - pa_context_set_source_output_volume(ctx->context, (uint32_t) index - 1, volume, success_callback, data); + pa_context_set_source_output_volume(ctx->context, (uint32_t) index - 1, &vol->inner, success_callback, data); if (op == NULL) { int error = pa_context_errno(ctx->context); lua_pushvalue(L, 2);