| Message ID | 1333614140-9785-1-git-send-email-feng.wei@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 04/05/2012 10:22 AM, Feng Wei wrote: > When set latency is less than period time, we will assgin working > buffer less than a period size, which will cause unnecessary xrun > and wrong message indicating alsa driver bug. NAK, I don't think this is correct. This is tricky stuff though, so don't take my word for it! ;-) On what machine was this solving a problem for you? Have you verified it leads to better behaviour (or at least not worse) on both tsched and non-tsched systems? I e, try a VOIP call with ~20 ms of latency, on a desktop machine that has 2 x 1s periods, running in tsched mode. I'm guessing the latency will be unacceptable with this patch applied. > > Signed-off-by: Feng Wei<feng.wei@linaro.org> > --- > src/modules/alsa/alsa-sink.c | 6 +++--- > src/modules/alsa/alsa-source.c | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c > index 5a4b606..a53fb6f 100644 > --- a/src/modules/alsa/alsa-sink.c > +++ b/src/modules/alsa/alsa-sink.c > @@ -953,10 +953,10 @@ static int update_sw_params(struct userdata *u) { > > b = pa_usec_to_bytes(latency,&u->sink->sample_spec); > > - /* We need at least one sample in our buffer */ > + /* We need at least one period and one sample in our buffer to avoid underflow */ > > - if (PA_UNLIKELY(b< u->frame_size)) > - b = u->frame_size; > + if (PA_UNLIKELY(b< u->frame_size + u->fragment_size)) > + b = u->frame_size + u->fragment_size; > > u->hwbuf_unused = PA_LIKELY(b< u->hwbuf_size) ? (u->hwbuf_size - b) : 0; > } > diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c > index 27fd3b9..8c41ca6 100644 > --- a/src/modules/alsa/alsa-source.c > +++ b/src/modules/alsa/alsa-source.c > @@ -876,10 +876,10 @@ static int update_sw_params(struct userdata *u) { > > b = pa_usec_to_bytes(latency,&u->source->sample_spec); > > - /* We need at least one sample in our buffer */ > + /* We need at least one period and one sample in our buffer to avoid overflow */ > > - if (PA_UNLIKELY(b< u->frame_size)) > - b = u->frame_size; > + if (PA_UNLIKELY(b< u->frame_size + u->fragment_size)) > + b = u->frame_size + u->fragment_size; > > u->hwbuf_unused = PA_LIKELY(b< u->hwbuf_size) ? (u->hwbuf_size - b) : 0; > }
David, I'd met xrun on some embedded platform, which will only report hw_ptr at period boundary. Maybe it shouldn't use tsched, I'm not sure. Sorry for inconvenience. 2012/4/5 David Henningsson <david.henningsson@canonical.com>: > On 04/05/2012 10:22 AM, Feng Wei wrote: >> >> When set latency is less than period time, we will assgin working >> buffer less than a period size, which will cause unnecessary xrun >> and wrong message indicating alsa driver bug. > > > NAK, I don't think this is correct. > > This is tricky stuff though, so don't take my word for it! ;-) > > On what machine was this solving a problem for you? > > Have you verified it leads to better behaviour (or at least not worse) on > both tsched and non-tsched systems? > > I e, try a VOIP call with ~20 ms of latency, on a desktop machine that has 2 > x 1s periods, running in tsched mode. I'm guessing the latency will be > unacceptable with this patch applied. > >> >> Signed-off-by: Feng Wei<feng.wei@linaro.org> >> --- >> src/modules/alsa/alsa-sink.c | 6 +++--- >> src/modules/alsa/alsa-source.c | 6 +++--- >> 2 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c >> index 5a4b606..a53fb6f 100644 >> --- a/src/modules/alsa/alsa-sink.c >> +++ b/src/modules/alsa/alsa-sink.c >> @@ -953,10 +953,10 @@ static int update_sw_params(struct userdata *u) { >> >> b = pa_usec_to_bytes(latency,&u->sink->sample_spec); >> >> >> - /* We need at least one sample in our buffer */ >> + /* We need at least one period and one sample in our buffer >> to avoid underflow */ >> >> - if (PA_UNLIKELY(b< u->frame_size)) >> - b = u->frame_size; >> + if (PA_UNLIKELY(b< u->frame_size + u->fragment_size)) >> + b = u->frame_size + u->fragment_size; >> >> u->hwbuf_unused = PA_LIKELY(b< u->hwbuf_size) ? >> (u->hwbuf_size - b) : 0; >> } >> diff --git a/src/modules/alsa/alsa-source.c >> b/src/modules/alsa/alsa-source.c >> index 27fd3b9..8c41ca6 100644 >> --- a/src/modules/alsa/alsa-source.c >> +++ b/src/modules/alsa/alsa-source.c >> @@ -876,10 +876,10 @@ static int update_sw_params(struct userdata *u) { >> >> b = pa_usec_to_bytes(latency,&u->source->sample_spec); >> >> >> - /* We need at least one sample in our buffer */ >> + /* We need at least one period and one sample in our buffer >> to avoid overflow */ >> >> - if (PA_UNLIKELY(b< u->frame_size)) >> - b = u->frame_size; >> + if (PA_UNLIKELY(b< u->frame_size + u->fragment_size)) >> + b = u->frame_size + u->fragment_size; >> >> u->hwbuf_unused = PA_LIKELY(b< u->hwbuf_size) ? >> (u->hwbuf_size - b) : 0; >> } > > > > > -- > David Henningsson, Canonical Ltd. > http://launchpad.net/~diwic
On 04/05/2012 10:59 AM, Feng Wei wrote: > David, > I'd met xrun on some embedded platform, which will only report hw_ptr > at period boundary. Maybe it shouldn't use tsched, I'm not sure. Sorry > for inconvenience. Actually it does raise an important question about how we deal with platforms that have bad hw_ptr granularity. We don't to that very well today. Also is there much difference, powersave wise, between running in non-tsched and tsched mode? If there isn't, maybe using non-tsched for your particular device is the best option (if it works). > > 2012/4/5 David Henningsson<david.henningsson@canonical.com>: >> On 04/05/2012 10:22 AM, Feng Wei wrote: >>> >>> When set latency is less than period time, we will assgin working >>> buffer less than a period size, which will cause unnecessary xrun >>> and wrong message indicating alsa driver bug. >> >> >> NAK, I don't think this is correct. >> >> This is tricky stuff though, so don't take my word for it! ;-) >> >> On what machine was this solving a problem for you? >> >> Have you verified it leads to better behaviour (or at least not worse) on >> both tsched and non-tsched systems? >> >> I e, try a VOIP call with ~20 ms of latency, on a desktop machine that has 2 >> x 1s periods, running in tsched mode. I'm guessing the latency will be >> unacceptable with this patch applied. >> >>> >>> Signed-off-by: Feng Wei<feng.wei@linaro.org> >>> --- >>> src/modules/alsa/alsa-sink.c | 6 +++--- >>> src/modules/alsa/alsa-source.c | 6 +++--- >>> 2 files changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c >>> index 5a4b606..a53fb6f 100644 >>> --- a/src/modules/alsa/alsa-sink.c >>> +++ b/src/modules/alsa/alsa-sink.c >>> @@ -953,10 +953,10 @@ static int update_sw_params(struct userdata *u) { >>> >>> b = pa_usec_to_bytes(latency,&u->sink->sample_spec); >>> >>> >>> - /* We need at least one sample in our buffer */ >>> + /* We need at least one period and one sample in our buffer >>> to avoid underflow */ >>> >>> - if (PA_UNLIKELY(b< u->frame_size)) >>> - b = u->frame_size; >>> + if (PA_UNLIKELY(b< u->frame_size + u->fragment_size)) >>> + b = u->frame_size + u->fragment_size; >>> >>> u->hwbuf_unused = PA_LIKELY(b< u->hwbuf_size) ? >>> (u->hwbuf_size - b) : 0; >>> } >>> diff --git a/src/modules/alsa/alsa-source.c >>> b/src/modules/alsa/alsa-source.c >>> index 27fd3b9..8c41ca6 100644 >>> --- a/src/modules/alsa/alsa-source.c >>> +++ b/src/modules/alsa/alsa-source.c >>> @@ -876,10 +876,10 @@ static int update_sw_params(struct userdata *u) { >>> >>> b = pa_usec_to_bytes(latency,&u->source->sample_spec); >>> >>> >>> - /* We need at least one sample in our buffer */ >>> + /* We need at least one period and one sample in our buffer >>> to avoid overflow */ >>> >>> - if (PA_UNLIKELY(b< u->frame_size)) >>> - b = u->frame_size; >>> + if (PA_UNLIKELY(b< u->frame_size + u->fragment_size)) >>> + b = u->frame_size + u->fragment_size; >>> >>> u->hwbuf_unused = PA_LIKELY(b< u->hwbuf_size) ? >>> (u->hwbuf_size - b) : 0; >>> } >> >> >> >> >> -- >> David Henningsson, Canonical Ltd. >> http://launchpad.net/~diwic > > >
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c index 5a4b606..a53fb6f 100644 --- a/src/modules/alsa/alsa-sink.c +++ b/src/modules/alsa/alsa-sink.c @@ -953,10 +953,10 @@ static int update_sw_params(struct userdata *u) { b = pa_usec_to_bytes(latency, &u->sink->sample_spec); - /* We need at least one sample in our buffer */ + /* We need at least one period and one sample in our buffer to avoid underflow */ - if (PA_UNLIKELY(b < u->frame_size)) - b = u->frame_size; + if (PA_UNLIKELY(b < u->frame_size + u->fragment_size)) + b = u->frame_size + u->fragment_size; u->hwbuf_unused = PA_LIKELY(b < u->hwbuf_size) ? (u->hwbuf_size - b) : 0; } diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c index 27fd3b9..8c41ca6 100644 --- a/src/modules/alsa/alsa-source.c +++ b/src/modules/alsa/alsa-source.c @@ -876,10 +876,10 @@ static int update_sw_params(struct userdata *u) { b = pa_usec_to_bytes(latency, &u->source->sample_spec); - /* We need at least one sample in our buffer */ + /* We need at least one period and one sample in our buffer to avoid overflow */ - if (PA_UNLIKELY(b < u->frame_size)) - b = u->frame_size; + if (PA_UNLIKELY(b < u->frame_size + u->fragment_size)) + b = u->frame_size + u->fragment_size; u->hwbuf_unused = PA_LIKELY(b < u->hwbuf_size) ? (u->hwbuf_size - b) : 0; }
When set latency is less than period time, we will assgin working buffer less than a period size, which will cause unnecessary xrun and wrong message indicating alsa driver bug. Signed-off-by: Feng Wei <feng.wei@linaro.org> --- src/modules/alsa/alsa-sink.c | 6 +++--- src/modules/alsa/alsa-source.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)