Re: [pulseaudio-discuss] [PATCH 3/4] move streams to new appeared sinks if they prefer these sinks
On 1/2/19 7:55 AM, Hui Wang wrote:
> On 2019/1/1 上午2:10, Tanu Kaskinen wrote:
>> On Mon, 2018-12-31 at 20:01 +0200, Tanu Kaskinen wrote:
>>> On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 63a3456e7..a2a390beb 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -722,6 +722,8 @@ void pa_sink_put(pa_sink* s) {
/* This function must be called after the
PA_CORE_HOOK_SINK_PUT hook,
* because module-switch-on-connect needs to know the old
default sink */
pa_core_update_default_sink(s->core, false);
+
+ pa_sink_bind_preferred_stream_to_a_sink(s);
}
/* Called from main context */
@@ -3919,3 +3921,32 @@ void
pa_sink_move_streams_from_oldsink_to_newsink(pa_sink *old_sink,
pa_sink *ne
return;
}
+
+void pa_sink_bind_preferred_stream_to_a_sink(pa_sink *s) {
>>> "Bind" is new terminology, and I'd like to avoid introducing new
>>> terminology if possible. Also, "preferred stream" as a term doesn't
>>> really make sense. So some better name for the function would be
>>> desirable, but I can't immediately think of any obvious names... The
>>> function is about moving streams to a sink that just became available.
>>> "Move" and "streams" should be included in the name... Maybe
>>> "pa_sink_move_streams_to_newly_available_sink()"? I also suggest moving
>>> it away from the pa_sink namespace, because the function operates on
>>> streams, not on a sink.
>> That last sentence can be objected to - the function takes the sink as
>> an argument, so it can be said to operate on the sink. But then the
>> verb should be something that the sink does, like "take" ("move" is
>> something that streams do). So pa_sink_take_streams()? Or
>> pa_sink_take_streams_that_prefer_it()? I don't know. My weak preference
>> at this point is pa_core_move_streams_to_newly_available_sink(). It's
>> quite descriptive, but unfortunately lacks the distinction that only
>> those streams that prefer the newly available sink are moved.
>>
> Understand what you mean here, will have a try to provide a new
> function name and address the rest comments of this patch.
>
> Thanks.
>
>
>
> ___
> pulseaudio-discuss mailing list
> pulseaudio-discuss@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Don't know the code at all, but from a purely English standpoint, how about
"pa_sink_move_streams_to_newly_available_preferred_sink()"
if it's not too impossibly long for a name.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 4/4] move stream when the active_port changes
On 2019/1/1 上午2:33, Tanu Kaskinen wrote:
On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
When the active port of a sink becomes unavailable, all streams from
that sink should be moved to the default sink.
When the active port of an existing sink changes state from
unavailable, all streams that have their "preferred_sink"
set to the new sink should be moved to the new sink.
Signed-off-by: Hui Wang
---
src/pulsecore/device-port.c | 16
src/pulsecore/sink.c| 13 +
src/pulsecore/sink.h| 1 +
3 files changed, 30 insertions(+)
diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
index 464c3f8a2..2604c9051 100644
--- a/src/pulsecore/device-port.c
+++ b/src/pulsecore/device-port.c
@@ -92,6 +92,7 @@ void pa_device_port_set_available(pa_device_port *p,
pa_available_t status) {
* be created before port objects, and then p->card could be non-NULL for
* the whole lifecycle of pa_device_port. */
if (p->card && p->card->linked) {
+ pa_sink *sink;
/* A sink or source whose active port is unavailable can't be the
* default sink/source, so port availability changes may affect the
* default sink/source choice. */
@@ -102,6 +103,21 @@ void pa_device_port_set_available(pa_device_port *p,
pa_available_t status) {
pa_subscription_post(p->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, p->card->index);
pa_hook_fire(>core->hooks[PA_CORE_HOOK_PORT_AVAILABLE_CHANGED], p);
+
+ sink = pa_sink_get_sink_from_device_port(p);
+ if (!sink)
+ return;
+ switch (p->direction) {
+case PA_DIRECTION_OUTPUT:
+ if (sink->active_port->available == PA_AVAILABLE_NO)
+ pa_sink_move_streams_from_oldsink_to_newsink(sink,
p->core->default_sink, false);
This logic isn't quite right. We should move streams away from the sink
only if the active port is the same that just changed its availability
status. If the current port is something other than the port that
changed, then we should ignore the change event.
I will change this part and make it follow the correct logic.
+ else
+ pa_sink_bind_preferred_stream_to_a_sink(sink);
+
+break;
+case PA_DIRECTION_INPUT:
+ break;
+ }
}
}
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index a2a390beb..9ebc18fa1 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -3950,3 +3950,16 @@ void pa_sink_bind_preferred_stream_to_a_sink(pa_sink *s)
{
pa_sink_input_move_to(si, s, false);
}
}
+
+pa_sink *pa_sink_get_sink_from_device_port(pa_device_port *p) {
This is a good helper function, but I think it would fit slightly
better in the pa_device_port namespace. So I'd rename it to
pa_device_port_get_sink().
OK, got it.
+pa_sink *rs = NULL;
+pa_sink *sink;
+uint32_t state;
+
+PA_IDXSET_FOREACH(sink, p->card->sinks, state)
+ if (p == pa_hashmap_get(sink->ports, p->name)) {
+ rs = sink;
+ break;
+ }
+return rs;
+}
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 24e4678b1..693344ac3 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -563,4 +563,5 @@ void pa_sink_set_reference_volume_direct(pa_sink *s, const
pa_cvolume *volume);
void pa_sink_move_streams_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *new_sink, bool from_user);
void pa_sink_bind_preferred_stream_to_a_sink(pa_sink *s);
+pa_sink *pa_sink_get_sink_from_device_port(pa_device_port *p);
#endif
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 3/4] move streams to new appeared sinks if they prefer these sinks
On 2019/1/1 上午2:10, Tanu Kaskinen wrote:
On Mon, 2018-12-31 at 20:01 +0200, Tanu Kaskinen wrote:
On Mon, 2018-11-05 at 09:47 +0800, Hui Wang wrote:
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 63a3456e7..a2a390beb 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -722,6 +722,8 @@ void pa_sink_put(pa_sink* s) {
/* This function must be called after the PA_CORE_HOOK_SINK_PUT hook,
* because module-switch-on-connect needs to know the old default sink */
pa_core_update_default_sink(s->core, false);
+
+pa_sink_bind_preferred_stream_to_a_sink(s);
}
/* Called from main context */
@@ -3919,3 +3921,32 @@ void
pa_sink_move_streams_from_oldsink_to_newsink(pa_sink *old_sink, pa_sink *ne
return;
}
+
+void pa_sink_bind_preferred_stream_to_a_sink(pa_sink *s) {
"Bind" is new terminology, and I'd like to avoid introducing new
terminology if possible. Also, "preferred stream" as a term doesn't
really make sense. So some better name for the function would be
desirable, but I can't immediately think of any obvious names... The
function is about moving streams to a sink that just became available.
"Move" and "streams" should be included in the name... Maybe
"pa_sink_move_streams_to_newly_available_sink()"? I also suggest moving
it away from the pa_sink namespace, because the function operates on
streams, not on a sink.
That last sentence can be objected to - the function takes the sink as
an argument, so it can be said to operate on the sink. But then the
verb should be something that the sink does, like "take" ("move" is
something that streams do). So pa_sink_take_streams()? Or
pa_sink_take_streams_that_prefer_it()? I don't know. My weak preference
at this point is pa_core_move_streams_to_newly_available_sink(). It's
quite descriptive, but unfortunately lacks the distinction that only
those streams that prefer the newly available sink are moved.
Understand what you mean here, will have a try to provide a new function
name and address the rest comments of this patch.
Thanks.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-mixer: Adding Jack of Front Headphone Front and Surround
On 2018/12/31 下午9:46, Tanu Kaskinen wrote: On Fri, 2018-12-21 at 11:05 +0800, Hui Wang wrote: We have 2 HP desktop models which have one front headphone Jack and one front headset Jack, in the Linux kernel, these two jacks are named "Front Headphone Front Jack" and "Front Headphone Surround Jack", now adding them into the pathes conf file. How do the kcontrols map to physical connectors? Is there only one physical connector or two? How do different kinds of devices (headphones, headsets, stand-alone mics) change the state of the two kcontrols? There are 2 physical connectors/jacks, one is the pure headphone jack, the other is the headset jack (headphone + mic). For the headphone jack, the kcontrols are: 'Front Headphone Surround Jack' and 'Headphone Playback Volume' index 1 For the headset jack, the kcontrols are: 'Front Headphone Front Jack', 'Headphone Playback Volume' index 0, 'Mic Jack' and 'Mic Boost Volume' If we plug a headphone in either of the two jacks, the 'Surround Jack' or 'Front Jack' will be true. If we plug a headset in the headset jack, the 'Front Jack' and 'Mic Jack' will be true. This is not the Dell machine, most Dell machines can't detect mic of headset, so we add headset-mic, headphone-mic for them. This HP machine can detect mic of headset jack, and it doesn't support stand-alone mics too. I think at least the headset-mic path configuration in your patch is incorrect, but I don't know how it should be configured before knowing how the two kcontrols behave. After reading your comments above, I think you are right. And I think I shouldn't touch analog-input-headset-mic.conf in the patch, since this is a HP machine, and the headset jack of this machine has capability to detect the mic on the headset and the kcontrols are "Mic Jack" and "Headphone Front Jack". It is different from the Dell machines. (A sidenote: it would be awesome to have some documentation that explains what combinations of headphone/headset/mic jacks exist in the wild and how those combinations map to physical connectors. The details of the headphone/headset/mic jack detection mess are impossible to remember.) It is not easy to fully understand this part, need to study it bit by bit. :-) Without this change, if users plug a headset or headphone, the path of headphone will not be activated, and users can't see headphones from UI like gnome-control-center. Signed-off-by: Hui Wang --- For alsa-info.txt of one of machines, please access: https://pastebin.ubuntu.com/p/CjCGySMpM5/ .../alsa/mixer/paths/analog-input-headset-mic.conf| 6 ++ .../alsa/mixer/paths/analog-output-headphones.conf| 6 ++ .../alsa/mixer/paths/analog-output-speaker-always.conf| 8 src/modules/alsa/mixer/paths/analog-output-speaker.conf | 8 4 files changed, 28 insertions(+) diff --git a/src/modules/alsa/mixer/paths/analog-input-headset-mic.conf b/src/modules/alsa/mixer/paths/analog-input-headset-mic.conf index 579db6bb7..bc687b6e4 100644 --- a/src/modules/alsa/mixer/paths/analog-input-headset-mic.conf +++ b/src/modules/alsa/mixer/paths/analog-input-headset-mic.conf @@ -35,6 +35,12 @@ state.plugged = unknown [Jack Front Headphone] state.plugged = unknown +[Jack Front Headphone Front] +state.plugged = unknown + +[Jack Front Headphone Surround] +state.plugged = unknown + [Jack Headphone Mic] state.plugged = unknown diff --git a/src/modules/alsa/mixer/paths/analog-output-headphones.conf b/src/modules/alsa/mixer/paths/analog-output-headphones.conf index d2147c50f..2d751607c 100644 --- a/src/modules/alsa/mixer/paths/analog-output-headphones.conf +++ b/src/modules/alsa/mixer/paths/analog-output-headphones.conf @@ -40,6 +40,12 @@ required-any = any state.plugged = unknown state.unplugged = unknown +[Jack Front Headphone Front] +required-any = any + +[Jack Front Headphone Surround] +required-any = any + [Jack Headphone] required-any = any diff --git a/src/modules/alsa/mixer/paths/analog-output-speaker-always.conf b/src/modules/alsa/mixer/paths/analog-output-speaker-always.conf index 71f356dce..c9374e471 100644 --- a/src/modules/alsa/mixer/paths/analog-output-speaker-always.conf +++ b/src/modules/alsa/mixer/paths/analog-output-speaker-always.conf @@ -33,6 +33,14 @@ state.unplugged = unknown state.plugged = no state.unplugged = unknown +[Jack Front Headphone Front] +state.plugged = no +state.unplugged = unknown + +[Jack Front Headphone Surround] +state.plugged = no +state.unplugged = unknown + [Jack Line Out] state.plugged = no state.unplugged = unknown diff --git a/src/modules/alsa/mixer/paths/analog-output-speaker.conf b/src/modules/alsa/mixer/paths/analog-output-speaker.conf index 9f4dac414..536630d0a 100644 --- a/src/modules/alsa/mixer/paths/analog-output-speaker.conf +++ b/src/modules/alsa/mixer/paths/analog-output-speaker.conf @@ -36,6 +36,14 @@ state.unplugged = unknown state.plugged = no
