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 22:01:16 UTC
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7501: esp32/i2s: use internal buffer to handle multiple audio formats
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