Re: [pulseaudio-discuss] [PATCH] loopback: underrun_protection argument
On 02/15/2018 12:43 PM, Georg Chini wrote: > On 15.02.2018 10:31, Raman Shishniou wrote: >> On 02/15/2018 10:20 AM, Georg Chini wrote: >>> On 14.02.2018 22:51, Raman Shishniou wrote: On 02/14/2018 07:20 PM, Georg Chini wrote: > On 14.02.2018 15:25, Raman Shyshniou wrote: >> This patch adds a underrun_protection argument to >> control underrun protection algorithm. Disabling >> protection will keep loopback latency regardless >> of underruns. > Again I do not understand the motivation of the patch. > In what situations are you seeing so many underruns and > still want to keep the original configured latency value? > Audio will be very bad in that case. > All situations where where latency is more important than data integrity. Voice over IP (telephony) for example, receiving audio data using network by UDP/RTP. Any data loss leads to underruns in loopback module. >>> This is not correct. It will only lead to underruns, if module-loopback runs >>> out of data. So if you buffer enough data, missing packets in the voice >>> stream would just appear to be small sample rate variations from the >>> perspective of the loopback module. Because the loopback module >>> does some adaptive re-sampling, these variations are no problem. >>> >>> Maybe this happens for you because module-pipe-source has no buffer >>> at all and simply passes the values through whenever data arrives. >>> With missing data packets, this can surely lead to underruns on the >>> source side which are passed on to the loopback module. >>> Perhaps you should implement some buffering in pipe-source? >>> >>> I don't see the point of your change. If you are seeing massive underruns, >>> audio quality will be really bad and just sticking to the configured latency >>> will not improve the situation. For me, the permanent occurrence of >>> underruns shows that there is something wrong with your setup in the first >>> place. The better idea is to correct the audio chain, so that no (or only >>> very >>> few) underruns happen. >> I Agree. For live streaming or internet radio I can buffer more data up to >> several seconds (or just use tcp). But not for telephony, where 10-20 >> underruns per hour is acceptable, but latency more than 50-100ms is not. >> Loopback module increases latency permanently. There is no way to decrease >> it without unload and load again. > > How about a max_latency_msec argument in the sense that the logic > will not be completely disabled, but module-loopback will not try to > increase the latency above max_latency_msec (if specified) even if > underruns occur? > I can see that, it makes sense to me. I'll make the patch. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] loopback: underrun_protection argument
On 15.02.2018 10:31, Raman Shishniou wrote: On 02/15/2018 10:20 AM, Georg Chini wrote: On 14.02.2018 22:51, Raman Shishniou wrote: On 02/14/2018 07:20 PM, Georg Chini wrote: On 14.02.2018 15:25, Raman Shyshniou wrote: This patch adds a underrun_protection argument to control underrun protection algorithm. Disabling protection will keep loopback latency regardless of underruns. Again I do not understand the motivation of the patch. In what situations are you seeing so many underruns and still want to keep the original configured latency value? Audio will be very bad in that case. All situations where where latency is more important than data integrity. Voice over IP (telephony) for example, receiving audio data using network by UDP/RTP. Any data loss leads to underruns in loopback module. This is not correct. It will only lead to underruns, if module-loopback runs out of data. So if you buffer enough data, missing packets in the voice stream would just appear to be small sample rate variations from the perspective of the loopback module. Because the loopback module does some adaptive re-sampling, these variations are no problem. Maybe this happens for you because module-pipe-source has no buffer at all and simply passes the values through whenever data arrives. With missing data packets, this can surely lead to underruns on the source side which are passed on to the loopback module. Perhaps you should implement some buffering in pipe-source? I don't see the point of your change. If you are seeing massive underruns, audio quality will be really bad and just sticking to the configured latency will not improve the situation. For me, the permanent occurrence of underruns shows that there is something wrong with your setup in the first place. The better idea is to correct the audio chain, so that no (or only very few) underruns happen. I Agree. For live streaming or internet radio I can buffer more data up to several seconds (or just use tcp). But not for telephony, where 10-20 underruns per hour is acceptable, but latency more than 50-100ms is not. Loopback module increases latency permanently. There is no way to decrease it without unload and load again. How about a max_latency_msec argument in the sense that the logic will not be completely disabled, but module-loopback will not try to increase the latency above max_latency_msec (if specified) even if underruns occur? ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] loopback: underrun_protection argument
On 02/15/2018 10:20 AM, Georg Chini wrote: > On 14.02.2018 22:51, Raman Shishniou wrote: >> On 02/14/2018 07:20 PM, Georg Chini wrote: >>> On 14.02.2018 15:25, Raman Shyshniou wrote: This patch adds a underrun_protection argument to control underrun protection algorithm. Disabling protection will keep loopback latency regardless of underruns. >>> Again I do not understand the motivation of the patch. >>> In what situations are you seeing so many underruns and >>> still want to keep the original configured latency value? >>> Audio will be very bad in that case. >>> >> All situations where where latency is more important than data integrity. >> Voice over IP (telephony) for example, receiving audio data using network >> by UDP/RTP. Any data loss leads to underruns in loopback module. > > This is not correct. It will only lead to underruns, if module-loopback runs > out of data. So if you buffer enough data, missing packets in the voice > stream would just appear to be small sample rate variations from the > perspective of the loopback module. Because the loopback module > does some adaptive re-sampling, these variations are no problem. > > Maybe this happens for you because module-pipe-source has no buffer > at all and simply passes the values through whenever data arrives. > With missing data packets, this can surely lead to underruns on the > source side which are passed on to the loopback module. > Perhaps you should implement some buffering in pipe-source? > > I don't see the point of your change. If you are seeing massive underruns, > audio quality will be really bad and just sticking to the configured latency > will not improve the situation. For me, the permanent occurrence of > underruns shows that there is something wrong with your setup in the first > place. The better idea is to correct the audio chain, so that no (or only very > few) underruns happen. I Agree. For live streaming or internet radio I can buffer more data up to several seconds (or just use tcp). But not for telephony, where 10-20 underruns per hour is acceptable, but latency more than 50-100ms is not. Loopback module increases latency permanently. There is no way to decrease it without unload and load again. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] loopback: underrun_protection argument
On 14.02.2018 22:51, Raman Shishniou wrote: On 02/14/2018 07:20 PM, Georg Chini wrote: On 14.02.2018 15:25, Raman Shyshniou wrote: This patch adds a underrun_protection argument to control underrun protection algorithm. Disabling protection will keep loopback latency regardless of underruns. Again I do not understand the motivation of the patch. In what situations are you seeing so many underruns and still want to keep the original configured latency value? Audio will be very bad in that case. All situations where where latency is more important than data integrity. Voice over IP (telephony) for example, receiving audio data using network by UDP/RTP. Any data loss leads to underruns in loopback module. This is not correct. It will only lead to underruns, if module-loopback runs out of data. So if you buffer enough data, missing packets in the voice stream would just appear to be small sample rate variations from the perspective of the loopback module. Because the loopback module does some adaptive re-sampling, these variations are no problem. Maybe this happens for you because module-pipe-source has no buffer at all and simply passes the values through whenever data arrives. With missing data packets, this can surely lead to underruns on the source side which are passed on to the loopback module. Perhaps you should implement some buffering in pipe-source? I don't see the point of your change. If you are seeing massive underruns, audio quality will be really bad and just sticking to the configured latency will not improve the situation. For me, the permanent occurrence of underruns shows that there is something wrong with your setup in the first place. The better idea is to correct the audio chain, so that no (or only very few) underruns happen. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] loopback: underrun_protection argument
On 02/14/2018 07:20 PM, Georg Chini wrote:
> On 14.02.2018 15:25, Raman Shyshniou wrote:
>> This patch adds a underrun_protection argument to
>> control underrun protection algorithm. Disabling
>> protection will keep loopback latency regardless
>> of underruns.
>
> Again I do not understand the motivation of the patch.
> In what situations are you seeing so many underruns and
> still want to keep the original configured latency value?
> Audio will be very bad in that case.
>
All situations where where latency is more important than data integrity.
Voice over IP (telephony) for example, receiving audio data using network
by UDP/RTP. Any data loss leads to underruns in loopback module. Current
implementation will increase overall latency by 5 msec every 3 underruns.
>> ---
>> src/modules/module-loopback.c | 31 +--
>> 1 file changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
>> index aecac0a..c452e1a 100644
>> --- a/src/modules/module-loopback.c
>> +++ b/src/modules/module-loopback.c
>> @@ -45,6 +45,7 @@ PA_MODULE_USAGE(
>> "sink= "
>> "adjust_time= "
>> "latency_msec= "
>> +"underrun_protection= "
>> "format= "
>> "rate= "
>> "channels= "
>> @@ -90,6 +91,7 @@ struct userdata {
>> /* Values from command line configuration */
>> pa_usec_t latency;
>> pa_usec_t adjust_time;
>> +bool underrun_protection;
>> /* Latency boundaries and current values */
>> pa_usec_t min_source_latency;
>> @@ -158,6 +160,7 @@ static const char* const valid_modargs[] = {
>> "sink",
>> "adjust_time",
>> "latency_msec",
>> +"underrun_protection",
>> "format",
>> "rate",
>> "channels",
>> @@ -325,11 +328,20 @@ static void adjust_rates(struct userdata *u) {
>> /* If we are seeing underruns then the latency is too small */
>> if (u->underrun_counter > 2) {
>> -u->underrun_latency_limit = PA_MAX(u->latency, u->minimum_latency)
>> + 5 * PA_USEC_PER_MSEC;
>> -u->underrun_latency_limit =
>> PA_CLIP_SUB((int64_t)u->underrun_latency_limit, u->sink_latency_offset +
>> u->source_latency_offset);
>> -update_minimum_latency(u, u->sink_input->sink, false);
>> -pa_log_warn("Too many underruns, increasing latency to %0.2f ms",
>> (double)u->minimum_latency / PA_USEC_PER_MSEC);
>> -u->underrun_counter = 0;
>> +int64_t target_latency;
>> +
>> +target_latency = PA_MAX(u->latency, u->minimum_latency);
>> +
>> +if (u->underrun_protection)
>> +target_latency += 5 * PA_USEC_PER_MSEC;
>> +
>> +u->underrun_latency_limit = PA_CLIP_SUB(target_latency,
>> u->sink_latency_offset + u->source_latency_offset);
>> +
>> +if (u->minimum_latency < u->underrun_latency_limit) {
>
> The above comparison does not make sense to me. The minimum latency
> should always be smaller than (or equal to) the underrun_latency_limit.
> Otherwise we would not be seeing underruns. There are two ways, the
> minimum latency is calculated:
> 1) Theoretically. Anything below that value will definitely cause underruns.
> The value is directly derived from the minimum sink/source latency.
> 2) Practically: If we are seeing underruns at some latency, we know the
> theoretical value is too small and update it to the empirically found value.
>
>> +update_minimum_latency(u, u->sink_input->sink, false);
>> +pa_log_warn("Too many underruns, increasing latency to %0.2f
>> ms", (double)u->minimum_latency / PA_USEC_PER_MSEC);
>> +u->underrun_counter = 0;
>> +}
>> }
>> /* Allow one underrun per hour */
>> @@ -347,7 +359,7 @@ static void adjust_rates(struct userdata *u) {
>> }
>> u->adjust_time_stamp = now;
>> -/* Rates and latencies*/
>> +/* Rates and latencies */
>> old_rate = u->sink_input->sample_spec.rate;
>> base_rate = u->source_output->sample_spec.rate;
>> @@ -1251,6 +1263,7 @@ int pa__init(pa_module *m) {
>> pa_source *source = NULL;
>> pa_source_output_new_data source_output_data;
>> bool source_dont_move;
>> +bool underrun_protection = true;
>> uint32_t latency_msec;
>> pa_sample_spec ss;
>> pa_channel_map map;
>> @@ -1336,10 +1349,16 @@ int pa__init(pa_module *m) {
>> goto fail;
>> }
>> +if (pa_modargs_get_value_boolean(ma, "underrun_protection",
>> _protection) < 0) {
>> +pa_log("Failed to parse underrun_protection value.");
>> +goto fail;
>> +}
>> +
>> m->userdata = u = pa_xnew0(struct userdata, 1);
>> u->core = m->core;
>> u->module = m;
>> u->latency = (pa_usec_t) latency_msec * PA_USEC_PER_MSEC;
>> +u->underrun_protection = underrun_protection;
>> u->output_thread_info.pop_called = false;
>>
Re: [pulseaudio-discuss] [PATCH] loopback: underrun_protection argument
On 14.02.2018 15:25, Raman Shyshniou wrote:
This patch adds a underrun_protection argument to
control underrun protection algorithm. Disabling
protection will keep loopback latency regardless
of underruns.
Again I do not understand the motivation of the patch.
In what situations are you seeing so many underruns and
still want to keep the original configured latency value?
Audio will be very bad in that case.
---
src/modules/module-loopback.c | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
index aecac0a..c452e1a 100644
--- a/src/modules/module-loopback.c
+++ b/src/modules/module-loopback.c
@@ -45,6 +45,7 @@ PA_MODULE_USAGE(
"sink= "
"adjust_time= "
"latency_msec= "
+"underrun_protection= "
"format= "
"rate= "
"channels= "
@@ -90,6 +91,7 @@ struct userdata {
/* Values from command line configuration */
pa_usec_t latency;
pa_usec_t adjust_time;
+bool underrun_protection;
/* Latency boundaries and current values */
pa_usec_t min_source_latency;
@@ -158,6 +160,7 @@ static const char* const valid_modargs[] = {
"sink",
"adjust_time",
"latency_msec",
+"underrun_protection",
"format",
"rate",
"channels",
@@ -325,11 +328,20 @@ static void adjust_rates(struct userdata *u) {
/* If we are seeing underruns then the latency is too small */
if (u->underrun_counter > 2) {
-u->underrun_latency_limit = PA_MAX(u->latency, u->minimum_latency) + 5
* PA_USEC_PER_MSEC;
-u->underrun_latency_limit = PA_CLIP_SUB((int64_t)u->underrun_latency_limit,
u->sink_latency_offset + u->source_latency_offset);
-update_minimum_latency(u, u->sink_input->sink, false);
-pa_log_warn("Too many underruns, increasing latency to %0.2f ms",
(double)u->minimum_latency / PA_USEC_PER_MSEC);
-u->underrun_counter = 0;
+int64_t target_latency;
+
+target_latency = PA_MAX(u->latency, u->minimum_latency);
+
+if (u->underrun_protection)
+target_latency += 5 * PA_USEC_PER_MSEC;
+
+u->underrun_latency_limit = PA_CLIP_SUB(target_latency,
u->sink_latency_offset + u->source_latency_offset);
+
+if (u->minimum_latency < u->underrun_latency_limit) {
The above comparison does not make sense to me. The minimum latency
should always be smaller than (or equal to) the underrun_latency_limit.
Otherwise we would not be seeing underruns. There are two ways, the
minimum latency is calculated:
1) Theoretically. Anything below that value will definitely cause underruns.
The value is directly derived from the minimum sink/source latency.
2) Practically: If we are seeing underruns at some latency, we know the
theoretical value is too small and update it to the empirically found value.
+update_minimum_latency(u, u->sink_input->sink, false);
+pa_log_warn("Too many underruns, increasing latency to %0.2f ms",
(double)u->minimum_latency / PA_USEC_PER_MSEC);
+u->underrun_counter = 0;
+}
}
/* Allow one underrun per hour */
@@ -347,7 +359,7 @@ static void adjust_rates(struct userdata *u) {
}
u->adjust_time_stamp = now;
-/* Rates and latencies*/
+/* Rates and latencies */
old_rate = u->sink_input->sample_spec.rate;
base_rate = u->source_output->sample_spec.rate;
@@ -1251,6 +1263,7 @@ int pa__init(pa_module *m) {
pa_source *source = NULL;
pa_source_output_new_data source_output_data;
bool source_dont_move;
+bool underrun_protection = true;
uint32_t latency_msec;
pa_sample_spec ss;
pa_channel_map map;
@@ -1336,10 +1349,16 @@ int pa__init(pa_module *m) {
goto fail;
}
+if (pa_modargs_get_value_boolean(ma, "underrun_protection", _protection) < 0) {
+pa_log("Failed to parse underrun_protection value.");
+goto fail;
+}
+
m->userdata = u = pa_xnew0(struct userdata, 1);
u->core = m->core;
u->module = m;
u->latency = (pa_usec_t) latency_msec * PA_USEC_PER_MSEC;
+u->underrun_protection = underrun_protection;
u->output_thread_info.pop_called = false;
u->output_thread_info.pop_adjust = false;
u->output_thread_info.push_called = false;
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] loopback: underrun_protection argument
This patch adds a underrun_protection argument to
control underrun protection algorithm. Disabling
protection will keep loopback latency regardless
of underruns.
---
src/modules/module-loopback.c | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
index aecac0a..c452e1a 100644
--- a/src/modules/module-loopback.c
+++ b/src/modules/module-loopback.c
@@ -45,6 +45,7 @@ PA_MODULE_USAGE(
"sink= "
"adjust_time= "
"latency_msec= "
+"underrun_protection= "
"format= "
"rate= "
"channels= "
@@ -90,6 +91,7 @@ struct userdata {
/* Values from command line configuration */
pa_usec_t latency;
pa_usec_t adjust_time;
+bool underrun_protection;
/* Latency boundaries and current values */
pa_usec_t min_source_latency;
@@ -158,6 +160,7 @@ static const char* const valid_modargs[] = {
"sink",
"adjust_time",
"latency_msec",
+"underrun_protection",
"format",
"rate",
"channels",
@@ -325,11 +328,20 @@ static void adjust_rates(struct userdata *u) {
/* If we are seeing underruns then the latency is too small */
if (u->underrun_counter > 2) {
-u->underrun_latency_limit = PA_MAX(u->latency, u->minimum_latency) + 5
* PA_USEC_PER_MSEC;
-u->underrun_latency_limit =
PA_CLIP_SUB((int64_t)u->underrun_latency_limit, u->sink_latency_offset +
u->source_latency_offset);
-update_minimum_latency(u, u->sink_input->sink, false);
-pa_log_warn("Too many underruns, increasing latency to %0.2f ms",
(double)u->minimum_latency / PA_USEC_PER_MSEC);
-u->underrun_counter = 0;
+int64_t target_latency;
+
+target_latency = PA_MAX(u->latency, u->minimum_latency);
+
+if (u->underrun_protection)
+target_latency += 5 * PA_USEC_PER_MSEC;
+
+u->underrun_latency_limit = PA_CLIP_SUB(target_latency,
u->sink_latency_offset + u->source_latency_offset);
+
+if (u->minimum_latency < u->underrun_latency_limit) {
+update_minimum_latency(u, u->sink_input->sink, false);
+pa_log_warn("Too many underruns, increasing latency to %0.2f ms",
(double)u->minimum_latency / PA_USEC_PER_MSEC);
+u->underrun_counter = 0;
+}
}
/* Allow one underrun per hour */
@@ -347,7 +359,7 @@ static void adjust_rates(struct userdata *u) {
}
u->adjust_time_stamp = now;
-/* Rates and latencies*/
+/* Rates and latencies */
old_rate = u->sink_input->sample_spec.rate;
base_rate = u->source_output->sample_spec.rate;
@@ -1251,6 +1263,7 @@ int pa__init(pa_module *m) {
pa_source *source = NULL;
pa_source_output_new_data source_output_data;
bool source_dont_move;
+bool underrun_protection = true;
uint32_t latency_msec;
pa_sample_spec ss;
pa_channel_map map;
@@ -1336,10 +1349,16 @@ int pa__init(pa_module *m) {
goto fail;
}
+if (pa_modargs_get_value_boolean(ma, "underrun_protection",
_protection) < 0) {
+pa_log("Failed to parse underrun_protection value.");
+goto fail;
+}
+
m->userdata = u = pa_xnew0(struct userdata, 1);
u->core = m->core;
u->module = m;
u->latency = (pa_usec_t) latency_msec * PA_USEC_PER_MSEC;
+u->underrun_protection = underrun_protection;
u->output_thread_info.pop_called = false;
u->output_thread_info.pop_adjust = false;
u->output_thread_info.push_called = false;
--
1.8.3.1
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
