wwmm
`target.object` is not enough
EasyEffects Version
7.1.7
What package are you using?
Other (specify below)
Distribution
debian sid
Describe the bug
Follow up for https://github.com/wwmm/easyeffects/issues/3188 I suspect this is known, but it's unfortunate.
Consider the following PW graph:
pw-dot.dot.txt pw-dump.log.txt
However, once EE is started, we get:
pw-dot.new.dot.txt
pw-dump.log.new.txt
IOW it intercepted a stream that outputs to something other than the output configured in EE,
and outputted it to the same output it was originally going and not the one configured in EE.
While the former, i'm guessing is because there is no target.object for the Google Chrome node,
the latter seems like a bug?
But, i feel like there should be a toggle "no, but really, only touch streams that are known to go to the selected output",
if the target.object really is the only possible method to detect that (as opposed to the actual graph).
Expected Behavior
No response
Debug Log
No response
Additional Information
No response
Additionally, if i uncheck "Process all output streams", and manually click "Enable" for a specific stream in Players tab, it seems like that data is not saved in presets?
While the former, i'm guessing is because there is no target.object for the Google Chrome node, the latter seems like a bug?
Clients are not forced to set a target node. So it is not a bug. I doubt Chrome sets one. So it makes sense it was moved. But I thought that those loopback devices were setting a target.
But, i feel like there should be a toggle "no, but really, only touch streams that are known to go to the >selected output", if the target.object really is the only possible method to detect that (as opposed to the actual graph).
A lot of information has to come for the whole graph to be known. And they do not come at the same time. The nodes have their own signal. And the ports and links have separated signals too. All of them emitted when PipeWire considers it has to inform about them. And it does not say "Hey! I am going to send you more information in a few moments". Maybe it comes. Maybe it doesn't. For an app that only shows the graph this is not a problem. They just have to draw the graph again if something new comes. This approach is not going to work for us.
it seems like that data is not saved in presets?
We do not save this information. Mostly because there was never a reason to do things this way. And it will probably require a careful rewrite of the whole code used to handle streams do something like this now.
Thank you!
While the former, i'm guessing is because there is no target.object for the Google Chrome node, the latter seems like a bug?
Clients are not forced to set a target node. So it is not a bug. I doubt Chrome sets one. So it makes sense it was moved.
(1)
But I thought that those loopback devices were setting a target.
But, i feel like there should be a toggle "no, but really, only touch streams that are known to go to the >selected output", if the target.object really is the only possible method to detect that (as opposed to the actual graph).
A lot of information has to come for the whole graph to be known. And they do not come at the same time. The nodes have their own signal. And the ports and links have separated signals too. All of them emitted when PipeWire considers it has to inform about them. And it does not say "Hey! I am going to send you more information in a few moments". Maybe it comes. Maybe it doesn't. For an app that only shows the graph this is not a problem. They just have to draw the graph again if something new comes. This approach is not going to work for us.
Note that there was a first part of the question there, namely:
But, i feel like there should be a toggle "no, but really, only touch streams that are known to go to the >selected output",
... because combined with (1), it seems like there's really no way to actually achieve the desired result of only intercepting things that are known to go to the specific output, especially given that:
it seems like that data is not saved in presets?
We do not save this information. Mostly because there was never a reason to do things this way. And it will probably require a careful rewrite of the whole code used to handle streams do something like this now.
Having the toggle in settings "intercept only streams that are known to go to the specified output" vs "intercept streams unless they are known NOT to go to the specified output" seems less invasive that the forementioned preset surgery. Not sure if that can be combined with the existing "Process all streams" / Output selection options.
Again, thank you.
it seems like there's really no way to actually achieve the desired result of only intercepting things that are known to go to the specific output
Unfortunately I am not aware of any straightforward way to get this information. The only possibility that comes to my mind would be the most annoying of all. Waiting for the signals about the link objects to come. Then look at the link input and output nodes to see if the link is related to the player node we received before and if the other end of the link is connected or not to our virtual device. Not only a deep rewrite is required for this but the whole process involved in handling a stream increases in complexity.
Not sure if that can be combined with the existing "Process all streams" / Output selection options.
I am not sure. It feels like completely changing the approach to some kind of "allowed list" instead of blocklist would be required.
I'm not quite following. Let's look at the actual code: https://github.com/wwmm/easyeffects/blob/9b708d4/src/pipe_manager.cpp#L341-L367
The only thing i'm saying is that it should simply be
if (nd->nd_info->media_class == tags::pipewire::media_class::output_stream) {
if(!pm->output_device.name.empty()) {
if (const auto* target_object = spa_dict_lookup(info->props, PW_KEY_TARGET_OBJECT)) {
/*
target.object can a name or serial number:
https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/src/pipewire/keys.h#L334
*/
uint64_t serial = SPA_ID_INVALID;
util::str_to_num(target_object, serial);
if (target_object != pm->output_device.name || target_object != pm->ee_sink_node.name) {
ignore_output_stream = true;
} else if (serial != SPA_ID_INVALID &&
(serial != pm->output_device.serial && serial != pm->ee_sink_node.serial)) {
ignore_output_stream = true;
}
}
} else {
if(should_ignore_streams_with_unknown_output_devices) # !!!!
ignore_output_stream = true; # !!!!
}
if (ignore_output_stream) {
util::debug("The output stream " + nd->nd_info->name +
" does not have as target the same output device used as EE: " + pm->output_device.name +
"\n The user wants it to play to device " + target_object + ". We will ignore this stream.");
remove_node = true;
}
}
... where should_ignore_streams_with_unknown_output_devices is the boolean from the new config option.
where should_ignore_streams_with_unknown_output_devices is the boolean from the new config option.
I do not see how this is going to work. The variable pm->output_device.name is the device EasyEffects should be outputting to. If this is unknown at all times then there is no audio. The reason why I added the empty check there is because there is a chance we have to deal with the player stream before the output device variable is initialized.
The information comes from PipeWire asynchronously. Usually we receive streams after the signal about devices has come. But there is no guarantee. In case we do not know the output device yet we assume the stream should not be ignored because there is nothing better that can be done at this point.
Maybe you were thinking about ignoring streams that do not set a target. But these are the majority. We would be essentially ignoring almost all valid streams.
Thank you!
Maybe you were thinking about ignoring streams that do not set a target. But these are the majority. We would be essentially ignoring almost all valid streams.
Yes, that is indeed what i was thinking about, and i thought that was obvious from previous comments.
But indeed, if the detection is based solely on the target.object, which is more often absent than not,
this behavior would need to be opt-in.