Re: [PATCH v4 01/16] ALSA: pcm: Introduce MSBITS subformat interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-11-16 4:10 PM, Jaroslav Kysela wrote:
On 16. 11. 23 12:22, Cezary Rojewski wrote:
From: Jaroslav Kysela <perex@xxxxxxxx>

Improve granularity of format selection for S32/U32 formats by adding
constants representing 20, 24 and MAX most significant bit >
The MAX means the maximum number of significant bits which can
the physical format hold. For 32-bit formats, MAX is related
to 32 bits. For 8-bit formats, MAX is related to 8 bits etc.

The drivers may use snd_pcm_hw_constraint_subformats with
a simple format -> subformats table.

I am afraid, the above sentence is no more correct and the current code does not follow my original idea. It's a bit step back to the initial code. But I admit that from the API POV, it's workable now (with the added refine mechanism for one format).

I'll update the commit message, indeed no longer up-to date. Will also try to explain the thought process which is currently missing, as you noted below.

As noted several times, this is not my preferred implementation (I would keep only the constraint function which will be called by drivers on demand in the ALSA PCM core code). The latest proposed simplification may be applied in the ASoC core (store S32_LE subformat mask in snd_soc_pcm_runtime and install this intersected constraint for FE PCM - user space). Something like ASoC core does for the msbits constraint.

If nobody else thinks that it's a good direction, please, add a note to the comment that this implementation (extend snd_pcm_hardware structure) is a compromise for the ASoC code with details.

I'd like to avoid complicating ASoC runtime structures as storing hw_params is not among their current task list, at least not currently.

But I'm open for any improvements in the future.

+        if (f == SNDRV_PCM_FORMAT_S32_LE)

Missing mask check: (f == SNDRV_PCM_FORMAT_S32_LE && *subformats)

Otherwise the MSBITS_MAX won't be set for S32_LE by default.

Ack.

+            m.bits[0] |= *subformats;

                 Jaroslav




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux