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

Add target.object to capture.props in exemplary pipewire config #162

Open
jkhsjdhjs opened this issue Jan 12, 2023 · 5 comments
Open

Add target.object to capture.props in exemplary pipewire config #162

jkhsjdhjs opened this issue Jan 12, 2023 · 5 comments

Comments

@jkhsjdhjs
Copy link

jkhsjdhjs commented Jan 12, 2023

The readme currently gives an exemplary pipewire config using the filter-chain module, which really helps when setting this up. The filter chain simply listens on the default source, which might not be the source you actually want it to use.
To get around this, you can specify a target source as input for the filter-chain (thanks to @licentiapoetica for figuring this out):

capture.props = {
    node.name =  "capture.rnnoise_source"
    node.passive = true
    audio.rate = 48000
    target.object = "alsa_input.usb-Logitech_Logitech_G930_Headset-00.mono-fallback"
}

I propose to either add this to the example or at least note it somewhere, so that other users don't face the same issue as I did.

See also: #162 (comment)

@pallaswept
Copy link
Contributor

I've just been setting my pipewire up like this, and thought I should mention that I'd noticed node.target is deprecated and target.object is the new way to do this, so you'd want

capture.props = {
    node.name =  "capture.rnnoise_source"
    node.passive = true
    audio.rate = 48000
    target.object = "alsa_input.usb-Logitech_Logitech_G930_Headset-00.mono-fallback"
}

The only issue I can see with this is that if you set it up like this then you can't dynamically switch the default source, which might actually be desirable (ie you have multiple mics and want to be able to toggle which one goes to a single instance of rnnoise, thus avoiding the need to set up multiple rnnoise instances)

@jkhsjdhjs
Copy link
Author

Thanks for the info! Do you happen to have a source on the deprecation?

The only issue I can see with this is that if you set it up like this then you can't dynamically switch the default source, which might actually be desirable (ie you have multiple mics and want to be able to toggle which one goes to a single instance of rnnoise, thus avoiding the need to set up multiple rnnoise instances)

That's true, or at least it's not as easy anymore. You can still do this via tools like qpwgraph or pw-cli.

@pallaswept
Copy link
Contributor

Thanks for the info! Do you happen to have a source on the deprecation?

Sure, it's in the official docs https://docs.pipewire.org/group__pw__keys.html#ga33441b02b8e14e14451272c2b497a5c9
There's more talk of it elsewhere where they explain why the new way is better because it allows you to use a more unique target identifier. I really wish the search engine on those pages actually worked, sorry I couldn't find it right away. (IIRC the old way can take object .name or object .id, and the new way takes object .name or object .serial? I might be mis-remembering here but I'm pretty sure that was it.

fwiw, node.target still does actually work, but it's pretty clear from the docs that it won't be that way forever and they encourage the use of the new way in a few places.

That's true, or at least it's not as easy anymore. You can still do this via tools like qpwgraph or pw-cli.

Yeh and you could always set up the input device that way too, but it's not a nice way to go about it which is why we like target designators :). My point is that if this hard-wiring a mic to the rnnoise input is your intended use case (like it is for me and obviously you) then this seems like a good idea... but it doesn't just add something, it takes something away, and for anyone with the other use-case as their intention, this change would cause things to not work. As in, this fixes a problem for some people and creates a problem for others. And now that I'm thinking about it, there may well be people who want both approaches... So either way you win some and you lose some.

Perhaps it would be best to give both alternatives, and an explanation of the behaviour in the comments.

@jkhsjdhjs
Copy link
Author

Sure, it's in the official docs https://docs.pipewire.org/group__pw__keys.html#ga33441b02b8e14e14451272c2b497a5c9 There's more talk of it elsewhere where they explain why the new way is better because it allows you to use a more unique target identifier. I really wish the search engine on those pages actually worked, sorry I couldn't find it right away. (IIRC the old way can take object .name or object .id, and the new way takes object .name or object .serial? I might be mis-remembering here but I'm pretty sure that was it.

Ah, interesting! Thanks for linking that. I've always found the pipewire docs hard to navigate and sadly wasn't able to locate this myself. I wonder why pipewire doesn't print a deprecation warning when using node.target though. Anyway, I just switched my configs to target.object and it works fine!

Yeh and you could always set up the input device that way too, but it's not a nice way to go about it which is why we like target designators :). My point is that if this hard-wiring a mic to the rnnoise input is your intended use case (like it is for me and obviously you) then this seems like a good idea... but it doesn't just add something, it takes something away, and for anyone with the other use-case as their intention, this change would cause things to not work. As in, this fixes a problem for some people and creates a problem for others. And now that I'm thinking about it, there may well be people who want both approaches... So either way you win some and you lose some.

Perhaps it would be best to give both alternatives, and an explanation of the behaviour in the comments.

Yeah, I get your point, and I agree. If this is added the (dis-)advantages and usecases of either method should be stated.

@jkhsjdhjs jkhsjdhjs changed the title Add node.target to capture.props in exemplary pipewire config Add target.object to capture.props in exemplary pipewire config Aug 14, 2023
@pallaswept
Copy link
Contributor

pallaswept commented Aug 14, 2023

I've always found the pipewire docs hard to navigate

You are not alone. A working search engine would go a long way. I just stumbled across this when I was setting up some node-targets myself.

I wonder why pipewire doesn't print a deprecation warning

Good point, I'm about to file an issue that it doesn't, because it should. There we go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants