You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/11/01 17:59:59 UTC
[GitHub] [incubator-nuttx] tmedicci opened a new pull request, #7501: esp32/i2s: use internal buffer to handle multiple audio formats
tmedicci opened a new pull request, #7501:
URL: https://github.com/apache/incubator-nuttx/pull/7501
## Summary
1. Creates an internal buffer handled by the ESP32's I2S driver to be able to make the necessary manipulations to correctly fit the characteristics of the I2S's FIFO (byte alignment, L-R channels etc).
2. Stops and restarts I2S peripheral every time master clock and sample rate are changed.
3. Other small fixes (like using `nxmutex` instead `nxsem` and formatting) and fixing CS4344's configure function when `i2s_mclkfrequency` method is not available.
## Impact
Supporting multiple sample rates, bit widths and mono/stereo sources.
## Testing
Hardware testing with ESP32-DevKitC + CS4344 and internal CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7501: esp32/i2s: use internal buffer to handle multiple audio formats
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7501:
URL: https://github.com/apache/incubator-nuttx/pull/7501#discussion_r1010940988
##########
arch/xtensa/src/esp32/esp32_i2s.c:
##########
@@ -711,27 +736,114 @@ static int i2s_txdma_setup(struct esp32_i2s_s *priv,
/* Get the transfer information, accounting for any data offset */
- samp = (uintptr_t)&apb->samp[apb->curbyte];
- nbytes = apb->nbytes - apb->curbyte;
+ const apb_samp_t sample_size = priv->data_width / 8;
+ samp = (uintptr_t)&apb->samp[apb->curbyte];
+ samp_size = (apb->nbytes - apb->curbyte) + priv->tx.carry.bytes;
+ carry_size = samp_size % sample_size;
+
+ /* Padding contains the size (in bytes) to be skipped on the
+ * internal buffer do provide 1) the ability to handle mono
+ * audio files and 2) to correctly fill the buffer to 16 or 32-bits
+ * aligned positions
+ */
+
+ padding = priv->channels == 1 ?
+ ((priv->data_width <= I2S_DATA_BIT_WIDTH_16BIT ?
+ I2S_DATA_BIT_WIDTH_16BIT : I2S_DATA_BIT_WIDTH_32BIT) / 8) : 0;
+
+ padding += priv->data_width != I2S_DATA_BIT_WIDTH_16BIT ?
+ (priv->data_width != I2S_DATA_BIT_WIDTH_32BIT ? 1 : 0) : 0;
+
+ /* Copy audio data into internal buffer */
+
+ bfcontainer->buf = (uint8_t *)calloc(bfcontainer->nbytes, 1);
+
+ data_copied = 0;
+ buf = bfcontainer->buf + padding;
+
+ if (priv->tx.carry.bytes)
+ {
+ memcpy(buf, (const void *)&priv->tx.carry.value,
+ priv->tx.carry.bytes);
+ buf += priv->tx.carry.bytes;
+ data_copied += priv->tx.carry.bytes;
+ memcpy(buf, (const void *)samp,
Review Comment:
```suggestion
memcpy(buf, samp,
```
##########
arch/xtensa/src/esp32/esp32_i2s.c:
##########
@@ -711,27 +736,114 @@ static int i2s_txdma_setup(struct esp32_i2s_s *priv,
/* Get the transfer information, accounting for any data offset */
- samp = (uintptr_t)&apb->samp[apb->curbyte];
- nbytes = apb->nbytes - apb->curbyte;
+ const apb_samp_t sample_size = priv->data_width / 8;
+ samp = (uintptr_t)&apb->samp[apb->curbyte];
+ samp_size = (apb->nbytes - apb->curbyte) + priv->tx.carry.bytes;
+ carry_size = samp_size % sample_size;
+
+ /* Padding contains the size (in bytes) to be skipped on the
+ * internal buffer do provide 1) the ability to handle mono
+ * audio files and 2) to correctly fill the buffer to 16 or 32-bits
+ * aligned positions
+ */
+
+ padding = priv->channels == 1 ?
+ ((priv->data_width <= I2S_DATA_BIT_WIDTH_16BIT ?
+ I2S_DATA_BIT_WIDTH_16BIT : I2S_DATA_BIT_WIDTH_32BIT) / 8) : 0;
+
+ padding += priv->data_width != I2S_DATA_BIT_WIDTH_16BIT ?
+ (priv->data_width != I2S_DATA_BIT_WIDTH_32BIT ? 1 : 0) : 0;
+
+ /* Copy audio data into internal buffer */
+
+ bfcontainer->buf = (uint8_t *)calloc(bfcontainer->nbytes, 1);
+
+ data_copied = 0;
+ buf = bfcontainer->buf + padding;
+
+ if (priv->tx.carry.bytes)
+ {
+ memcpy(buf, (const void *)&priv->tx.carry.value,
Review Comment:
```suggestion
memcpy(buf, &priv->tx.carry.value,
```
##########
arch/xtensa/src/esp32/esp32_i2s.c:
##########
@@ -711,27 +736,114 @@ static int i2s_txdma_setup(struct esp32_i2s_s *priv,
/* Get the transfer information, accounting for any data offset */
- samp = (uintptr_t)&apb->samp[apb->curbyte];
- nbytes = apb->nbytes - apb->curbyte;
+ const apb_samp_t sample_size = priv->data_width / 8;
+ samp = (uintptr_t)&apb->samp[apb->curbyte];
+ samp_size = (apb->nbytes - apb->curbyte) + priv->tx.carry.bytes;
+ carry_size = samp_size % sample_size;
+
+ /* Padding contains the size (in bytes) to be skipped on the
+ * internal buffer do provide 1) the ability to handle mono
+ * audio files and 2) to correctly fill the buffer to 16 or 32-bits
+ * aligned positions
+ */
+
+ padding = priv->channels == 1 ?
+ ((priv->data_width <= I2S_DATA_BIT_WIDTH_16BIT ?
+ I2S_DATA_BIT_WIDTH_16BIT : I2S_DATA_BIT_WIDTH_32BIT) / 8) : 0;
+
+ padding += priv->data_width != I2S_DATA_BIT_WIDTH_16BIT ?
+ (priv->data_width != I2S_DATA_BIT_WIDTH_32BIT ? 1 : 0) : 0;
+
+ /* Copy audio data into internal buffer */
+
+ bfcontainer->buf = (uint8_t *)calloc(bfcontainer->nbytes, 1);
+
+ data_copied = 0;
+ buf = bfcontainer->buf + padding;
+
+ if (priv->tx.carry.bytes)
+ {
+ memcpy(buf, (const void *)&priv->tx.carry.value,
+ priv->tx.carry.bytes);
+ buf += priv->tx.carry.bytes;
+ data_copied += priv->tx.carry.bytes;
+ memcpy(buf, (const void *)samp,
+ (sample_size - priv->tx.carry.bytes));
+ buf += (sample_size - priv->tx.carry.bytes + padding);
+ samp += (sample_size - priv->tx.carry.bytes);
+ data_copied += (sample_size - priv->tx.carry.bytes);
+ }
+
+ /* If there is no need to add padding bytes, the memcpy may be done at
+ * once. Otherwise, the operation must add the padding bytes to each
+ * sample in the internal buffer
+ */
+
+ if (padding)
+ {
+ while (data_copied < (samp_size - carry_size))
+ {
+ memcpy(buf, (const void *)samp, sample_size);
+ buf += (sample_size + padding);
+ samp += sample_size;
+ data_copied += sample_size;
+ }
+ }
+ else
+ {
+ memcpy(buf, (const void *)samp,
Review Comment:
ditto
##########
arch/xtensa/src/esp32/esp32_i2s.c:
##########
@@ -711,27 +736,114 @@ static int i2s_txdma_setup(struct esp32_i2s_s *priv,
/* Get the transfer information, accounting for any data offset */
- samp = (uintptr_t)&apb->samp[apb->curbyte];
- nbytes = apb->nbytes - apb->curbyte;
+ const apb_samp_t sample_size = priv->data_width / 8;
+ samp = (uintptr_t)&apb->samp[apb->curbyte];
+ samp_size = (apb->nbytes - apb->curbyte) + priv->tx.carry.bytes;
+ carry_size = samp_size % sample_size;
+
+ /* Padding contains the size (in bytes) to be skipped on the
+ * internal buffer do provide 1) the ability to handle mono
+ * audio files and 2) to correctly fill the buffer to 16 or 32-bits
+ * aligned positions
+ */
+
+ padding = priv->channels == 1 ?
+ ((priv->data_width <= I2S_DATA_BIT_WIDTH_16BIT ?
+ I2S_DATA_BIT_WIDTH_16BIT : I2S_DATA_BIT_WIDTH_32BIT) / 8) : 0;
+
+ padding += priv->data_width != I2S_DATA_BIT_WIDTH_16BIT ?
+ (priv->data_width != I2S_DATA_BIT_WIDTH_32BIT ? 1 : 0) : 0;
+
+ /* Copy audio data into internal buffer */
+
+ bfcontainer->buf = (uint8_t *)calloc(bfcontainer->nbytes, 1);
+
+ data_copied = 0;
+ buf = bfcontainer->buf + padding;
+
+ if (priv->tx.carry.bytes)
+ {
+ memcpy(buf, (const void *)&priv->tx.carry.value,
+ priv->tx.carry.bytes);
+ buf += priv->tx.carry.bytes;
+ data_copied += priv->tx.carry.bytes;
+ memcpy(buf, (const void *)samp,
+ (sample_size - priv->tx.carry.bytes));
+ buf += (sample_size - priv->tx.carry.bytes + padding);
+ samp += (sample_size - priv->tx.carry.bytes);
+ data_copied += (sample_size - priv->tx.carry.bytes);
+ }
+
+ /* If there is no need to add padding bytes, the memcpy may be done at
+ * once. Otherwise, the operation must add the padding bytes to each
+ * sample in the internal buffer
+ */
+
+ if (padding)
+ {
+ while (data_copied < (samp_size - carry_size))
+ {
+ memcpy(buf, (const void *)samp, sample_size);
+ buf += (sample_size + padding);
+ samp += sample_size;
+ data_copied += sample_size;
+ }
+ }
+ else
+ {
+ memcpy(buf, (const void *)samp,
+ samp_size - (data_copied + carry_size));
+ buf += samp_size - (data_copied + carry_size);
+ samp += samp_size - (data_copied + carry_size);
+ data_copied += samp_size - (data_copied + carry_size);
+ }
+
+ /* If the audio buffer's size is not a multiple of the sample size,
+ * it's necessary to carry the remaining bytes that are part of what
+ * would be the last sample on this buffer. These bytes will then be
+ * saved and inserted at the beginning of the next DMA buffer to
+ * rebuild the sample correctly.
+ */
+
+ priv->tx.carry.bytes = carry_size;
+
+ if (priv->tx.carry.bytes)
+ {
+ memcpy((uint8_t *)&priv->tx.carry.value,
+ (const void *)samp, priv->tx.carry.bytes);
Review Comment:
```suggestion
samp, priv->tx.carry.bytes);
```
##########
arch/xtensa/src/esp32/esp32_i2s.c:
##########
@@ -711,27 +736,114 @@ static int i2s_txdma_setup(struct esp32_i2s_s *priv,
/* Get the transfer information, accounting for any data offset */
- samp = (uintptr_t)&apb->samp[apb->curbyte];
- nbytes = apb->nbytes - apb->curbyte;
+ const apb_samp_t sample_size = priv->data_width / 8;
+ samp = (uintptr_t)&apb->samp[apb->curbyte];
+ samp_size = (apb->nbytes - apb->curbyte) + priv->tx.carry.bytes;
+ carry_size = samp_size % sample_size;
+
+ /* Padding contains the size (in bytes) to be skipped on the
+ * internal buffer do provide 1) the ability to handle mono
+ * audio files and 2) to correctly fill the buffer to 16 or 32-bits
+ * aligned positions
+ */
+
+ padding = priv->channels == 1 ?
+ ((priv->data_width <= I2S_DATA_BIT_WIDTH_16BIT ?
+ I2S_DATA_BIT_WIDTH_16BIT : I2S_DATA_BIT_WIDTH_32BIT) / 8) : 0;
+
+ padding += priv->data_width != I2S_DATA_BIT_WIDTH_16BIT ?
+ (priv->data_width != I2S_DATA_BIT_WIDTH_32BIT ? 1 : 0) : 0;
+
+ /* Copy audio data into internal buffer */
+
+ bfcontainer->buf = (uint8_t *)calloc(bfcontainer->nbytes, 1);
+
+ data_copied = 0;
+ buf = bfcontainer->buf + padding;
+
+ if (priv->tx.carry.bytes)
+ {
+ memcpy(buf, (const void *)&priv->tx.carry.value,
+ priv->tx.carry.bytes);
+ buf += priv->tx.carry.bytes;
+ data_copied += priv->tx.carry.bytes;
+ memcpy(buf, (const void *)samp,
+ (sample_size - priv->tx.carry.bytes));
+ buf += (sample_size - priv->tx.carry.bytes + padding);
+ samp += (sample_size - priv->tx.carry.bytes);
+ data_copied += (sample_size - priv->tx.carry.bytes);
+ }
+
+ /* If there is no need to add padding bytes, the memcpy may be done at
+ * once. Otherwise, the operation must add the padding bytes to each
+ * sample in the internal buffer
+ */
+
+ if (padding)
+ {
+ while (data_copied < (samp_size - carry_size))
+ {
+ memcpy(buf, (const void *)samp, sample_size);
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #7501: esp32/i2s: use internal buffer to handle multiple audio formats
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7501:
URL: https://github.com/apache/incubator-nuttx/pull/7501
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org