Ambivalent about this one - I think I agree with the general concept,
but OTOH it somehow feels like the big rewind is there for a reason too.
Maybe if you dig deeper into the problem about filter sinks that you
noticed, maybe that can give us some hint to whether or not this is a
correct patch?
I'm marking it as "changes requested" in patchwork for the time being.
On 2015-11-17 08:40, [email protected] wrote:
From: Arun Raghavan <[email protected]>
This makes us not report more than the hardware buffer size being used
as the max rewind that sink inputs need to support. The rationale is
that we won't ever rewind more than the length of the buffer that we are
currently using, so we don't need to keep the extra data around. On
large HDA buffers, this should save some memory for each sink input.
However, I do notice a problem -- when there is a filter sink attached,
the first stream after a resume from suspend playse fine, but the second
stream has a lag that seems to be in the order of the buffer size. I'm
not sure where the problem lies.
I'll try to take a look at what's going wrong, but if anyone has any
ideas, I'm all ears.
---
src/modules/alsa/alsa-sink.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index c5a72c3..94aeff0 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -956,12 +956,14 @@ static int suspend(struct userdata *u) {
}
/* Called from IO context */
-static int update_sw_params(struct userdata *u) {
+static int update_sw_params(struct userdata *u, bool rewind) {
snd_pcm_uframes_t avail_min;
+ size_t before;
int err;
pa_assert(u);
+ before = u->hwbuf_unused;
/* Use the full buffer if no one asked us for anything specific */
u->hwbuf_unused = 0;
@@ -1006,9 +1008,21 @@ static int update_sw_params(struct userdata *u) {
return err;
}
+ if (rewind) {
+ /* Let's check whether we now use only a smaller part of the
+ buffer then before. If so, we need to make sure that subsequent
+ rewinds are relative to the new maximum fill level and not to the
+ current fill level. Thus, let's do a full rewind once, to clear
+ things up. */
+ if (u->hwbuf_unused > before) {
+ pa_log_debug("Requesting rewind due to latency change.");
+ pa_sink_request_rewind(u->sink, (size_t) -1);
+ }
+ }
+
pa_sink_set_max_request_within_thread(u->sink, u->hwbuf_size -
u->hwbuf_unused);
- if (pa_alsa_pcm_is_hw(u->pcm_handle))
- pa_sink_set_max_rewind_within_thread(u->sink, u->hwbuf_size);
+ if (pa_alsa_pcm_is_hw(u->pcm_handle))
+ pa_sink_set_max_rewind_within_thread(u->sink, u->hwbuf_size -
u->hwbuf_unused);
else {
pa_log_info("Disabling rewind_within_thread for device %s",
u->device_name);
pa_sink_set_max_rewind_within_thread(u->sink, 0);
@@ -1109,7 +1123,7 @@ static int unsuspend(struct userdata *u) {
goto fail;
}
- if (update_sw_params(u) < 0)
+ if (update_sw_params(u, false) < 0)
goto fail;
if (build_pollfd(u) < 0)
@@ -1505,7 +1519,6 @@ static int sink_set_port_cb(pa_sink *s, pa_device_port
*p) {
static void sink_update_requested_latency_cb(pa_sink *s) {
struct userdata *u = s->userdata;
- size_t before;
pa_assert(u);
pa_assert(u->use_tsched); /* only when timer scheduling is used
* we can dynamically adjust the
@@ -1514,19 +1527,7 @@ static void sink_update_requested_latency_cb(pa_sink *s)
{
if (!u->pcm_handle)
return;
- before = u->hwbuf_unused;
- update_sw_params(u);
-
- /* Let's check whether we now use only a smaller part of the
- buffer then before. If so, we need to make sure that subsequent
- rewinds are relative to the new maximum fill level and not to the
- current fill level. Thus, let's do a full rewind once, to clear
- things up. */
-
- if (u->hwbuf_unused > before) {
- pa_log_debug("Requesting rewind due to latency change.");
- pa_sink_request_rewind(s, (size_t) -1);
- }
+ update_sw_params(u, true);
}
static pa_idxset* sink_get_formats(pa_sink *s) {
@@ -2360,7 +2361,7 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma,
const char*driver, pa_ca
reserve_update(u);
- if (update_sw_params(u) < 0)
+ if (update_sw_params(u, false) < 0)
goto fail;
if (u->ucm_context) {
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
