[alsa-devel] [alsa-lib][PATCH v3 0/4] add SNDRV_PCM_FORMAT_{S, U}20

[Takashi Iwai]

On Mon, 18 Dec 2017 15:20:59 +0100,
Takashi Sakamoto wrote:
> 
> Iwai-san,
> 
> On Dec 17 2017 17:37, Takashi Sakamoto wrote:
> > Hi,
> >
> > On Dec 14 2017 22:50, Maciej S. Szmigiero wrote:
> >> This format is similar to an existing 20-bit PCM format
> >> SNDRV_PCM_FORMAT_{S,U}20_3, however it occupies 4 bytes instead of 3.
> >>
> >> Changes from v1: Split the monolithic submission into separate
> >> commits. (Note that v2 wasn't tagged as such.)
> >>
> >> Changes from v2: Add commas at the end of two possible last entries of
> >> the snd_pcm_format_t enum so diffs will be more readable when new PCM
> >> formats are added in the future, remove asserts from
> >> snd_pcm_linear_{get,put}_index().
> >>
> >> Maciej S. Szmigiero (4):
> >>    asound.h: add SNDRV_PCM_FORMAT_{S,U}20
> >>    pcm: add and describe SND_PCM_FORMAT_{S,U}20
> >>    pcm: linear, route: handle linear formats with 20-bit sample on 4
> >>      bytes
> >>    pcm: plug: add SND_PCM_FORMAT_{S,U}20 to linear_preferred_formats
> >>
> >>   include/pcm.h          | 20 ++++++++++++++++++--
> >>   include/sound/asound.h |  9 +++++++++
> >>   src/pcm/pcm.c          | 10 ++++++++++
> >>   src/pcm/pcm_linear.c   | 14 +++++++++++---
> >>   src/pcm/pcm_local.h    |  4 ++++
> >>   src/pcm/pcm_misc.c     | 41 ++++++++++++++++++++++++++++++++++++++---
> >>   src/pcm/pcm_plug.c     | 11 +++++++++++
> >>   src/pcm/pcm_route.c    |  6 ++++--
> >>   src/pcm/plugin_ops.h   | 50
> >> ++++++++++++++++++++++++++++++++++++++++++++++----
> >>   9 files changed, 151 insertions(+), 14 deletions(-)
> >
> > I reviewed all of these four patches.
> >
> > Reviewed-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
> >
> >
> > I found some minor issues in current implementation of linear
> > interpolation in alsa-lib.
> >
> > * 'src/pcm/plugin_ops.h' includes some unused macros:
> >   * COPY_LABELS/COPY_END
> >   * GETU_LABELS/GETU_END
> >   * NORMS_LABELS/NORMS_END
> > * 'put32_labels' includes wrong comments for 18/20 bits formats.
> > * A reorder of entries in below tables may allow us to simplify
> >    implementation of snd_pcm_linear_get_index() and
> >    snd_pcm_linear_put_index().
> >   * get16_labels
> >   * put16_labels
> >   * get32_labels
> >   * put32_labels
> >
> > Anyway, the above issues are irrelevant to your patchset. Your work
> > can be merged to alsa-lib independently.
> 
> His patchset is good enough to me, except for failure to spin them
> with this cover letter (at least on my MUA). Could I ask you to deal
> with them for current alsa-lib master? The patchset are:
> 
> * [alsa-devel] [alsa-lib][PATCH v3 1/4] asound.h: add
> SNDRV_PCM_FORMAT_{S, U}20
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129371.html
> * [alsa-devel] [alsa-lib][PATCH v3 2/4] pcm: add and describe
> SND_PCM_FORMAT_{S, U}20
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129372.html
> * [alsa-devel] [alsa-lib][PATCH v3 3/4] pcm: linear, route: handle
> linear formats with 20-bit sample on 4 bytes
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129373.html
> * [alsa-devel] [alsa-lib][PATCH v3 4/4] pcm: plug: add
> SND_PCM_FORMAT_{S, U}20 to linear_preferred_formats
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129374.html

Don't worry, there is a time zone and a week end in the peaceful
world.  I applied the patches now.

The patch 4 is still not sure whether it's the best: actually 3 bytes
format isn't so rare, it's the standard format for USB-audio after
all.  So, only judging from "typical" format, the precedence must be
S24_3.  But since the conflicts between S20 and S24_3 are likely very
rare, practically seen, it doesn't matter which format is put earlier
in the list, so I took as is.


thanks,

Takashi