From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8757BC004D3 for ; Wed, 24 Oct 2018 08:20:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 36585207DD for ; Wed, 24 Oct 2018 08:20:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 36585207DD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=eircom.net Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727641AbeJXQrU convert rfc822-to-8bit (ORCPT ); Wed, 24 Oct 2018 12:47:20 -0400 Received: from vie01a-dmta-pe08-3.mx.upcmail.net ([84.116.36.22]:31584 "EHLO vie01a-dmta-pe08-3.mx.upcmail.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726531AbeJXQrU (ORCPT ); Wed, 24 Oct 2018 12:47:20 -0400 Received: from [172.31.216.235] (helo=vie01a-pemc-psmtp-pe12.mail.upcmail.net) by vie01a-dmta-pe08.mx.upcmail.net with esmtp (Exim 4.88) (envelope-from ) id 1gFEOh-000183-6B for linux-kernel@vger.kernel.org; Wed, 24 Oct 2018 10:20:11 +0200 Received: from helix.aillwee.com ([37.228.204.209]) by vie01a-pemc-psmtp-pe12.mail.upcmail.net with ESMTP id FEOfgQA8ukosQFEOggF1UE; Wed, 24 Oct 2018 10:20:11 +0200 X-Env-Mailfrom: mikebrady@eircom.net X-Env-Rcptto: linux-kernel@vger.kernel.org X-SourceIP: 37.228.204.209 X-CNFS-Analysis: v=2.3 cv=NNQEBHyg c=1 sm=1 tr=0 a=/+iDkf0alGTUGXENEoGzTg==:117 a=/+iDkf0alGTUGXENEoGzTg==:17 a=IkcTkHD0fZMA:10 a=smKx5t2vBNcA:10 a=pGLkceISAAAA:8 a=NEAV23lmAAAA:8 a=k7f7euTfAAAA:8 a=foHCeV_ZAAAA:8 a=y0jdxBOuuxOgm-QiZ8EA:9 a=AAWk5B1plMHTeopH:21 a=HwXA8Gq0tMqA7mWy:21 a=QEXdDO2ut3YA:10 a=h8a9FgHX5U4dIE3jaWyr:22 Received: from [172.20.10.4] (unknown [77.75.241.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by helix.aillwee.com (Postfix) with ESMTPSA id 85B5D4E607; Wed, 24 Oct 2018 09:20:08 +0100 (IST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: Re: [alsa-devel] [PATCH v2] staging: bcm2835-audio: interpolate audio delay From: Mike Brady In-Reply-To: Date: Wed, 24 Oct 2018 09:20:09 +0100 Cc: stefan.wahren@i2se.com, devel@driverdev.osuosl.org, alsa-devel@alsa-project.org, f.fainelli@gmail.com, julia.lawall@lip6.fr, tiwai@suse.de, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, eric@anholt.net, linux-rpi-kernel@lists.infradead.org, nishka.dasgupta_ug18@ashoka.edu.in, linux-arm-kernel@lists.infradead.org Content-Transfer-Encoding: 8BIT Message-Id: References: <20181022191708.GA4659@ubuntu> To: Kirill Marinushkin X-Mailer: Apple Mail (2.3445.100.39) X-CMAE-Envelope: MS4wfEwWJmYGFcdTp+qKYun7Wv3/P3lnJAO0r0xg1QBxIZUQKiLP4FhcxlXnSJi+LZGVugtcvSQzLZE0gKjx3ty+JbOp1UP9Y+IZRItaR9LaEim8RM+DcIma aR4k30rTYeFXhic7Ausi5S2C/elhqHFSeJ568mJtV++CPK1ruCHsSKpaFvqk+7AxTNLcVqU9fDPwrX54v2xF2qjit6YPLO7RO48KPIOrvec3kJZMEXL7l57U kmYtWm0uRtOGgsKGEUs1wzaUXFwMDGLU85OyMWfm09o9Hg5QzB6dtLoaql6K7IcZcqBPtnpKdE54iN5QHM3KQGrIf/wtVhRsbut3TiWJeuvAc5BsTABBjw9t 2GRV0/ehd3t7KFRRSd5qDDmG5qavcux3L41SSJ0cz11AYVhajQBY1umuWxvMq6prVtSeNMcVAHEvK5gDNFy1XC6o4ACEJlXkesC8/ZIEMNOi/ESkVcRe24Ap PgMWQzlkKHjHdTJ4ZMG49uug9CNVtCiXz1WZiJg7eAMKBwTCZxVEoODnQIl37BpRfA3etwTSfgZWd/9+/nQ5YyppWd45hLVMkkpDVnta9sH+vi/ehM+OXIcX bRiHuMMcz+Su1WJnb+iUVUWi5SCcO+XN0M5aQUk1mvkkIUjk4LcN7A9DMp2ZZ+Zqq4c= Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Kirill. Thanks for your comments. > On 22 Oct 2018, at 23:25, Kirill Marinushkin wrote: > > AFAIU, this patch is wrong. Please correct me, maybe I misunderstand something. > >> The problem that this patch seeks to resolve is that when userland asks for >> the delay > > The userspace asks not for delay, but for the pointer. The call in question is snd_pcm_delay. I presume this delay is calculated from knowledge of the stream position “pos", the period (buffer?) number (and period/buffer size) and the snd_pcm_runtime structure’s “delay" field (“runtime->delay”). > You modify the function, which is called `snd_bcm2835_pcm_pointer`. Here you are > supposed to increase `alsa_stream->pos` with the proper offset. Instead, you > imitate a delay, but in fact the delay is not increased. > > So, the proper solution should be to fix the reported pointer. I think there is a difficulty with this. The “pos” pointer looks to have to be modulo the buffer size. This causes a problem, as I see it, in that if the calculated (pos + interpolated delay in bytes) is longer than the buffer size, it must be wrapped, but AFAIK we are unable to increment a buffer index used in the snd_pcm_delay calculation. Hence the calculation of the actual position would be wrong. This is why the snd_pcm_runtime delay field is used. On reflection, BTW, the patch assumes that the field's original value was zero — that can be rectified. > As a result, > userspace will recieve the correct delay, instead of these crazy 10 ms. Just to point out that with the proposed patch, it appears that the correct delay is being reported, (apart, possibly, from any delay originally set in the snd_pcm_delay field, as mentioned above). All the best, Mike > FYI, there is >> a discussion of the effects of a downstream equivalent of this suggested patch >> at: >> https://github.com/raspberrypi/firmware/issues/1026#issuecomment-415746016. > > Thank you for the link, it clarified for me what you try to achieve. > > On 10/22/18 21:17, Mike Brady wrote: >> When the BCM2835 audio output is used, userspace sees a jitter up to 10ms >> in the audio position, aka "delay" -- the number of frames that must >> be output before a new frame would be played. >> Make this a bit nicer for userspace by interpolating the position >> using the CPU clock. >> The overhead is small -- an extra ktime_get() every time a GPU message >> is sent -- and another call and a few calculations whenever the delay >> is sought from userland. >> At 48,000 frames per second, i.e. approximately 20 microseconds per >> frame, it would take a clock inaccuracy of >> 20 microseconds in 10 milliseconds -- 2,000 parts per million -- >> to result in an inaccurate estimate, whereas >> crystal- or resonator-based clocks typically have an >> inaccuracy of 10s to 100s of parts per million. >> >> Signed-off-by: Mike Brady >> --- >> Changes in v2 -- remove inappropriate addition of SNDRV_PCM_INFO_BATCH flag >> >> .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 20 +++++++++++++++++++ >> .../vc04_services/bcm2835-audio/bcm2835.h | 1 + >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c >> index e66da11af5cf..9053b996cada 100644 >> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c >> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c >> @@ -74,6 +74,7 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream, >> atomic_set(&alsa_stream->pos, pos); >> >> alsa_stream->period_offset += bytes; >> + alsa_stream->interpolate_start = ktime_get(); >> if (alsa_stream->period_offset >= alsa_stream->period_size) { >> alsa_stream->period_offset %= alsa_stream->period_size; >> snd_pcm_period_elapsed(substream); >> @@ -243,6 +244,7 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) >> atomic_set(&alsa_stream->pos, 0); >> alsa_stream->period_offset = 0; >> alsa_stream->draining = false; >> + alsa_stream->interpolate_start = ktime_get(); >> >> return 0; >> } >> @@ -292,6 +294,24 @@ snd_bcm2835_pcm_pointer(struct snd_pcm_substream *substream) >> { >> struct snd_pcm_runtime *runtime = substream->runtime; >> struct bcm2835_alsa_stream *alsa_stream = runtime->private_data; >> + ktime_t now = ktime_get(); >> + >> + /* Give userspace better delay reporting by interpolating between GPU >> + * notifications, assuming audio speed is close enough to the clock >> + * used for ktime >> + */ >> + >> + if ((ktime_to_ns(alsa_stream->interpolate_start)) && >> + (ktime_compare(alsa_stream->interpolate_start, now) < 0)) { >> + u64 interval = >> + (ktime_to_ns(ktime_sub(now, >> + alsa_stream->interpolate_start))); >> + u64 frames_output_in_interval = >> + div_u64((interval * runtime->rate), 1000000000); >> + snd_pcm_sframes_t frames_output_in_interval_sized = >> + -frames_output_in_interval; >> + runtime->delay = frames_output_in_interval_sized; >> + } >> >> return snd_pcm_indirect_playback_pointer(substream, >> &alsa_stream->pcm_indirect, >> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h >> index e13435d1c205..595ad584243f 100644 >> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h >> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h >> @@ -78,6 +78,7 @@ struct bcm2835_alsa_stream { >> unsigned int period_offset; >> unsigned int buffer_size; >> unsigned int period_size; >> + ktime_t interpolate_start; >> >> struct bcm2835_audio_instance *instance; >> int idx; >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Brady Subject: Re: [PATCH v2] staging: bcm2835-audio: interpolate audio delay Date: Wed, 24 Oct 2018 09:20:09 +0100 Message-ID: References: <20181022191708.GA4659@ubuntu> Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from vie01a-dmta-pe06-1.mx.upcmail.net (vie01a-dmta-pe06-1.mx.upcmail.net [84.116.36.14]) by alsa0.perex.cz (Postfix) with ESMTP id 8E57A26752C for ; Wed, 24 Oct 2018 10:20:11 +0200 (CEST) Received: from [172.31.216.235] (helo=vie01a-pemc-psmtp-pe12.mail.upcmail.net) by vie01a-dmta-pe06.mx.upcmail.net with esmtp (Exim 4.88) (envelope-from ) id 1gFEOh-0000y8-64 for alsa-devel@alsa-project.org; Wed, 24 Oct 2018 10:20:11 +0200 In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Kirill Marinushkin Cc: stefan.wahren@i2se.com, devel@driverdev.osuosl.org, alsa-devel@alsa-project.org, f.fainelli@gmail.com, eric@anholt.net, tiwai@suse.de, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, julia.lawall@lip6.fr, linux-rpi-kernel@lists.infradead.org, nishka.dasgupta_ug18@ashoka.edu.in, linux-arm-kernel@lists.infradead.org List-Id: alsa-devel@alsa-project.org SGkgS2lyaWxsLiBUaGFua3MgZm9yIHlvdXIgY29tbWVudHMuCgo+IE9uIDIyIE9jdCAyMDE4LCBh dCAyMzoyNSwgS2lyaWxsIE1hcmludXNoa2luIDxrLm1hcmludXNoa2luQGdtYWlsLmNvbT4gd3Jv dGU6Cj4gCj4gQUZBSVUsIHRoaXMgcGF0Y2ggaXMgd3JvbmcuIFBsZWFzZSBjb3JyZWN0IG1lLCBt YXliZSBJIG1pc3VuZGVyc3RhbmQgc29tZXRoaW5nLgo+IAo+PiBUaGUgcHJvYmxlbSB0aGF0IHRo aXMgcGF0Y2ggc2Vla3MgdG8gcmVzb2x2ZSBpcyB0aGF0IHdoZW4gdXNlcmxhbmQgYXNrcyBmb3IK Pj4gdGhlIGRlbGF5Cj4gCj4gVGhlIHVzZXJzcGFjZSBhc2tzIG5vdCBmb3IgZGVsYXksIGJ1dCBm b3IgdGhlIHBvaW50ZXIuCgpUaGUgY2FsbCBpbiBxdWVzdGlvbiBpcyBzbmRfcGNtX2RlbGF5LiBJ IHByZXN1bWUgdGhpcyBkZWxheSBpcyBjYWxjdWxhdGVkIGZyb20ga25vd2xlZGdlIG9mIHRoZSBz dHJlYW0gcG9zaXRpb24g4oCccG9zIiwgdGhlIHBlcmlvZCAoYnVmZmVyPykgbnVtYmVyIChhbmQg cGVyaW9kL2J1ZmZlciBzaXplKSBhbmQgdGhlIHNuZF9wY21fcnVudGltZSBzdHJ1Y3R1cmXigJlz IOKAnGRlbGF5IiBmaWVsZCAo4oCccnVudGltZS0+ZGVsYXnigJ0pLgoKPiBZb3UgbW9kaWZ5IHRo ZSBmdW5jdGlvbiwgd2hpY2ggaXMgY2FsbGVkIGBzbmRfYmNtMjgzNV9wY21fcG9pbnRlcmAuIEhl cmUgeW91IGFyZQo+IHN1cHBvc2VkIHRvIGluY3JlYXNlIGBhbHNhX3N0cmVhbS0+cG9zYCB3aXRo IHRoZSBwcm9wZXIgb2Zmc2V0LiBJbnN0ZWFkLCB5b3UKPiBpbWl0YXRlIGEgZGVsYXksIGJ1dCBp biBmYWN0IHRoZSBkZWxheSBpcyBub3QgaW5jcmVhc2VkLgo+IAo+IFNvLCB0aGUgcHJvcGVyIHNv bHV0aW9uIHNob3VsZCBiZSB0byBmaXggdGhlIHJlcG9ydGVkIHBvaW50ZXIuCgpJIHRoaW5rIHRo ZXJlIGlzIGEgZGlmZmljdWx0eSB3aXRoIHRoaXMuIFRoZSDigJxwb3PigJ0gcG9pbnRlciBsb29r cyB0byBoYXZlIHRvIGJlIG1vZHVsbyB0aGUgYnVmZmVyIHNpemUuIFRoaXMgY2F1c2VzIGEgcHJv YmxlbSwgYXMgSSBzZWUgaXQsIGluIHRoYXQgaWYgdGhlIGNhbGN1bGF0ZWQgKHBvcyArIGludGVy cG9sYXRlZCBkZWxheSBpbiBieXRlcykgaXMgbG9uZ2VyIHRoYW4gdGhlIGJ1ZmZlciBzaXplLCBp dCBtdXN0IGJlIHdyYXBwZWQsIGJ1dCBBRkFJSyB3ZSBhcmUgdW5hYmxlIHRvIGluY3JlbWVudCBh IGJ1ZmZlciBpbmRleCB1c2VkIGluIHRoZSBzbmRfcGNtX2RlbGF5IGNhbGN1bGF0aW9uLiBIZW5j ZSB0aGUgY2FsY3VsYXRpb24gb2YgdGhlIGFjdHVhbCBwb3NpdGlvbiB3b3VsZCBiZSB3cm9uZy4g VGhpcyBpcyB3aHkgdGhlIHNuZF9wY21fcnVudGltZSBkZWxheSBmaWVsZCBpcyB1c2VkLiBPbiBy ZWZsZWN0aW9uLCBCVFcsIHRoZSBwYXRjaCBhc3N1bWVzIHRoYXQgdGhlIGZpZWxkJ3Mgb3JpZ2lu YWwgdmFsdWUgd2FzIHplcm8g4oCUIHRoYXQgY2FuIGJlIHJlY3RpZmllZC4KCj4gQXMgYSByZXN1 bHQsCj4gdXNlcnNwYWNlIHdpbGwgcmVjaWV2ZSB0aGUgY29ycmVjdCBkZWxheSwgaW5zdGVhZCBv ZiB0aGVzZSBjcmF6eSAxMCBtcy4KCkp1c3QgdG8gcG9pbnQgb3V0IHRoYXQgd2l0aCB0aGUgcHJv cG9zZWQgcGF0Y2gsIGl0IGFwcGVhcnMgdGhhdCB0aGUgY29ycmVjdCBkZWxheSBpcyBiZWluZyBy ZXBvcnRlZCwgKGFwYXJ0LCBwb3NzaWJseSwgZnJvbSBhbnkgZGVsYXkgb3JpZ2luYWxseSBzZXQg aW4gdGhlIHNuZF9wY21fZGVsYXkgZmllbGQsIGFzIG1lbnRpb25lZCBhYm92ZSkuCgpBbGwgdGhl IGJlc3QsCk1pa2UKCj4gRllJLCB0aGVyZSBpcwo+PiBhIGRpc2N1c3Npb24gb2YgdGhlIGVmZmVj dHMgb2YgYSBkb3duc3RyZWFtIGVxdWl2YWxlbnQgb2YgdGhpcyBzdWdnZXN0ZWQgcGF0Y2gKPj4g YXQ6Cj4+IGh0dHBzOi8vZ2l0aHViLmNvbS9yYXNwYmVycnlwaS9maXJtd2FyZS9pc3N1ZXMvMTAy NiNpc3N1ZWNvbW1lbnQtNDE1NzQ2MDE2Lgo+IAo+IFRoYW5rIHlvdSBmb3IgdGhlIGxpbmssIGl0 IGNsYXJpZmllZCBmb3IgbWUgd2hhdCB5b3UgdHJ5IHRvIGFjaGlldmUuCj4gCj4gT24gMTAvMjIv MTggMjE6MTcsIE1pa2UgQnJhZHkgd3JvdGU6Cj4+IFdoZW4gdGhlIEJDTTI4MzUgYXVkaW8gb3V0 cHV0IGlzIHVzZWQsIHVzZXJzcGFjZSBzZWVzIGEgaml0dGVyIHVwIHRvIDEwbXMKPj4gaW4gdGhl IGF1ZGlvIHBvc2l0aW9uLCBha2EgImRlbGF5IiAtLSB0aGUgbnVtYmVyIG9mIGZyYW1lcyB0aGF0 IG11c3QKPj4gYmUgb3V0cHV0IGJlZm9yZSBhIG5ldyBmcmFtZSB3b3VsZCBiZSBwbGF5ZWQuCj4+ IE1ha2UgdGhpcyBhIGJpdCBuaWNlciBmb3IgdXNlcnNwYWNlIGJ5IGludGVycG9sYXRpbmcgdGhl IHBvc2l0aW9uCj4+IHVzaW5nIHRoZSBDUFUgY2xvY2suCj4+IFRoZSBvdmVyaGVhZCBpcyBzbWFs bCAtLSBhbiBleHRyYSBrdGltZV9nZXQoKSBldmVyeSB0aW1lIGEgR1BVIG1lc3NhZ2UKPj4gaXMg c2VudCAtLSBhbmQgYW5vdGhlciBjYWxsIGFuZCBhIGZldyBjYWxjdWxhdGlvbnMgd2hlbmV2ZXIg dGhlIGRlbGF5Cj4+IGlzIHNvdWdodCBmcm9tIHVzZXJsYW5kLgo+PiBBdCA0OCwwMDAgZnJhbWVz IHBlciBzZWNvbmQsIGkuZS4gYXBwcm94aW1hdGVseSAyMCBtaWNyb3NlY29uZHMgcGVyCj4+IGZy YW1lLCBpdCB3b3VsZCB0YWtlIGEgY2xvY2sgaW5hY2N1cmFjeSBvZgo+PiAyMCBtaWNyb3NlY29u ZHMgaW4gMTAgbWlsbGlzZWNvbmRzIC0tIDIsMDAwIHBhcnRzIHBlciBtaWxsaW9uIC0tCj4+IHRv IHJlc3VsdCBpbiBhbiBpbmFjY3VyYXRlIGVzdGltYXRlLCB3aGVyZWFzCj4+IGNyeXN0YWwtIG9y IHJlc29uYXRvci1iYXNlZCBjbG9ja3MgdHlwaWNhbGx5IGhhdmUgYW4KPj4gaW5hY2N1cmFjeSBv ZiAxMHMgdG8gMTAwcyBvZiBwYXJ0cyBwZXIgbWlsbGlvbi4KPj4gCj4+IFNpZ25lZC1vZmYtYnk6 IE1pa2UgQnJhZHkgPG1pa2VicmFkeUBlaXJjb20ubmV0Pgo+PiAtLS0KPj4gQ2hhbmdlcyBpbiB2 MiAtLSByZW1vdmUgaW5hcHByb3ByaWF0ZSBhZGRpdGlvbiBvZiBTTkRSVl9QQ01fSU5GT19CQVRD SCBmbGFnCj4+IAo+PiAuLi4vdmMwNF9zZXJ2aWNlcy9iY20yODM1LWF1ZGlvL2JjbTI4MzUtcGNt LmMgfCAyMCArKysrKysrKysrKysrKysrKysrCj4+IC4uLi92YzA0X3NlcnZpY2VzL2JjbTI4MzUt YXVkaW8vYmNtMjgzNS5oICAgICB8ICAxICsKPj4gMiBmaWxlcyBjaGFuZ2VkLCAyMSBpbnNlcnRp b25zKCspCj4+IAo+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5nL3ZjMDRfc2VydmljZXMv YmNtMjgzNS1hdWRpby9iY20yODM1LXBjbS5jIGIvZHJpdmVycy9zdGFnaW5nL3ZjMDRfc2Vydmlj ZXMvYmNtMjgzNS1hdWRpby9iY20yODM1LXBjbS5jCj4+IGluZGV4IGU2NmRhMTFhZjVjZi4uOTA1 M2I5OTZjYWRhIDEwMDY0NAo+PiAtLS0gYS9kcml2ZXJzL3N0YWdpbmcvdmMwNF9zZXJ2aWNlcy9i Y20yODM1LWF1ZGlvL2JjbTI4MzUtcGNtLmMKPj4gKysrIGIvZHJpdmVycy9zdGFnaW5nL3ZjMDRf c2VydmljZXMvYmNtMjgzNS1hdWRpby9iY20yODM1LXBjbS5jCj4+IEBAIC03NCw2ICs3NCw3IEBA IHZvaWQgYmNtMjgzNV9wbGF5YmFja19maWZvKHN0cnVjdCBiY20yODM1X2Fsc2Ffc3RyZWFtICph bHNhX3N0cmVhbSwKPj4gCWF0b21pY19zZXQoJmFsc2Ffc3RyZWFtLT5wb3MsIHBvcyk7Cj4+IAo+ PiAJYWxzYV9zdHJlYW0tPnBlcmlvZF9vZmZzZXQgKz0gYnl0ZXM7Cj4+ICsJYWxzYV9zdHJlYW0t PmludGVycG9sYXRlX3N0YXJ0ID0ga3RpbWVfZ2V0KCk7Cj4+IAlpZiAoYWxzYV9zdHJlYW0tPnBl cmlvZF9vZmZzZXQgPj0gYWxzYV9zdHJlYW0tPnBlcmlvZF9zaXplKSB7Cj4+IAkJYWxzYV9zdHJl YW0tPnBlcmlvZF9vZmZzZXQgJT0gYWxzYV9zdHJlYW0tPnBlcmlvZF9zaXplOwo+PiAJCXNuZF9w Y21fcGVyaW9kX2VsYXBzZWQoc3Vic3RyZWFtKTsKPj4gQEAgLTI0Myw2ICsyNDQsNyBAQCBzdGF0 aWMgaW50IHNuZF9iY20yODM1X3BjbV9wcmVwYXJlKHN0cnVjdCBzbmRfcGNtX3N1YnN0cmVhbSAq c3Vic3RyZWFtKQo+PiAJYXRvbWljX3NldCgmYWxzYV9zdHJlYW0tPnBvcywgMCk7Cj4+IAlhbHNh X3N0cmVhbS0+cGVyaW9kX29mZnNldCA9IDA7Cj4+IAlhbHNhX3N0cmVhbS0+ZHJhaW5pbmcgPSBm YWxzZTsKPj4gKwlhbHNhX3N0cmVhbS0+aW50ZXJwb2xhdGVfc3RhcnQgPSBrdGltZV9nZXQoKTsK Pj4gCj4+IAlyZXR1cm4gMDsKPj4gfQo+PiBAQCAtMjkyLDYgKzI5NCwyNCBAQCBzbmRfYmNtMjgz NV9wY21fcG9pbnRlcihzdHJ1Y3Qgc25kX3BjbV9zdWJzdHJlYW0gKnN1YnN0cmVhbSkKPj4gewo+ PiAJc3RydWN0IHNuZF9wY21fcnVudGltZSAqcnVudGltZSA9IHN1YnN0cmVhbS0+cnVudGltZTsK Pj4gCXN0cnVjdCBiY20yODM1X2Fsc2Ffc3RyZWFtICphbHNhX3N0cmVhbSA9IHJ1bnRpbWUtPnBy aXZhdGVfZGF0YTsKPj4gKwlrdGltZV90IG5vdyA9IGt0aW1lX2dldCgpOwo+PiArCj4+ICsJLyog R2l2ZSB1c2Vyc3BhY2UgYmV0dGVyIGRlbGF5IHJlcG9ydGluZyBieSBpbnRlcnBvbGF0aW5nIGJl dHdlZW4gR1BVCj4+ICsJICogbm90aWZpY2F0aW9ucywgYXNzdW1pbmcgYXVkaW8gc3BlZWQgaXMg Y2xvc2UgZW5vdWdoIHRvIHRoZSBjbG9jawo+PiArCSAqIHVzZWQgZm9yIGt0aW1lCj4+ICsJICov Cj4+ICsKPj4gKwlpZiAoKGt0aW1lX3RvX25zKGFsc2Ffc3RyZWFtLT5pbnRlcnBvbGF0ZV9zdGFy dCkpICYmCj4+ICsJICAgIChrdGltZV9jb21wYXJlKGFsc2Ffc3RyZWFtLT5pbnRlcnBvbGF0ZV9z dGFydCwgbm93KSA8IDApKSB7Cj4+ICsJCXU2NCBpbnRlcnZhbCA9Cj4+ICsJCQkoa3RpbWVfdG9f bnMoa3RpbWVfc3ViKG5vdywKPj4gKwkJCQlhbHNhX3N0cmVhbS0+aW50ZXJwb2xhdGVfc3RhcnQp KSk7Cj4+ICsJCXU2NCBmcmFtZXNfb3V0cHV0X2luX2ludGVydmFsID0KPj4gKwkJCWRpdl91NjQo KGludGVydmFsICogcnVudGltZS0+cmF0ZSksIDEwMDAwMDAwMDApOwo+PiArCQlzbmRfcGNtX3Nm cmFtZXNfdCBmcmFtZXNfb3V0cHV0X2luX2ludGVydmFsX3NpemVkID0KPj4gKwkJCS1mcmFtZXNf b3V0cHV0X2luX2ludGVydmFsOwo+PiArCQlydW50aW1lLT5kZWxheSA9IGZyYW1lc19vdXRwdXRf aW5faW50ZXJ2YWxfc2l6ZWQ7Cj4+ICsJfQo+PiAKPj4gCXJldHVybiBzbmRfcGNtX2luZGlyZWN0 X3BsYXliYWNrX3BvaW50ZXIoc3Vic3RyZWFtLAo+PiAJCSZhbHNhX3N0cmVhbS0+cGNtX2luZGly ZWN0LAo+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5nL3ZjMDRfc2VydmljZXMvYmNtMjgz NS1hdWRpby9iY20yODM1LmggYi9kcml2ZXJzL3N0YWdpbmcvdmMwNF9zZXJ2aWNlcy9iY20yODM1 LWF1ZGlvL2JjbTI4MzUuaAo+PiBpbmRleCBlMTM0MzVkMWMyMDUuLjU5NWFkNTg0MjQzZiAxMDA2 NDQKPj4gLS0tIGEvZHJpdmVycy9zdGFnaW5nL3ZjMDRfc2VydmljZXMvYmNtMjgzNS1hdWRpby9i Y20yODM1LmgKPj4gKysrIGIvZHJpdmVycy9zdGFnaW5nL3ZjMDRfc2VydmljZXMvYmNtMjgzNS1h dWRpby9iY20yODM1LmgKPj4gQEAgLTc4LDYgKzc4LDcgQEAgc3RydWN0IGJjbTI4MzVfYWxzYV9z dHJlYW0gewo+PiAJdW5zaWduZWQgaW50IHBlcmlvZF9vZmZzZXQ7Cj4+IAl1bnNpZ25lZCBpbnQg YnVmZmVyX3NpemU7Cj4+IAl1bnNpZ25lZCBpbnQgcGVyaW9kX3NpemU7Cj4+ICsJa3RpbWVfdCBp bnRlcnBvbGF0ZV9zdGFydDsKPj4gCj4+IAlzdHJ1Y3QgYmNtMjgzNV9hdWRpb19pbnN0YW5jZSAq aW5zdGFuY2U7Cj4+IAlpbnQgaWR4Owo+PiAKPiBfX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fXwo+IEFsc2EtZGV2ZWwgbWFpbGluZyBsaXN0Cj4gQWxzYS1kZXZl bEBhbHNhLXByb2plY3Qub3JnCj4gaHR0cDovL21haWxtYW4uYWxzYS1wcm9qZWN0Lm9yZy9tYWls bWFuL2xpc3RpbmZvL2Fsc2EtZGV2ZWwKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fCkFsc2EtZGV2ZWwgbWFpbGluZyBsaXN0CkFsc2EtZGV2ZWxAYWxzYS1w cm9qZWN0Lm9yZwpodHRwOi8vbWFpbG1hbi5hbHNhLXByb2plY3Qub3JnL21haWxtYW4vbGlzdGlu Zm8vYWxzYS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: mikebrady@eircom.net (Mike Brady) Date: Wed, 24 Oct 2018 09:20:09 +0100 Subject: [alsa-devel] [PATCH v2] staging: bcm2835-audio: interpolate audio delay In-Reply-To: References: <20181022191708.GA4659@ubuntu> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Kirill. Thanks for your comments. > On 22 Oct 2018, at 23:25, Kirill Marinushkin wrote: > > AFAIU, this patch is wrong. Please correct me, maybe I misunderstand something. > >> The problem that this patch seeks to resolve is that when userland asks for >> the delay > > The userspace asks not for delay, but for the pointer. The call in question is snd_pcm_delay. I presume this delay is calculated from knowledge of the stream position ?pos", the period (buffer?) number (and period/buffer size) and the snd_pcm_runtime structure?s ?delay" field (?runtime->delay?). > You modify the function, which is called `snd_bcm2835_pcm_pointer`. Here you are > supposed to increase `alsa_stream->pos` with the proper offset. Instead, you > imitate a delay, but in fact the delay is not increased. > > So, the proper solution should be to fix the reported pointer. I think there is a difficulty with this. The ?pos? pointer looks to have to be modulo the buffer size. This causes a problem, as I see it, in that if the calculated (pos + interpolated delay in bytes) is longer than the buffer size, it must be wrapped, but AFAIK we are unable to increment a buffer index used in the snd_pcm_delay calculation. Hence the calculation of the actual position would be wrong. This is why the snd_pcm_runtime delay field is used. On reflection, BTW, the patch assumes that the field's original value was zero ? that can be rectified. > As a result, > userspace will recieve the correct delay, instead of these crazy 10 ms. Just to point out that with the proposed patch, it appears that the correct delay is being reported, (apart, possibly, from any delay originally set in the snd_pcm_delay field, as mentioned above). All the best, Mike > FYI, there is >> a discussion of the effects of a downstream equivalent of this suggested patch >> at: >> https://github.com/raspberrypi/firmware/issues/1026#issuecomment-415746016. > > Thank you for the link, it clarified for me what you try to achieve. > > On 10/22/18 21:17, Mike Brady wrote: >> When the BCM2835 audio output is used, userspace sees a jitter up to 10ms >> in the audio position, aka "delay" -- the number of frames that must >> be output before a new frame would be played. >> Make this a bit nicer for userspace by interpolating the position >> using the CPU clock. >> The overhead is small -- an extra ktime_get() every time a GPU message >> is sent -- and another call and a few calculations whenever the delay >> is sought from userland. >> At 48,000 frames per second, i.e. approximately 20 microseconds per >> frame, it would take a clock inaccuracy of >> 20 microseconds in 10 milliseconds -- 2,000 parts per million -- >> to result in an inaccurate estimate, whereas >> crystal- or resonator-based clocks typically have an >> inaccuracy of 10s to 100s of parts per million. >> >> Signed-off-by: Mike Brady >> --- >> Changes in v2 -- remove inappropriate addition of SNDRV_PCM_INFO_BATCH flag >> >> .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 20 +++++++++++++++++++ >> .../vc04_services/bcm2835-audio/bcm2835.h | 1 + >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c >> index e66da11af5cf..9053b996cada 100644 >> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c >> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c >> @@ -74,6 +74,7 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream, >> atomic_set(&alsa_stream->pos, pos); >> >> alsa_stream->period_offset += bytes; >> + alsa_stream->interpolate_start = ktime_get(); >> if (alsa_stream->period_offset >= alsa_stream->period_size) { >> alsa_stream->period_offset %= alsa_stream->period_size; >> snd_pcm_period_elapsed(substream); >> @@ -243,6 +244,7 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) >> atomic_set(&alsa_stream->pos, 0); >> alsa_stream->period_offset = 0; >> alsa_stream->draining = false; >> + alsa_stream->interpolate_start = ktime_get(); >> >> return 0; >> } >> @@ -292,6 +294,24 @@ snd_bcm2835_pcm_pointer(struct snd_pcm_substream *substream) >> { >> struct snd_pcm_runtime *runtime = substream->runtime; >> struct bcm2835_alsa_stream *alsa_stream = runtime->private_data; >> + ktime_t now = ktime_get(); >> + >> + /* Give userspace better delay reporting by interpolating between GPU >> + * notifications, assuming audio speed is close enough to the clock >> + * used for ktime >> + */ >> + >> + if ((ktime_to_ns(alsa_stream->interpolate_start)) && >> + (ktime_compare(alsa_stream->interpolate_start, now) < 0)) { >> + u64 interval = >> + (ktime_to_ns(ktime_sub(now, >> + alsa_stream->interpolate_start))); >> + u64 frames_output_in_interval = >> + div_u64((interval * runtime->rate), 1000000000); >> + snd_pcm_sframes_t frames_output_in_interval_sized = >> + -frames_output_in_interval; >> + runtime->delay = frames_output_in_interval_sized; >> + } >> >> return snd_pcm_indirect_playback_pointer(substream, >> &alsa_stream->pcm_indirect, >> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h >> index e13435d1c205..595ad584243f 100644 >> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h >> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h >> @@ -78,6 +78,7 @@ struct bcm2835_alsa_stream { >> unsigned int period_offset; >> unsigned int buffer_size; >> unsigned int period_size; >> + ktime_t interpolate_start; >> >> struct bcm2835_audio_instance *instance; >> int idx; >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel at alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel