<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<blockquote type="cite"
cite="mid:2dbc0b8b-2ea3-19e5-cc19-ad2f59b213c1@linux.intel.com">
<p>Hey!<br>
<br>
Thanks for Your input! <br>
<br>
I've created that patch because our validation is actually
checking if values<br>
in SDnFMT are matching their expectations, and they've found it
indicates 32 bits in 32 container while playing 24 bits in 32
container.<br>
This could be fixed without touching checks of maxbps:<br>
<br>
</p>
<pre> switch (snd_pcm_format_width(format)) {
case 8:
val |= AC_FMT_BITS_8;
break;
case 16:
val |= AC_FMT_BITS_16;
break;
case 20:
val |= AC_FMT_BITS_20;
break;
case 24:
if (maxbps >= 24)
val |= AC_FMT_BITS_24;
else
val |= AC_FMT_BITS_20;
break;
case 32:
if (maxbps >= 32 || format == SNDRV_PCM_FORMAT_FLOAT_LE)
val |= AC_FMT_BITS_32;
else if (maxbps >= 24)
val |= AC_FMT_BITS_24;
else
val |= AC_FMT_BITS_20;
break;
default:
return 0;
}
</pre>
<p><br>
I've simplified that because maxbps seems redundant here -
thansk for catching Kai!<br>
Although reason of usage maxbps is still not clear (at least
for me).</p>
<p>On 8/25/2020 10:25 AM, Takashi Iwai wrote:<br>
</p>
<blockquote type="cite" cite="mid:s5h5z976siz.wl-tiwai@suse.de">
<pre class="moz-quote-pre" wrap="">On Mon, 24 Aug 2020 14:16:26 +0200,
Kai Vehmanen wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Hey,
On Mon, 24 Aug 2020, Pawel Harlozinski wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Set SDnFMT depending on which format was given, as maxbps only describes container size.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">hmm, I'm not entirely sure that is correct. Usage may be a bit varied, but
most places in existing code, "maxbps" is treated as number of significant
bits, not the container size. E.g. in hdac_hda.c:
» if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
» » maxbps = dai->driver->playback.sig_bits;
» else
» » maxbps = dai->driver->capture.sig_bits;
It would seem "maxbps" is a bit superfluous given the same information can
be relayed in "format" as well. But currently it's still used. E.g. if you
look at snd_hdac_query_supported_pcm(), if codec reports 24bit support,
format is always set to SNDRV_PCM_FMTBIT_S32_LE even if only 24bit are valid.</pre>
</blockquote>
</blockquote>
So, for me looks like place where we can align with actual format,
right ? <br>
<blockquote type="cite" cite="mid:s5h5z976siz.wl-tiwai@suse.de">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> So snd_pcm_format_width() will not return the expected significant
bits info, but you have to use "maxbps". So original code seems correct
(or at least you'd need to update both places).</pre>
</blockquote>
</blockquote>
<br>
<blockquote type="cite" cite="mid:s5h5z976siz.wl-tiwai@suse.de">
<pre class="moz-quote-pre" wrap="">Hm, we need to check the call pattern, then. The maxbps passed to
this function was supposed to be the value obtained from
snd_hdac_query_supported_pcm(), i.e. the codec capability.</pre>
</blockquote>
Here I'm also not sure if we should just "cut" format in
snd_hdac_calc_stream_format (eg. 32 to 24) if codec does not
support 32?<br>
<br>
<blockquote type="cite" cite="mid:s5h5z976siz.wl-tiwai@suse.de">
<pre class="moz-quote-pre" wrap="">But, basically this patch wouldn't change any practical behavior. In
the current code, snd_pcm_format_width() can be never 20 or 24,
because the 24 and 24bit supports are also with SNDRV_PCM_FMT_S32_LE.
That is, the cases 20 and 24 there are superfluous from the
beginning (although the checks of maxbps are still needed
Instead, what we could improve is:
- Set up the proper msbits hw_constraint to reflect the maxbps value
- Choose the right AC_FMT_BITS_* depending on the hw_params msbitsWe may change the query function not to return a single maxbps value
but rather storing the raw PCM parameter value (AC_SUPPCM_*), and pass
it at re-encoding the format value, too, if we want to make
thanks,
Takashi
</pre>
</blockquote>
<blockquote type="cite" cite="mid:s5h5z976siz.wl-tiwai@suse.de"> </blockquote>
</blockquote>
</body>
</html>