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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 1DE6EC52D71 for ; Fri, 9 Aug 2024 07:26:41 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=j6G6fV+w; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4WgFpR5Yyxz2yt0 for ; Fri, 9 Aug 2024 17:26:39 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=j6G6fV+w; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=linux.intel.com (client-ip=192.198.163.16; helo=mgamail.intel.com; envelope-from=pierre-louis.bossart@linux.intel.com; receiver=lists.ozlabs.org) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4WgFnc4Bk9z2yRZ for ; Fri, 9 Aug 2024 17:25:55 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1723188357; x=1754724357; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=EZEmD9DuOTren8Yb/4Eh1tc1aNVW50mWaRZilWNBwi8=; b=j6G6fV+wZqpztoJYxnESVtIHLCeSeD2qzWo9B54oXuDPYYMYt+ljwL4z NSfRKcJ0OoYXSB4QFM9g3fn09smed0Ds8IFn/6Bm5SYYJOPaxHod1Vzn6 jLMJ2Rq2SpxlY8zs6kOLXQLjN7J/Qx6QDGFlpluZDxdiv7mKq5ycjxuCw 5sGPlA1Kmm1FhWdQQI4xB6ClYVLUq2+Q4ULWRdh03/wMFXfH6c9pICjcD TpFF0oDB46yWSCxCc8PO3vRwyV9tgclu6xNr71xIPB1H5mPK/eWYI6R5T FDfRFSY2mRMuGPWP4QV6yIB8Cy/aZQK2KADwm3bsosbD2bRUwhz9scRiE g==; X-CSE-ConnectionGUID: h0LrJztzRLeY0uqy2cWiUw== X-CSE-MsgGUID: 2h1b5hRVQBu+aUxhHlZTqg== X-IronPort-AV: E=McAfee;i="6700,10204,11158"; a="12929856" X-IronPort-AV: E=Sophos;i="6.09,275,1716274800"; d="scan'208";a="12929856" Received: from orviesa010.jf.intel.com ([10.64.159.150]) by fmvoesa110.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2024 00:25:50 -0700 X-CSE-ConnectionGUID: gLUemNmQTBu18Mf1kZVGHQ== X-CSE-MsgGUID: Q8VA64rwRrKZxNZdPwAWKA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,275,1716274800"; d="scan'208";a="57360819" Received: from kniemiec-mobl1.ger.corp.intel.com (HELO [10.245.246.249]) ([10.245.246.249]) by orviesa010-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2024 00:25:45 -0700 Message-ID: Date: Fri, 9 Aug 2024 09:19:58 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH 1/6] ALSA: compress: add Sample Rate Converter codec support To: Jaroslav Kysela , Shengjiu Wang References: <1722940003-20126-1-git-send-email-shengjiu.wang@nxp.com> <1722940003-20126-2-git-send-email-shengjiu.wang@nxp.com> <116041ee-7139-4b77-89be-3a68f699c01b@perex.cz> <930bb152-860a-4ec5-9ef0-1c96f554f365@linux.intel.com> Content-Language: en-US From: Pierre-Louis Bossart In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, Xiubo.Lee@gmail.com, lgirdwood@gmail.com, Shengjiu Wang , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-sound@vger.kernel.org, tiwai@suse.com, nicoleotsuka@gmail.com, vkoul@kernel.org, broonie@kernel.org, festevam@gmail.com Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" >>>> Then there's the issue of parameters, we chose to only add parameters >>>> for standard encoders/decoders. Post-processing is highly specific and >>>> the parameter definitions varies from one implementation to another - >>>> and usually parameters are handled in an opaque way with binary >>>> controls. This is best handled with a UUID that needs to be known only >>>> to applications and low-level firmware/hardware, the kernel code should >>>> not have to be modified for each and every processing and to add new >>>> parameters. It just does not scale and it's unmaintainable. >>>> >>>> At the very least if you really want to use this compress API, >>>> extend it >>>> to use a non-descript "UUID-defined" type and an opaque set of >>>> parameters with this UUID passed in a header. >>> >>> We don't need to use UUID-defined scheme for simple (A)SRC >>> implementation. As I noted, the specific runtime controls may use >>> existing ALSA control API. >> >> "Simple (A)SRC" is an oxymoron. There are multiple ways to define the >> performance, and how the drift estimator is handled. There's nothing >> simple if you look under the hood. The SOF implementation has for >> example those parameters: >> >> uint32_t source_rate;           /**< Define fixed source rate or */ >>                 /**< use 0 to indicate need to get */ >>                 /**< the rate from stream */ >> uint32_t sink_rate;             /**< Define fixed sink rate or */ >>                 /**< use 0 to indicate need to get */ >>                 /**< the rate from stream */ >> uint32_t asynchronous_mode;     /**< synchronous 0, asynchronous 1 */ >>                 /**< When 1 the ASRC tracks and */ >>                 /**< compensates for drift. */ >> uint32_t operation_mode;        /**< push 0, pull 1, In push mode the */ >>                 /**< ASRC consumes a defined number */ >>                 /**< of frames at input, with varying */ >>                 /**< number of frames at output. */ >>                 /**< In pull mode the ASRC outputs */ >>                 /**< a defined number of frames while */ >>                 /**< number of input frames varies. */ >> >> They are clearly different from what is suggested above with a 'ratio- >> mod'. > > I don't think so. The proposed (A)SRC for compress-accel is just one > case for the above configs where the input is known and output is > controlled by the requested rate. The I/O mechanism is abstracted enough > in this case and the driver/hardware/firmware must follow it. ASRC is usually added when the nominal rates are known but the clock sources differ and the drift needs to be estimated at run-time and the coefficients or interpolation modified dynamically If the ratio is known exactly and there's no clock drift, then it's a different problem where the filter coefficients are constant. >> Same if you have a 'simple EQ'. there are dozens of ways to implement >> the functionality with FIR, IIR or a combination of the two, and >> multiple bands. >> >> The point is that you have to think upfront about a generic way to pass >> parameters. We didn't have to do it for encoders/decoders because we >> only catered to well-documented standard solutions only. By choosing to >> support PCM processing, a new can of worms is now open. >> >> I repeat: please do not make the mistake of listing all processing with >> an enum and a new structure for parameters every time someone needs a >> specific transform in their pipeline. We made that mistake with SOF and >> had to backtrack rather quickly. The only way to scale is an identifier >> that is NOT included in the kernel code but is known to higher and >> lower-levels only. > > There are two ways - black box (UUID - as you suggested) - or well > defined purpose (abstraction). For your example 'simple EQ', the > parameters should be the band (frequency range) volume values. It's > abstract and the real filters (resp. implementation) used behind may > depend on the hardware/driver capabilities. Indeed there is a possibility that the parameters are high-level, but that would require firmware or hardware to be able to generate actual coefficients from those parameters. That usually requires some advanced math which isn't necessarily obvious to implement with fixed-point hardware. > From my view, the really special cases may be handled as black box, but > others like (A)SRC should follow some well-defined abstraction IMHO to > not force user space to handle all special cases. I am not against the high-level abstractions, e.g. along the lines of what Android defined: https://developer.android.com/reference/android/media/audiofx/AudioEffect That's not sufficient however, we also need to make sure there's an ability to provide pre-computed coefficients in an opaque manner for processing that doesn't fit in the well-defined cases. In practice there are very few 3rd party IP that fits in well-defined cases, everyone has secret-sauce parameters and options.