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 849CEC77B78 for ; Thu, 4 May 2023 14:51:30 +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 305162198; Thu, 4 May 2023 16:50:37 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 305162198 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1683211887; bh=CtJFQF393BxENgpup0ny5rdz4wXP5+RBQ2N65ms3GOQ=; 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=siGoqLOV85//pVJ5ErAuwPGK+1Uax3cBT6+9Fhihhvkmn6cwF55EzioVlDsr0P5ZS 05pQz7ybFCXZiCh9M3TkaJ8oWaVdTXXW93MQUJSVegHVj+jhovZWMJ+FlXcAeXVjFF rSD6S0l0scXsBibtm9dMip3rEyXAdRxe9EkDqlk0= Received: from mailman-core.alsa-project.org (mailman-core.alsa-project.org [10.254.200.10]) by alsa1.perex.cz (Postfix) with ESMTP id E716AF8052D; Thu, 4 May 2023 16:50:11 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id B5D52F8052D; Thu, 4 May 2023 16:50:07 +0200 (CEST) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (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 A85A0F80520 for ; Thu, 4 May 2023 16:49:59 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz A85A0F80520 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=wXy6dQ1D; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=jgu889lr 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 B7E011FD79; Thu, 4 May 2023 14:49:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1683211798; 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=vQLOzvli3hN95pyfrGZ8BVwRzTVb9kcnRxPMHW+VCEU=; b=wXy6dQ1DwOAP04VMC2zzwSM9Zn7pOL4NHCYhrth9LLa0uGdWz5A042M51NueRbD6is2B95 qQrwL88xGeMn0Z8G9+r3t/FuitzCfR6AQznFlyd1v6r6B+1mrQ0gtHDlPBCRrqv+qw4Ftz 8s3XvC7RLWjtXLe39li+OEpdLhhNxWg= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1683211798; 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=vQLOzvli3hN95pyfrGZ8BVwRzTVb9kcnRxPMHW+VCEU=; b=jgu889lrSMoRIeeGTpDeRNM6LMqgbgSjDJUGwjbHj1VcbKL+nWRJ8y9+vuQc8u9NLgV1q7 hd1mGkKR5kF9+gCA== 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 942BE133F7; Thu, 4 May 2023 14:49:58 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 8WIwIxbGU2S6DgAAMHmgww (envelope-from ); Thu, 04 May 2023 14:49:58 +0000 Date: Thu, 04 May 2023 16:49:58 +0200 Message-ID: <877ctokv6x.wl-tiwai@suse.de> From: Takashi Iwai To: Oswald Buddenhagen Subject: Re: [PATCH] ALSA: pcm: fix snd_pcm_playback_silence() with free-running mode In-Reply-To: References: <20230504130007.2208916-1-oswald.buddenhagen@gmx.de> <87cz3gkyz9.wl-tiwai@suse.de> 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: 7SJRZZOK7INJ2BNJEJZ2MKMB3VAJZLNA X-Message-ID-Hash: 7SJRZZOK7INJ2BNJEJZ2MKMB3VAJZLNA 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-devel@alsa-project.org 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 Thu, 04 May 2023 15:54:15 +0200, Oswald Buddenhagen wrote: > > On Thu, May 04, 2023 at 03:33:01PM +0200, Jaroslav Kysela wrote: > > On 04. 05. 23 15:28, Takashi Iwai wrote: > >> Honestly speaking, this is really hard to review. > > >> As most of changes here are the revert of the previous commit, > >> > no they aren't. Erm, that is a big part of problems. We don't see clearly what part is the revert to the old logic and what is the actual fix. You mixed things, and it's really hard to follow. > >> I'd rather like to get it > >> reverted whole once, and re-apply the proper fix gradually. > > > > I fully agree here. Takashi, please, revert the broken patch right now. > > > actually reverting would include reverting the comments, which would > be just plain stupid. A whole revert sounds too much, but it makes the changes more straightforward afterwards. This is the biggest win. We want to see the fix applied in each commit in the right way. Not in a mixture. > > I think that the review and improving the code may take some days. > > > i'm not going to deliver anything more on that matter just to satisfy > some arbitrary process. It's not "some arbitrary process". The patch review is _the_ most important process, and if a patch makes it difficult, it implies that it's already fundamentally bad. > i think the patch does the right thing, with > the right granularity, and i'm not going to spend time breaking my > head on producing broken intermediate states which i cannot properly > reason about due to their internal inconsistency. > > you can "rebase" my patch by checking it out on top of a partial > revert, but you need to come up with your own commit message then. and > i think that it would be utterly counterproductive. viewing the diff > for review purposes may make sense, though. Sorry, that doesn't work. The review is done upon the patch, and if this patch can't be reviewed easily, it's simply no-go. Again, the problem is the mixture; it partially reverts to the original code while it modifies some part in some other way. For example, as already pointed, only from this patch, it's not clear whether the handling of silence_start in the patched code is OK or not. That's no part of revert, and I can't judge whether it's the correct and intended change or an oversight. By reverting the whole and reapplying fixes -- although it'll need more steps -- we can see more clearly what change fixes which part. The patch granularity, the patch description, rules and whatever, all of these are just for reviewing the patches properly; it results in better understanding and in the good code in the end. Takashi