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 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1A7F7C77B7D for ; Fri, 5 May 2023 09:58:33 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id B755B2C2F; Fri, 5 May 2023 11:57:40 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz B755B2C2F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1683280710; bh=NmOEYx3xLwij3Qx+9Tkxg/Yz+nNw1DVj8H9wxxFPZyI=; h=Date:From:To:Subject:In-Reply-To:References:CC:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=SUaoub4c+AhWCwl8af6pTd48rROL8SaYOGkG56fge2LBxiQ11GkQ0lHI3JlmVHrjG DsLYVi/TWSAR1mGMEKCPiUQKvVnXubRZAA1DhAErM0GPb9tAOoeGsXRhnGQrjGHfAt stZESgl7punRBA/rzooeVlTTUAaR2Z+0QsB8Ow4M= Received: from mailman-core.alsa-project.org (mailman-core.alsa-project.org [10.254.200.10]) by alsa1.perex.cz (Postfix) with ESMTP id 71010F8052D; Fri, 5 May 2023 11:57:15 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 8E7D7F8052D; Fri, 5 May 2023 11:57:11 +0200 (CEST) Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 7C465F80520 for ; Fri, 5 May 2023 11:57:01 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 7C465F80520 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key, unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=GPaDWZwl; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=tXyTCycP Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id C577F1F8C1; Fri, 5 May 2023 09:57:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1683280620; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mDGuywn6RQt+u2S5yCJ9o9CNhs33iQIe6+Q0HbQxMEY=; b=GPaDWZwl9jHdOqRRGVmie5aeSb8/Zk19gglAD8Bz2enEUKKEZvSt2ELsgVOogLkj35VBmO bcAhkBwfbtVo4A9LaBw5tQVhzFsOZ5hwb1WyGeub5mCBCE71q3tDOR3Grugy0zdhN9dS2m 2f9kjjIDwaDnCGtGqZ74KJNuXlI2X7Y= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1683280620; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mDGuywn6RQt+u2S5yCJ9o9CNhs33iQIe6+Q0HbQxMEY=; b=tXyTCycPZ9VWNCzfmnooPE7wj5GdQ86bGIwjvjd7muJzi988LBUd47nEQbC7j4qd6Eg7Xy zykZpXj/bOBPz4BA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 9B6DE13513; Fri, 5 May 2023 09:57:00 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id u54GJezSVGTpaQAAMHmgww (envelope-from ); Fri, 05 May 2023 09:57:00 +0000 Date: Fri, 05 May 2023 11:57:00 +0200 Message-ID: <87y1m3hzir.wl-tiwai@suse.de> From: Takashi Iwai To: Jaroslav Kysela Subject: Re: [PATCH 3/5] ALSA: pcm: fix playback silence - correct the incremental silencing In-Reply-To: <20230505073813.1219175-4-perex@perex.cz> References: <20230505073813.1219175-1-perex@perex.cz> <20230505073813.1219175-4-perex@perex.cz> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Message-ID-Hash: Q6VU4X4IYUX5MG7EQ3EA4GV3GD6N7JXZ X-Message-ID-Hash: Q6VU4X4IYUX5MG7EQ3EA4GV3GD6N7JXZ X-MailFrom: tiwai@suse.de X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-alsa-devel.alsa-project.org-0; header-match-alsa-devel.alsa-project.org-1; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: ALSA development , Oswald Buddenhagen , Jeff Chua X-Mailman-Version: 3.3.8 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, 05 May 2023 09:38:11 +0200, Jaroslav Kysela wrote: > > The incremental silencing was broken with the threshold mode. The silenced > area was smaller than expected in some cases. The updated area starts > at runtime->silence_start + runtime->silence_filled position not > only at runtime->silence_start in this mode. > > Unify the runtime->silence_start use for all cases (threshold and top-up). > > Suggested-by: Oswald Buddenhagen > Signed-off-by: Jaroslav Kysela While this change itself follows the original code, and is fine from the code-refactoring POV. But, the difficulty of the code (even after this patch) is that the filling behavior is completely different between the threshold and the fill-up modes, and we still try to use the similar code. When reconsidering what we actually need, you can notice that, in the fill-up mode, we don't have to keep tracking silence_start and silence_size at all. Namely, in the fill-up mode, what we need are: - at init, fill silence in the unused buffer: ofs = runtime->control->appl_ptr % runtime->buffer_size; frames = snd_pcm_playback_avail(runtime); fill_silence_in_loop(ofs, frames); - at each incremental hw_ptr update, fill the area with silence: ofs = runtime->status->hw_ptr % runtime->buffer_size; frames = new_hw_ptr - runtime->status->hw_ptr; if (frames < 0) frames += runtime->boundary; fill_silence_in_loop(ofs, frames); That's all, and far simpler than keeping silence_start and silence_filled. (I really had hard time to understand why filling at silence_start + silence_filled in the incremental mode works correctly...) I might have overlooked something and there can be a bit more room for optimization, but the point is that unifying the code for two behavior isn't always good. Treating separately can be sometimes easier. thanks, Takashi