arcnmx / wireplumber.rs

wireplumber rust bindings
https://arcnmx.github.io/wireplumber.rs/main/wireplumber/
MIT License
21 stars 3 forks source link

metadata changed event key must be optional #33

Closed saivert closed 11 months ago

saivert commented 11 months ago

changed event is emitted with key == NULL when a metadata item is removed.

see https://gitlab.freedesktop.org/pipewire/wireplumber/-/blob/ca58c68ef988e57f5bec232fe6c9ab86d5f0ea19/lib/wp/metadata.c#L218

Though I'm moving to using closure_local macro which makes me get at the event arguments directly instead of via GValue.

let changed_closure = closure_local!(@watch widget =>
                        move |_obj: &wp::pw::Metadata, id: u32, key: Option<String>, _type: Option<String>, _value: Option<String>| {
                        let key = key.unwrap_or_default();
                        if id == boundid && key.contains("target.") {
                            wp::log::info!("metadata changed handler id: {boundid} {key:?} {_value:?}!");
                            widget.update_output_device_dropdown();
                        }
                    });
                    metadata.connect_closure("changed", false, changed_closure);
arcnmx commented 11 months ago

This sounds like a duplicate of #30, was it not fixed by 95c55499881dea199a1c62f755f35b9e7c1b2fb0? What code reproduces the issue?

saivert commented 11 months ago

Yes, it was partly fixed then but key also has to be Optional.

Not sure why you need a reproducer but this is enough:

metadata.connect_changed(|obj, subject, key, type_, value| {
                        wp::log::info!("metadata changed handler id: {subject} {key:?} {type_:?} {value:?}!");
                    });

Then remove the metadata using command line: pw-metadata -d 59

And you have a crash:

thread 'main' panicked at 'assertion failed: !ptr.is_null()', /home/saivert/Projects/pwvucontrol/builddir/cargo-home/registry/src/index.crates.io-6f17d22bba15001f/glib-0.18.1/src/gstring.rs:1973:9
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic
   3: <glib::gstring::GString as glib::translate::FromGlibPtrBorrow<*const u8>>::from_glib_borrow
             at /home/saivert/Projects/pwvucontrol/builddir/cargo-home/registry/src/index.crates.io-6f17d22bba15001f/glib-0.18.1/src/gstring.rs:1973:9
   4: glib::translate::from_glib_borrow
             at /home/saivert/Projects/pwvucontrol/builddir/cargo-home/registry/src/index.crates.io-6f17d22bba15001f/glib-0.18.1/src/translate.rs:1640:5
   5: <glib::gstring::GString as glib::translate::FromGlibPtrBorrow<*mut i8>>::from_glib_borrow
             at /home/saivert/Projects/pwvucontrol/builddir/cargo-home/registry/src/index.crates.io-6f17d22bba15001f/glib-0.18.1/src/gstring.rs:2005:9
   6: wireplumber::auto::metadata::MetadataExt::connect_changed::changed_trampoline
             at /home/saivert/Projects/pwvucontrol/builddir/cargo-home/git/checkouts/wireplumber.rs-cb0425781db64edb/341b0c4/src/auto/metadata.rs:74:76
   7: <unknown>
   8: <unknown>
   9: ffi_call
  10: g_cclosure_marshal_generic
  11: g_closure_invoke
  12: <unknown>
  13: g_signal_emit_valist
  14: g_signal_emit
  15: <unknown>
  16: <unknown>
  17: <unknown>
  18: <unknown>
  19: <unknown>
  20: <unknown>
  21: g_main_context_dispatch
  22: <unknown>
  23: g_main_context_iteration
  24: g_application_run
  25: gio::application::ApplicationExtManual::run_with_args
             at /home/saivert/Projects/pwvucontrol/builddir/cargo-home/registry/src/index.crates.io-6f17d22bba15001f/gio-0.18.1/src/application.rs:29:13
  26: gio::application::ApplicationExtManual::run
             at /home/saivert/Projects/pwvucontrol/builddir/cargo-home/registry/src/index.crates.io-6f17d22bba15001f/gio-0.18.1/src/application.rs:22:9
  27: pwvucontrol::main
             at /home/saivert/Projects/pwvucontrol/src/main.rs:118:5
  28: core::ops::function::FnOnce::call_once
             at /builddir/build/BUILD/rustc-1.72.0-src/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
arcnmx commented 11 months ago

Gotcha, p0 was missed in that fix, thanks for the clarification!