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

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

 



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@xxxxxxxxxxxxx>


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-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-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-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-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


Thanks

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel




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

  Powered by Linux