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 2020/11/11 09:29:45 UTC

[GitHub] [incubator-nuttx] jerpelea opened a new pull request #2282: cxd56: add initial audio SRC implementation

jerpelea opened a new pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282


   ## Summary
   Add basic sample rate conversion to the CXD56 Spresense audio
   driver using libsamplerate.
   Separate contributions are sent to libsamplerate (https://github.com/libsndfile/libsamplerate)
   
   Status
   Currently conversion is only done
   during playback and all output is fixed at 48 kHz.
   
   Issues:
   - 16 kHz SRC has glitches (unless data dump is enabled)
   - 44.1 kHz SRC gets stuck
   
   ## Impact
   SONY spresense board (SRC is disabled by default)
   
   ## Testing
   tested on Sony spresense board with and without CONFIG_AUDIO_CXD56_SRC


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525574756



##########
File path: drivers/audio/cxd56_src.h
##########
@@ -0,0 +1,127 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.h
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_AUDIO_CXD56_SRC_H
+#define __DRIVERS_AUDIO_CXD56_SRC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <pthread.h>
+#include <mqueue.h>
+
+#include <nuttx/audio/audio.h>
+#include <nuttx/config.h>
+
+#ifdef CONFIG_AUDIO
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifndef CONFIG_SMP_NCPUS

Review comment:
       @jerpelea 
   I think we don't need ```#ifndef CONFIG_SMP_NCPUS```
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r524934470



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,606 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation

Review comment:
       @btashton they will all be migrated to Apache once SGA is in place. 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525015043



##########
File path: drivers/audio/cxd56.c
##########
@@ -3269,27 +3415,38 @@ static int cxd56_enqueuebuffer(FAR struct audio_lowerhalf_s *lower,
   FAR struct cxd56_dev_s *priv = (FAR struct cxd56_dev_s *)lower;
   struct audio_msg_s msg;
   irqstate_t flags;
+  int ret;
 
   flags = spin_lock_irqsave();
 
-  apb->dq_entry.flink = NULL;
-  dq_put(&priv->up_pendq, &apb->dq_entry);
-
-  spin_unlock_irqrestore(flags);
-
-  if (priv->mq != NULL)
+#ifdef CONFIG_AUDIO_CXD56_SRC
+  ret = cxd56_src_enqueue(apb);
+  if (ret != OK)
     {
-      int ret;
+      auderr("ERROR: SRC processing failed (%d)\n", ret);
+    }
+  else
+    {
+#endif
+      apb->dq_entry.flink = NULL;
+      dq_put(&priv->up_pendq, &apb->dq_entry);
 
-      msg.msg_id = AUDIO_MSG_ENQUEUE;
-      msg.u.data = 0;
+      spin_unlock_irqrestore(flags);
 
-      ret = nxmq_send(priv->mq, (FAR const char *) &msg,
-                      sizeof(msg), CONFIG_CXD56_MSG_PRIO);
-      if (ret != OK)
+      if (priv->mq != NULL)
         {
-          auderr("ERROR: nxmq_send to enqueue failed (%d)\n", ret);
-          return ret;
+          msg.msg_id = AUDIO_MSG_ENQUEUE;
+          msg.u.data = 0;
+
+          ret = nxmq_send(priv->mq, (FAR const char *) &msg,
+                          sizeof(msg), CONFIG_CXD56_MSG_PRIO);
+          if (ret != OK)
+            {
+              auderr("ERROR: nxmq_send to enqueue failed (%d)\n", ret);
+              return ret;
+#ifdef CONFIG_AUDIO_CXD56_SRC

Review comment:
       @jerpelea 
   I think this ```ifdef CONFIG_AUDIO_CXD56_SRC``` is miplaced.
   

##########
File path: drivers/audio/cxd56.c
##########
@@ -3269,27 +3415,38 @@ static int cxd56_enqueuebuffer(FAR struct audio_lowerhalf_s *lower,
   FAR struct cxd56_dev_s *priv = (FAR struct cxd56_dev_s *)lower;
   struct audio_msg_s msg;
   irqstate_t flags;
+  int ret;
 
   flags = spin_lock_irqsave();

Review comment:
       @jerpelea 
   I think this ```flag = spin_lock_irqsave();``` should be moved to inside the else condition.
   Otherwise, a deadlock would happen if ```cxd56_src_enqueue()``` returns an error.
   
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525028253



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,596 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      src_apb = kmm_zalloc(bufsize);

Review comment:
       @jerpelea 
   ```kmm_zalloc()``` should not be called inside spin_lock_irqsave() because nxsem_wait() will be called inside.
   Instead, call ```spin_unlock_irqrestore()``` before calling ```kmm_zalloc()``` then call ```spin_lock_irqsave()``` again. 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525876704



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,601 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      spin_unlock_irqrestore(flags);
+
+      src_apb = kmm_zalloc(bufsize);
+
+      flags = spin_lock_irqsave();
+
+      if (!src_apb)
+        {
+          auderr("ERROR: Couldn't allocate SRC APB (size %d)\n", bufsize);
+
+          return NULL;

Review comment:
       @jerpelea 
   This return will cause a deadlock because ```spin_unlock_irqresotre()``` is not called.
   I think you should set the return value and then ```goto errout_with_lock;```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r526012405



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,607 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      spin_unlock_irqrestore(flags);
+
+      src_apb = kmm_zalloc(bufsize);
+
+      flags = spin_lock_irqsave();
+
+      if (!src_apb)
+        {
+          auderr("ERROR: Couldn't allocate SRC APB (size %d)\n", bufsize);
+
+          goto errorout_with_lock;
+        }
+
+      src_apb->nmaxbytes = CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+      src_apb->nbytes = 0;
+      src_apb->samp = (FAR uint8_t *)(&src_apb->samp + 1);
+    }
+  else
+    {
+      src_apb = (struct ap_buffer_s *) dq_get(g_src.inq);
+    }
+
+  src_apb->flags = 0;
+
+  spin_unlock_irqrestore(flags);
+
+  return src_apb;
+
+errorout_with_lock:

Review comment:
       done




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525880123



##########
File path: drivers/audio/cxd56.c
##########
@@ -1272,6 +1276,149 @@ static void cxd56_reset_channel_sel(cxd56_dmahandle_t handle)
     }
 }
 
+#ifdef CONFIG_CXD56_AUCIO_SRC
+static void _process_audio_with_src(cxd56_dmahandle_t hdl, uint16_t err_code)
+{
+  struct audio_msg_s msg;
+  struct cxd56_dev_s *dev;
+  irqstate_t flags;
+  bool request_buffer = true;
+  int ret;
+
+  dev = g_dev[hdl];
+
+  /* Trigger new DMA job */
+
+  flags = spin_lock_irqsave();
+
+  if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
+    {
+      /* Notify end of data */
+
+      if (dev->state != CXD56_DEV_STATE_PAUSED
+          && dq_count(&dev->down_pendq) == 0)
+        {
+          msg.msg_id = AUDIO_MSG_STOP;
+          msg.u.data = 0;
+          ret = nxmq_send(dev->mq, (FAR const char *)&msg,

Review comment:
       @jerpelea 
   ```nxmq_send()``` should not be called within ```spin_lock_irqsave()```
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525575910



##########
File path: drivers/audio/cxd56_src.h
##########
@@ -0,0 +1,127 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.h
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_AUDIO_CXD56_SRC_H
+#define __DRIVERS_AUDIO_CXD56_SRC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <pthread.h>
+#include <mqueue.h>
+
+#include <nuttx/audio/audio.h>
+#include <nuttx/config.h>
+
+#ifdef CONFIG_AUDIO
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifndef CONFIG_SMP_NCPUS
+#  define CONFIG_SMP_NCPUS  4
+#endif
+
+#ifndef CONFIG_CXD56_AUDIO_SRC_CPU
+#  define CONFIG_CXD56_AUDIO_SRC_CPU  2
+#endif
+
+#ifndef CONFIG_CXD56_AUDIO_SRC_STACKSIZE
+#  define CONFIG_CXD56_AUDIO_SRC_STACKSIZE  768
+#endif
+
+#ifndef CONFIG_CXD56_SRC_MSG_PRIO
+#  define CONFIG_CXD56_SRC_MSG_PRIO  1
+#endif
+
+#ifndef SRC_SINC_BEST_QUALITY
+#  define SRC_SINC_BEST_QUALITY     0
+#endif
+
+#ifndef SRC_SINC_MEDIUM_QUALITY
+#  define SRC_SINC_MEDIUM_QUALITY   1
+#endif
+
+#ifndef SRC_SINC_FASTEST
+#  define SRC_SINC_FASTEST          2
+#endif
+
+#ifndef SRC_ZERO_ORDER_HOLD
+#  define SRC_ZERO_ORDER_HOLD       3
+#endif
+
+#ifndef SRC_LINEAR
+#  define SRC_LINEAR                4
+#endif
+
+#define AUDIO_APB_SRC_FINAL  (1 << 4) /* Last buffer in SRC processing */
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+typedef struct SRC_STATE_TAG SRC_STATE;
+
+typedef struct
+{       const float *data_in;

Review comment:
       @jerpelea 
   Though I'm not sure why nxstyle does not report warnings but ```const float ..``` should be placed in the next line. Also, indentation spaces should be 2.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525880123



##########
File path: drivers/audio/cxd56.c
##########
@@ -1272,6 +1276,149 @@ static void cxd56_reset_channel_sel(cxd56_dmahandle_t handle)
     }
 }
 
+#ifdef CONFIG_CXD56_AUCIO_SRC
+static void _process_audio_with_src(cxd56_dmahandle_t hdl, uint16_t err_code)
+{
+  struct audio_msg_s msg;
+  struct cxd56_dev_s *dev;
+  irqstate_t flags;
+  bool request_buffer = true;
+  int ret;
+
+  dev = g_dev[hdl];
+
+  /* Trigger new DMA job */
+
+  flags = spin_lock_irqsave();
+
+  if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
+    {
+      /* Notify end of data */
+
+      if (dev->state != CXD56_DEV_STATE_PAUSED
+          && dq_count(&dev->down_pendq) == 0)
+        {
+          msg.msg_id = AUDIO_MSG_STOP;
+          msg.u.data = 0;
+          ret = nxmq_send(dev->mq, (FAR const char *)&msg,

Review comment:
       @jerpelea 
   ```nxmq_send()``` should not be called within spin_lock_irqsave()
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r521963461



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,606 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      src_apb = kmm_zalloc(bufsize);
+      if (!src_apb)
+        {
+          auderr("ERROR: Couldn't allocate SRC APB (size %d)\n", bufsize);
+
+          return NULL;
+        }
+
+      src_apb->nmaxbytes = CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+      src_apb->nbytes = 0;
+      src_apb->samp = (FAR uint8_t *)(&src_apb->samp + 1);
+    }
+  else
+    {
+      src_apb = (struct ap_buffer_s *) dq_get(g_src.inq);
+    }
+
+  src_apb->flags = 0;
+
+  spin_unlock_irqrestore(flags);
+
+  return src_apb;
+}
+
+/* Apply SRC on incoming APB and add one or more APBs to the
+ * out queue accordingly.
+ */
+
+static int cxd56_src_process(FAR struct ap_buffer_s *apb)
+{
+  int ret = OK;
+  irqstate_t flags;
+  struct ap_buffer_s *src_apb;
+
+  /* audinfo("SRC: Process (size = %d)\n", apb->nbytes); */
+
+#ifdef DUMP_DATA
+  write(dump_file_pre,
+        (char *) (apb->samp + apb->curbyte),
+        apb->nbytes - apb->curbyte);
+#endif
+
+  /* Special case of one-to-one ratio */
+
+  if (g_src.src_data.src_ratio == 1.0f)
+    {
+      src_apb = cxd56_src_get_apb();
+      if (!src_apb)
+        {
+          ret = -ENOMEM;
+          goto exit;
+        }
+
+      memcpy(src_apb->samp, apb->samp, apb->nbytes);
+      src_apb->nbytes = apb->nbytes;
+      src_apb->flags |= AUDIO_APB_SRC_FINAL;
+
+      flags = spin_lock_irqsave();
+      dq_put(g_src.outq, &src_apb->dq_entry);
+      spin_unlock_irqrestore(flags);
+
+      goto exit;
+    }
+
+  /* Perform SRC on new buffer and left overs from previous ones */
+
+  while (apb->curbyte < apb->nbytes)
+    {
+      int float_in_left;
+      int frames_in;
+
+      short *apb_addr = (const short *)(apb->samp + apb->curbyte);
+
+      /* Fill up incoming float buffer */
+
+      float_in_left = BUFFER_SAMPLES - g_src.float_in_offset;
+
+      src_short_to_float_array(apb_addr,
+                               (g_src.float_in + g_src.float_in_offset),
+                               float_in_left);
+      g_src.src_data.output_frames = BUFFER_SAMPLES / g_src.channels;
+      g_src.src_data.input_frames = BUFFER_SAMPLES / g_src.channels;
+
+      /* Incoming data larger than ingoing float buffer? */
+
+      frames_in = (apb->nbytes - apb->curbyte) / g_src.bytewidth;
+
+      if (frames_in >= float_in_left || g_src.state == CXD56_SRC_STOPPING)
+        {
+          int apb_nframes;
+          int apb_nmaxframes;
+          int src_nframes;
+          int src_copyframes;
+
+          float *float_out_src;
+          short *src_apb_dest;
+
+          /* Run SRC */
+
+          g_src.src_data.data_out = g_src.float_out;
+          g_src.src_data.data_in = g_src.float_in;
+
+          ret = src_process(g_src.src_state, &g_src.src_data);
+          if (ret != 0)
+            {
+              auderr("ERROR: SRC failed (\"%s\")\n", src_strerror(ret));
+            }
+
+          /* Move unused data to start of float_in for next round */
+
+          g_src.float_in_offset =
+              g_src.src_data.input_frames_used * g_src.channels;
+          memcpy((void *)g_src.float_in,
+                 (void *)(g_src.float_in + g_src.float_in_offset),
+                 (BUFFER_SAMPLES - g_src.float_in_offset) * sizeof(float));
+
+          g_src.float_in_offset = BUFFER_SAMPLES - g_src.float_in_offset;
+
+          /* Prepare apb to dma */
+
+          src_apb = cxd56_src_get_apb();
+          if (!src_apb)
+            {
+              ret = -ENOMEM;
+              goto exit;
+            }
+
+          apb_nframes =
+              src_apb->nbytes / g_src.bytewidth / g_src.channels;
+          apb_nmaxframes =
+              src_apb->nmaxbytes / g_src.bytewidth / g_src.channels;
+
+          src_nframes = g_src.src_data.output_frames_gen;
+          src_copyframes = apb_nmaxframes - apb_nframes;
+
+          /* Generated frames will exceed apb size left */
+
+          if (apb_nframes + src_nframes >= apb_nmaxframes
+              || g_src.state == CXD56_SRC_STOPPING)
+            {
+              /* Convert SRC float data into APB to be sent */
+
+              float_out_src = g_src.float_out;
+              src_apb_dest = (short *)(src_apb->samp + src_apb->nbytes);
+
+              src_float_to_short_array(float_out_src, src_apb_dest,
+                                       src_copyframes * g_src.channels);
+              src_nframes -= src_copyframes;
+              src_apb->nbytes = src_apb->nmaxbytes;
+
+              /* Increase SRC buffer processing counter */
+
+              g_src.buf_count += g_src.buf_increment;
+              if (g_src.buf_count > 1.0f)
+                {
+                  src_apb->flags |= AUDIO_APB_SRC_FINAL;
+                  g_src.buf_count -= 1.0f;
+                }
+
+              /* Put in out queue to be DMA'd */
+
+              flags = spin_lock_irqsave();
+              dq_put(g_src.outq, &src_apb->dq_entry);
+              spin_unlock_irqrestore(flags);
+
+#ifdef DUMP_DATA
+              write(dump_file_post, src_apb->samp, src_apb->nbytes);
+#endif
+
+              /* Fetch the next APB to fill up */
+
+              src_apb = cxd56_src_get_apb();
+              if (!src_apb)
+                {
+                  ret = -ENOMEM;
+                  goto exit;
+                }
+
+              apb_nframes =
+                  src_apb->nbytes / g_src.bytewidth / g_src.channels;
+            }
+
+          /* Convert remaining SRC float data into next APB */
+
+          float_out_src = g_src.float_out + src_copyframes * g_src.channels;
+          src_apb_dest = (short *)(src_apb->samp + src_apb->nbytes);
+
+          src_float_to_short_array(float_out_src, src_apb_dest,
+                                   src_nframes * g_src.channels);
+
+          src_apb->nbytes += g_src.bytewidth * src_nframes * g_src.channels;
+
+          flags = spin_lock_irqsave();
+          dq_put_back(g_src.inq, &src_apb->dq_entry);
+          spin_unlock_irqrestore(flags);
+
+          apb->curbyte += (float_in_left * g_src.bytewidth);
+        }
+      else
+        {
+          g_src.float_in_offset += frames_in;
+          apb->curbyte = apb->nbytes - 1;
+
+          break;
+        }
+    }
+
+exit:
+  return ret;
+}
+
+/* SRC control and processing thread */
+
+static void *cxd56_src_thread(pthread_addr_t pvarg)
+{
+  struct audio_msg_s msg;
+  unsigned int prio;
+  int ret;
+  int size;
+
+  audinfo("SRC: Thread started\n");
+
+  g_src.state = CXD56_SRC_RUNNING;
+
+  while (g_src.state == CXD56_SRC_RUNNING)
+    {
+      size = nxmq_receive(g_src.mq, (FAR char *)&msg, sizeof(msg), &prio);
+
+      /* Handle the case when we return with no message */
+
+      if (size == 0)
+        {
+          audinfo("SRC: Zero message, stop\n");
+          g_src.state = CXD56_SRC_STOPPED;
+          break;
+        }
+
+      /* Process the message */
+
+      switch (msg.msg_id)
+        {
+          case AUDIO_MSG_START:
+            break;
+          case AUDIO_MSG_STOP:
+            g_src.state = CXD56_SRC_STOPPED;
+            break;
+          case AUDIO_MSG_ENQUEUE:
+            ret = cxd56_src_process(msg.u.ptr);
+            if (ret != OK)
+              {
+                auderr("ERROR: SRC processing failed (%d)\n", ret);
+                g_src.state = CXD56_SRC_STOPPED;
+              }
+            break;
+        }
+    }
+
+  return NULL;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: cxd56_src_init
+ *
+ * Description: Initializes the SRC using audio settings set in the given
+ *              audio device. When SRC is running the resulting buffers will
+ *              be put into the queue "outq" for playback, and after it has
+ *              been played it is expected to be put in the "inq" queue to be
+ *              filled up with new data.
+ *
+ ****************************************************************************/
+
+int cxd56_src_init(FAR struct cxd56_dev_s *dev,
+                   FAR struct dq_queue_s *inq,
+                   FAR struct dq_queue_s *outq)
+{
+  struct sched_param sparam;
+  struct mq_attr m_attr;
+  pthread_attr_t t_attr;
+  void *value;
+  int error;
+  int ret = OK;
+
+#ifdef CONFIG_SMP
+  cpu_set_t cpu_set = CONFIG_CXD56_AUDIO_SRC_CPU;
+#endif
+
+  g_src.buf_count = 0.0f;
+  if (dev->samplerate < 48000)
+    {
+      g_src.buf_increment = dev->samplerate / 48000.0f;
+    }
+
+  g_src.inq = inq;
+  g_src.outq = outq;
+  g_src.bytewidth = dev->bitwidth / 8;
+  g_src.channels = dev->channels;
+  g_src.float_in_offset = 0;
+  snprintf(g_src.mqname, sizeof(g_src.mqname), "/tmp/%X", &g_src);
+
+  audinfo("SRC: Init (rate = %d, channels = %d, width = %d)\n",
+          dev->samplerate, g_src.channels, g_src.bytewidth);
+
+  m_attr.mq_maxmsg  = 16;
+  m_attr.mq_msgsize = sizeof(struct audio_msg_s);
+  m_attr.mq_curmsgs = 0;
+  m_attr.mq_flags   = 0;
+
+  g_src.mq = mq_open(g_src.mqname, O_RDWR | O_CREAT, 0644, &m_attr);
+  if (g_src.mq == NULL)
+    {
+      auderr("ERROR: Could not allocate SRC message queue.\n");
+      return -ENOMEM;
+    }
+
+#ifdef DUMP_DATA
+  unlink(dump_name_pre);
+  unlink(dump_name_post);
+  dump_file_pre = open(dump_name_pre, O_WRONLY | O_CREAT | O_APPEND);
+  dump_file_post = open(dump_name_post, O_WRONLY | O_CREAT | O_APPEND);
+#endif
+
+  /* Join any old worker threads to prevent memory leaks */
+
+  if (g_src.threadid != 0)
+    {
+      pthread_join(g_src.threadid, &value);
+    }
+
+  pthread_attr_init(&t_attr);
+  sparam.sched_priority = sched_get_priority_max(SCHED_FIFO) - 3;
+  (void)pthread_attr_setschedparam(&t_attr, &sparam);
+  (void)pthread_attr_setstacksize(&t_attr,
+                                  CONFIG_CXD56_AUDIO_SRC_STACKSIZE);
+
+#ifdef CONFIG_SMP
+  /* Associate thread with second core */
+
+  (void)pthread_attr_setaffinity_np(&t_attr, CONFIG_SMP_NCPUS, &cpu_set);

Review comment:
       Do we need this code?
   I think the CPU assignment should be automatic.
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r524994088



##########
File path: drivers/audio/cxd56.c
##########
@@ -1272,6 +1276,151 @@ static void cxd56_reset_channel_sel(cxd56_dmahandle_t handle)
     }
 }
 
+#ifdef CONFIG_CXD56_AUCIO_SRC
+static void _process_audio_with_src(cxd56_dmahandle_t hdl, uint16_t err_code)
+{
+  struct audio_msg_s msg;
+  struct cxd56_dev_s *dev;
+  irqstate_t flags;
+  bool request_buffer = true;
+  int ret;
+
+  dev = g_dev[hdl];
+
+  /* Trigger new DMA job */
+
+  flags = spin_lock_irqsave();
+
+  if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
+    {
+      /* Notify end of data */
+
+      if (dev->state != CXD56_DEV_STATE_PAUSED
+          && dq_count(&dev->down_pendq) == 0)
+        {
+          msg.msg_id = AUDIO_MSG_STOP;
+          msg.u.data = 0;
+          ret = nxmq_send(dev->mq, (FAR const char *)&msg,
+                          sizeof(msg), CONFIG_CXD56_MSG_PRIO);
+          if (ret != OK)
+            {
+              auderr("ERROR: nxmq_send to stop failed (%d)\n", ret);
+            }
+        }
+    }
+
+  if (dq_count(&dev->down_runq) > 0)
+    {
+      FAR struct ap_buffer_s *src_apb;
+
+      src_apb = (struct ap_buffer_s *) dq_get(&dev->down_runq);
+      src_apb->nbytes = 0;
+      dq_put(&dev->down_doneq, &src_apb->dq_entry);
+
+      if (src_apb->flags & AUDIO_APB_SRC_FINAL)
+        {
+          struct ap_buffer_s *apb;
+
+          apb = dq_get(&dev->up_runq);
+          dev->dev.upper(dev->dev.priv, AUDIO_CALLBACK_DEQUEUE, apb, OK);
+
+          /* End of data? */
+
+          if ((apb->flags & AUDIO_APB_FINAL) != 0)
+            {
+              msg.msg_id = AUDIO_MSG_STOP;
+              msg.u.data = 0;
+              ret = nxmq_send(dev->mq, (FAR const char *)&msg,
+                              sizeof(msg), CONFIG_CXD56_MSG_PRIO);
+              if (ret != OK)
+                {
+                  auderr("ERROR: nxmq_send to stop failed (%d)\n", ret);
+                }
+
+              request_buffer = false;
+            }
+        }
+    }
+
+  if (request_buffer && dev->mq != NULL)
+    {
+      /* Request more data */
+
+      msg.msg_id = AUDIO_MSG_DATA_REQUEST;
+      msg.u.data = 0;
+      ret = nxmq_send(dev->mq, (FAR const char *) &msg,
+                          sizeof(msg), CONFIG_CXD56_MSG_PRIO);
+      if (ret != OK)
+        {
+          auderr("ERROR: nxmq_send to request failed (%d)\n", ret);
+        }
+    }
+
+  spin_unlock_irqrestore(flags);
+}
+
+#else
+static void _process_audio(cxd56_dmahandle_t hdl, uint16_t err_code)
+{
+  struct audio_msg_s msg;
+  struct cxd56_dev_s *dev;
+  irqstate_t flags;
+  bool request_buffer = true;
+  int ret;
+
+  dev = g_dev[hdl];
+
+  /* Trigger new DMA job */
+
+  flags = spin_lock_irqsave();
+
+  if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
+    {
+      /* Notify end of data */
+
+      if (dev->state != CXD56_DEV_STATE_PAUSED)
+        {
+          audinfo("DMA_TRANS up_pendq=%d \n",
+                 dq_count(&dev->up_pendq));
+          msg.msg_id = AUDIO_MSG_STOP;
+          msg.u.data = 0;
+          ret = nxmq_send(dev->mq, (FAR const char *)&msg,
+                          sizeof(msg), CONFIG_CXD56_MSG_PRIO);
+          if (ret != OK)
+            {
+              auderr("ERROR: nxmq_send to stop failed (%d)\n", ret);
+            }
+        }
+    }
+
+  if (dq_count(&dev->up_runq) > 0)
+    {
+      FAR struct ap_buffer_s *apb;
+
+      apb = (struct ap_buffer_s *) dq_get(&dev->up_runq);
+      spin_unlock_irqrestore(flags);
+      dev->dev.upper(dev->dev.priv, AUDIO_CALLBACK_DEQUEUE, apb, OK);
+      flags = spin_lock_irqsave();

Review comment:
       @jerpelea 
   Could you tell me why the sequence for _process_audio() was changed from the previous code?
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525575910



##########
File path: drivers/audio/cxd56_src.h
##########
@@ -0,0 +1,127 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.h
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_AUDIO_CXD56_SRC_H
+#define __DRIVERS_AUDIO_CXD56_SRC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <pthread.h>
+#include <mqueue.h>
+
+#include <nuttx/audio/audio.h>
+#include <nuttx/config.h>
+
+#ifdef CONFIG_AUDIO
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifndef CONFIG_SMP_NCPUS
+#  define CONFIG_SMP_NCPUS  4
+#endif
+
+#ifndef CONFIG_CXD56_AUDIO_SRC_CPU
+#  define CONFIG_CXD56_AUDIO_SRC_CPU  2
+#endif
+
+#ifndef CONFIG_CXD56_AUDIO_SRC_STACKSIZE
+#  define CONFIG_CXD56_AUDIO_SRC_STACKSIZE  768
+#endif
+
+#ifndef CONFIG_CXD56_SRC_MSG_PRIO
+#  define CONFIG_CXD56_SRC_MSG_PRIO  1
+#endif
+
+#ifndef SRC_SINC_BEST_QUALITY
+#  define SRC_SINC_BEST_QUALITY     0
+#endif
+
+#ifndef SRC_SINC_MEDIUM_QUALITY
+#  define SRC_SINC_MEDIUM_QUALITY   1
+#endif
+
+#ifndef SRC_SINC_FASTEST
+#  define SRC_SINC_FASTEST          2
+#endif
+
+#ifndef SRC_ZERO_ORDER_HOLD
+#  define SRC_ZERO_ORDER_HOLD       3
+#endif
+
+#ifndef SRC_LINEAR
+#  define SRC_LINEAR                4
+#endif
+
+#define AUDIO_APB_SRC_FINAL  (1 << 4) /* Last buffer in SRC processing */
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+typedef struct SRC_STATE_TAG SRC_STATE;
+
+typedef struct
+{       const float *data_in;

Review comment:
       @jerpelea 
   Though I'm not sure why nxstyle does not report a warning, but ```const float ..``` should be placed in the next line. Also, indentation spaces should be 2.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525138881



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,606 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation

Review comment:
       
   > @btashton they will all be migrated to Apache once SGA is in place.
   
   For all other PRs, we have insisted that the Apache license be in place prior to bringing the code into the repository.  I don't doubt for a minute that the headers will be updated if/when the SGA is in place.  But we need  apply this rule consistently to all code.
   
   Perhaps we need to clarification?  Perhaps we should wait until the SGA is in place?
   
   @justinmclean Do you have any guidance on this?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525574262



##########
File path: drivers/audio/cxd56_src.h
##########
@@ -0,0 +1,127 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.h
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_AUDIO_CXD56_SRC_H
+#define __DRIVERS_AUDIO_CXD56_SRC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <pthread.h>
+#include <mqueue.h>
+
+#include <nuttx/audio/audio.h>
+#include <nuttx/config.h>
+
+#ifdef CONFIG_AUDIO
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifndef CONFIG_SMP_NCPUS
+#  define CONFIG_SMP_NCPUS  4
+#endif
+
+#ifndef CONFIG_CXD56_AUDIO_SRC_CPU
+#  define CONFIG_CXD56_AUDIO_SRC_CPU  2

Review comment:
       @jerpelea 
   I think we don't need ```#ifndef CONFIG_CXD56_AUDIO_SRC_CPU```
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525876704



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,601 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      spin_unlock_irqrestore(flags);
+
+      src_apb = kmm_zalloc(bufsize);
+
+      flags = spin_lock_irqsave();
+
+      if (!src_apb)
+        {
+          auderr("ERROR: Couldn't allocate SRC APB (size %d)\n", bufsize);
+
+          return NULL;

Review comment:
       @jerpelea 
   This return will cause a deadlock because ```spin_unlock_irqresotre()``` is not called.
   I think you should use ```goto errout_with_lock;```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 merged pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 merged pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r524969933



##########
File path: drivers/audio/cxd56.c
##########
@@ -1370,22 +1375,24 @@ static void cxd56_dma_int_handler(void)
 
       flags = spin_lock_irqsave();
 
-      if (dq_count(&dev->up_runq) > 0)
-        {
-          FAR struct ap_buffer_s *apb;
-
-          apb = (struct ap_buffer_s *) dq_get(&dev->up_runq);
-          spin_unlock_irqrestore(flags);
-          dev->dev.upper(dev->dev.priv, AUDIO_CALLBACK_DEQUEUE, apb, OK);
-          flags = spin_lock_irqsave();
-        }
-
-      spin_unlock_irqrestore(flags);
-
       if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
         {
           /* Notify end of data */
 
+#ifdef CONFIG_AUDIO_CXD56_SRC
+          if (dev->state != CXD56_DEV_STATE_PAUSED

Review comment:
       @masayuki2009 done




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525028253



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,596 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      src_apb = kmm_zalloc(bufsize);

Review comment:
       @jerpelea 
   ```kmm_zalloc()``` should not be called inside spin_lock_irqsave() because nxsem_wait() will be called inside.
   Instead, call ```spin_unlock_irqrestore() before calling ```kmm_zalloc()``` then call ```spin_lock_irqsave()``` again. 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525880696



##########
File path: drivers/audio/cxd56.c
##########
@@ -1272,6 +1276,149 @@ static void cxd56_reset_channel_sel(cxd56_dmahandle_t handle)
     }
 }
 
+#ifdef CONFIG_CXD56_AUCIO_SRC
+static void _process_audio_with_src(cxd56_dmahandle_t hdl, uint16_t err_code)
+{
+  struct audio_msg_s msg;
+  struct cxd56_dev_s *dev;
+  irqstate_t flags;
+  bool request_buffer = true;
+  int ret;
+
+  dev = g_dev[hdl];
+
+  /* Trigger new DMA job */
+
+  flags = spin_lock_irqsave();
+
+  if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
+    {
+      /* Notify end of data */
+
+      if (dev->state != CXD56_DEV_STATE_PAUSED
+          && dq_count(&dev->down_pendq) == 0)
+        {
+          msg.msg_id = AUDIO_MSG_STOP;
+          msg.u.data = 0;
+          ret = nxmq_send(dev->mq, (FAR const char *)&msg,
+                          sizeof(msg), CONFIG_CXD56_MSG_PRIO);
+          if (ret != OK)
+            {
+              auderr("ERROR: nxmq_send to stop failed (%d)\n", ret);
+            }
+        }
+    }
+
+  if (dq_count(&dev->down_runq) > 0)
+    {
+      FAR struct ap_buffer_s *src_apb;
+
+      src_apb = (struct ap_buffer_s *) dq_get(&dev->down_runq);
+      src_apb->nbytes = 0;
+      dq_put(&dev->down_doneq, &src_apb->dq_entry);
+
+      if (src_apb->flags & AUDIO_APB_SRC_FINAL)
+        {
+          struct ap_buffer_s *apb;
+
+          apb = dq_get(&dev->up_runq);
+          dev->dev.upper(dev->dev.priv, AUDIO_CALLBACK_DEQUEUE, apb, OK);

Review comment:
       @jerpelea 
   ```dev->dev.upper()``` should not be called within ```spin_lock_irqsave()```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r523703756



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,606 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation

Review comment:
       Can these files come in with the Apache header?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525877072



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,601 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      spin_unlock_irqrestore(flags);
+
+      src_apb = kmm_zalloc(bufsize);
+
+      flags = spin_lock_irqsave();
+
+      if (!src_apb)
+        {
+          auderr("ERROR: Couldn't allocate SRC APB (size %d)\n", bufsize);
+
+          return NULL;
+        }
+
+      src_apb->nmaxbytes = CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+      src_apb->nbytes = 0;
+      src_apb->samp = (FAR uint8_t *)(&src_apb->samp + 1);
+    }
+  else
+    {
+      src_apb = (struct ap_buffer_s *) dq_get(g_src.inq);
+    }
+
+  src_apb->flags = 0;
+

Review comment:
       @jerpelea 
   Add ```errout_with_lock:``` here.
    




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea commented on pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
jerpelea commented on pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#issuecomment-725914174


   @acassis @masayuki2009 the nxstyle is expected to fail
   please check 
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525151767



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,606 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation

Review comment:
       @patacongo the SGA is on its way but since it needed to be changed it will take some more time




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525280954



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,606 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation

Review comment:
       I would be OK on this but only because I have seen the discussion about getting paperwork sorted out and trust that this will be resolved. We have pushed back on other submissions so I agree about needing to be consistent and fair here.
   
   If for some reason the paperwork fell through this would just end up in the same place as other BSD code and we would have to call it out in the NOTICE file. That would be unfortunate, but not a problem.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525093364



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,596 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      src_apb = kmm_zalloc(bufsize);

Review comment:
       @masayuki2009 thanks




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r525574262



##########
File path: drivers/audio/cxd56_src.h
##########
@@ -0,0 +1,127 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.h
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+#ifndef __DRIVERS_AUDIO_CXD56_SRC_H
+#define __DRIVERS_AUDIO_CXD56_SRC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <pthread.h>
+#include <mqueue.h>
+
+#include <nuttx/audio/audio.h>
+#include <nuttx/config.h>
+
+#ifdef CONFIG_AUDIO
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifndef CONFIG_SMP_NCPUS
+#  define CONFIG_SMP_NCPUS  4
+#endif
+
+#ifndef CONFIG_CXD56_AUDIO_SRC_CPU
+#  define CONFIG_CXD56_AUDIO_SRC_CPU  2

Review comment:
       @jerpelea 
   I think we don't need ```CONFIG_CXD56_AUDIO_SRC_CPU```
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r521976899



##########
File path: drivers/audio/cxd56.c
##########
@@ -1370,22 +1375,24 @@ static void cxd56_dma_int_handler(void)
 
       flags = spin_lock_irqsave();
 
-      if (dq_count(&dev->up_runq) > 0)
-        {
-          FAR struct ap_buffer_s *apb;
-
-          apb = (struct ap_buffer_s *) dq_get(&dev->up_runq);
-          spin_unlock_irqrestore(flags);
-          dev->dev.upper(dev->dev.priv, AUDIO_CALLBACK_DEQUEUE, apb, OK);
-          flags = spin_lock_irqsave();
-        }
-
-      spin_unlock_irqrestore(flags);
-
       if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
         {
           /* Notify end of data */
 
+#ifdef CONFIG_AUDIO_CXD56_SRC
+          if (dev->state != CXD56_DEV_STATE_PAUSED

Review comment:
       I think  cxd56_dma_int_handler() has many lines and many #ifdefs, so it's not easy to read.
   Shall we separete the original (i.e. w/o SRC) code and new code (i.e w/ SRC)?
   
   e.g.
   
   ```
   sttic void cxd56_dma_int_handler(void)
   {
   ....
     if (err_code != CXD56_AUDIO_ECODE_DMA_HANDLE_INV)
       {
   #ifdef CONFIG_AUDIO_CXD56_SRC
         _process_audio_with_src(....);
   #else 
         _process_audio(...);
   #endif 
       }
   }




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r521976899



##########
File path: drivers/audio/cxd56.c
##########
@@ -1370,22 +1375,24 @@ static void cxd56_dma_int_handler(void)
 
       flags = spin_lock_irqsave();
 
-      if (dq_count(&dev->up_runq) > 0)
-        {
-          FAR struct ap_buffer_s *apb;
-
-          apb = (struct ap_buffer_s *) dq_get(&dev->up_runq);
-          spin_unlock_irqrestore(flags);
-          dev->dev.upper(dev->dev.priv, AUDIO_CALLBACK_DEQUEUE, apb, OK);
-          flags = spin_lock_irqsave();
-        }
-
-      spin_unlock_irqrestore(flags);
-
       if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
         {
           /* Notify end of data */
 
+#ifdef CONFIG_AUDIO_CXD56_SRC
+          if (dev->state != CXD56_DEV_STATE_PAUSED

Review comment:
       I think  cxd56_dma_int_handler() has many lines and many #ifdefs, so it's not easy to read.
   Shall we separete the original (i.e. w/o SRC) code and new code (i.e w/ SRC)?
   
   e.g.
   
   ```
   sttic void cxd56_dma_int_handler(void)
   {
   ....
     if (err_code != CXD56_AUDIO_ECODE_DMA_HANDLE_INV)
       {
   #ifdef CONFIG_AUDIO_CXD56_SRC
         _process_audio_with_src(....);
   #else 
         _process_audio(...); <= original code should be moved to here
   #endif 
       }
   }




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #2282: cxd56: add initial audio SRC implementation

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2282:
URL: https://github.com/apache/incubator-nuttx/pull/2282#discussion_r526005031



##########
File path: drivers/audio/cxd56_src.c
##########
@@ -0,0 +1,607 @@
+/****************************************************************************
+ * drivers/audio/cxd56_src.c
+ *
+ *   Copyright 2020 Sony Semiconductor Solutions Corporation
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name NuttX nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <queue.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/config.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mqueue.h>
+
+#include "cxd56.h"
+#include "cxd56_src.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* For debugging: dump pre/post SRC data to sdcard */
+
+/* #define DUMP_DATA */
+
+/* Note: 24 bit samples not currently supported by SRC */
+
+#define BUFFER_SAMPLES  (CONFIG_CXD56_AUDIO_BUFFER_SIZE / 2)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+enum cxd56_srcstate_e
+{
+  CXD56_SRC_OFF,
+  CXD56_SRC_RUNNING,
+  CXD56_SRC_STOPPING,
+  CXD56_SRC_STOPPED
+};
+
+struct cxd56_srcdata_s
+{
+  enum cxd56_srcstate_e state;
+
+  float float_in[BUFFER_SAMPLES];
+  float float_out[BUFFER_SAMPLES];
+  int float_in_offset;
+
+  SRC_DATA src_data;
+  SRC_STATE *src_state;
+
+  struct dq_queue_s *inq;
+  struct dq_queue_s *outq;
+
+  char mqname[32];
+  mqd_t mq;
+  sem_t pendsem;
+  pthread_t threadid;
+
+  uint8_t bytewidth;
+  uint8_t channels;
+
+  float buf_count;
+  float buf_increment;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static struct cxd56_srcdata_s g_src;
+
+#ifdef DUMP_DATA
+static char *dump_name_pre  = "/mnt/sd0/dump/nx_player_dump_pre.pcm";
+static char *dump_name_post = "/mnt/sd0/dump/nx_player_dump_post.pcm";
+int dump_file_pre = -1;
+int dump_file_post = -1;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+extern void src_short_to_float_array (const short *in, float *out, int len);
+extern void src_float_to_short_array (const float *in, short *out, int len);
+extern void src_int_to_float_array (const int *in, float *out, int len);
+extern void src_float_to_int_array (const float *in, int *out, int len);
+
+static struct ap_buffer_s *cxd56_src_get_apb()
+{
+  struct ap_buffer_s *src_apb;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
+
+  if (dq_count(g_src.inq) == 0)
+    {
+      size_t bufsize = sizeof(struct ap_buffer_s) +
+                              CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+
+      spin_unlock_irqrestore(flags);
+
+      src_apb = kmm_zalloc(bufsize);
+
+      flags = spin_lock_irqsave();
+
+      if (!src_apb)
+        {
+          auderr("ERROR: Couldn't allocate SRC APB (size %d)\n", bufsize);
+
+          goto errorout_with_lock;
+        }
+
+      src_apb->nmaxbytes = CONFIG_CXD56_AUDIO_BUFFER_SIZE;
+      src_apb->nbytes = 0;
+      src_apb->samp = (FAR uint8_t *)(&src_apb->samp + 1);
+    }
+  else
+    {
+      src_apb = (struct ap_buffer_s *) dq_get(g_src.inq);
+    }
+
+  src_apb->flags = 0;
+
+  spin_unlock_irqrestore(flags);
+
+  return src_apb;
+
+errorout_with_lock:

Review comment:
       @jerpelea 
   I think the label should be moved to the above ```spin_unlock_irqrestore(flags);```
   And the following lines can be removed because src_apb is set to NULL in the case of error.
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org