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=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS 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 77A40C43441 for ; Tue, 13 Nov 2018 16:50:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3EBA6216FD for ; Tue, 13 Nov 2018 16:50:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3EBA6216FD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de 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 S1731230AbeKNCtf (ORCPT ); Tue, 13 Nov 2018 21:49:35 -0500 Received: from mx2.suse.de ([195.135.220.15]:46992 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726517AbeKNCte (ORCPT ); Tue, 13 Nov 2018 21:49:34 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 5EB87B634; Tue, 13 Nov 2018 16:50:39 +0000 (UTC) Date: Tue, 13 Nov 2018 17:50:37 +0100 Message-ID: From: Takashi Iwai To: Mike Brady Cc: Stefan Wahren , devel@driverdev.osuosl.org, alsa-devel@alsa-project.org, f.fainelli@gmail.com, Eric Anholt , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, julia.lawall@lip6.fr, linux-rpi-kernel@lists.infradead.org, nishka.dasgupta_ug18@ashoka.edu.in, Kirill Marinushkin , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2] ARM: staging: bcm2835-audio: interpolate audio delay In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/26 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 11 Nov 2018 19:21:29 +0100, Mike Brady wrote: > > /* hardware definition */ > static const struct snd_pcm_hardware snd_bcm2835_playback_hw = { > .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | > SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | > - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR), > + SNDRV_PCM_INFO_BATCH | SNDRV_PCM_INFO_DRAIN_TRIGGER | > + SNDRV_PCM_INFO_SYNC_APPLPTR), As already mentioned, the addition of SNDRV_PCM_INFO_BATCH should be irrelevant with this change. Please create another patch to add this and clarify it in the changelog. > diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c > index 4e6110d778bd..574df7d7a1fa 100644 > --- a/sound/core/pcm_lib.c > +++ b/sound/core/pcm_lib.c > @@ -229,19 +229,38 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream, > (runtime->audio_tstamp_report.actual_type == > SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT)) { > > - /* > - * provide audio timestamp derived from pointer position > - * add delay only if requested > - */ > + // provide audio timestamp derived from pointer position > > audio_frames = runtime->hw_ptr_wrap + runtime->status->hw_ptr; > > - if (runtime->audio_tstamp_config.report_delay) { > + /* > + * If the runtime->delay is greater than zero, it's a > + * genuine delay, e.g. a delay due to a hardware fifo. > + * > + * But if the runtime->delay is less than zero, it's an > + * interpolated estimate of the number of frames output > + * since the hardware pointer was last updated. > + * > + * It would be calculated in the pointer callback. > + * For example, for the bcm_2835 driver, it is calculated in > + * snd_bcm2835_pcm_pointer(). > + */ > + > + if (runtime->delay < 0) { > + // The delay is an interpolated estimate... > if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) > - audio_frames -= runtime->delay; > - else > - audio_frames += runtime->delay; > + audio_frames += runtime->delay; > + } else { > + // The delay is a real delay. Add it if requested. > + if (runtime->audio_tstamp_config.report_delay) { > + if (substream->stream == > + SNDRV_PCM_STREAM_PLAYBACK) > + audio_frames -= runtime->delay; > + else > + audio_frames += runtime->delay; > + } > } Well, honestly speaking, I'm really not fond of changing the PCM core just for this. Can we make bcm audio driver providing the finer pointer update instead? If we have a module option to disable that behavior, it's an enough excuse in case anyone really cares about the accuracy. thanks, Takashi