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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DCBAC433F5 for ; Tue, 12 Oct 2021 18:03:47 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id F39E8610CC for ; Tue, 12 Oct 2021 18:03:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F39E8610CC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 8B196829; Tue, 12 Oct 2021 20:02:53 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 8B196829 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1634061823; bh=96LyfBC+P7JSHIS5B/bKSZdNPEOF7TgWiVCgP/i7MbM=; h=Subject:To:References:From:Date:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=PqUVe7duMm9HVBR2p5fUPF96XYKEwJEXygZO/CoSgL0aanT7F9RBb/slmEhwt/yMU JOkttP1H4yGx6ew+OAKMk/Tnznj0Gz2lHrOiO+Ar08waW7s8ZOrhIwBx9QRRtkK04+ Q7i8NKwi5Kdg/3YJ0nkKDyj/BUXS4AY2NWMFD/l4= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id EF166F800CB; Tue, 12 Oct 2021 20:02:52 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id A379CF80212; Tue, 12 Oct 2021 20:02:50 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 4D423F800CB for ; Tue, 12 Oct 2021 20:02:46 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 4D423F800CB X-IronPort-AV: E=McAfee;i="6200,9189,10135"; a="288101818" X-IronPort-AV: E=Sophos;i="5.85,368,1624345200"; d="scan'208";a="288101818" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Oct 2021 11:02:10 -0700 X-IronPort-AV: E=Sophos;i="5.85,368,1624345200"; d="scan'208";a="486483857" Received: from csharp1-mobl.amr.corp.intel.com (HELO [10.213.183.127]) ([10.213.183.127]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Oct 2021 11:02:09 -0700 Subject: Re: [PATCH v2 1/3] ALSA: pcm: introduce INFO_NO_REWINDS flag To: Takashi Iwai References: <20211004162423.85323-1-pierre-louis.bossart@linux.intel.com> <20211004162423.85323-2-pierre-louis.bossart@linux.intel.com> <1ae2012b-d6bd-77ce-0a9e-98aec4d0f868@linux.intel.com> From: Pierre-Louis Bossart Message-ID: <51b47edf-cec2-6f11-17ee-3b8bca7e7c37@linux.intel.com> Date: Tue, 12 Oct 2021 13:02:07 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Cc: alsa-devel@alsa-project.org, broonie@kernel.org, P9ter Ujfalusi , Ranjani Sridharan , Kai Vehmanen X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" >> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c >> index a144a3f68e9e..e839459916ca 100644 >> --- a/sound/core/pcm_lib.c >> +++ b/sound/core/pcm_lib.c >> @@ -2127,11 +2127,30 @@ int pcm_lib_apply_appl_ptr(struct >> snd_pcm_substream *substream, >> { >> struct snd_pcm_runtime *runtime = substream->runtime; >> snd_pcm_uframes_t old_appl_ptr = runtime->control->appl_ptr; >> + snd_pcm_sframes_t diff; >> int ret; >> >> if (old_appl_ptr == appl_ptr) >> return 0; >> >> + /* >> + * check if a rewind is requested by the application, after >> + * verifying the new appl_ptr is in the 0..boundary range >> + */ >> + if (substream->runtime->info & SNDRV_PCM_INFO_NO_REWINDS) { >> + if (appl_ptr >= runtime->boundary) >> + appl_ptr -= runtime->boundary; > > The boundary check can (or should) be done unconditionally. > It was too naive to assume a sane appl_ptr passed always. > And, it can rather return an error. So, > > if (appl_ptr >= runtime->boundary) > return -EINVAL; ok, but that would be a separate patch then since it impacts all users, even without the NO_REWINDS. > > /* check if a rewind is requested by the application */ > if (substream->runtime->info & SNDRV_PCM_INFO_NO_REWINDS) { > diff = appl_ptr - old_appl_ptr; > .... > >> + if (diff >= 0) { >> + if (diff > runtime->buffer_size) >> + return 0; >> + } else { >> + if (runtime->boundary + diff > runtime->buffer_size) >> + return 0; > > I'm not sure whether we should return 0 here. In snd_pcm_rewind() it > returns 0 due to application breakage, though. We could return -EINVAL indeed, that would keep the work-around in place for PulseAudio. Even for other uses, it's not so bad: the selection of NO_REWINDS is an opt-in, and if a rewind still occurs a big fail would help detect a configuration issue.