casaroli commented on code in PR #3840:
URL: https://github.com/apache/nuttx-apps/pull/3840#discussion_r4466890587


##########
system/nxplayer/nxplayer.c:
##########
@@ -1904,6 +1954,68 @@ static int nxplayer_playinternal(FAR struct nxplayer_s 
*pplayer,
           ret = -EINVAL;
           goto err_out;
         }
+
+#if defined(CONFIG_AUDIO_FORMAT_PCM) && \
+    defined(CONFIG_NXPLAYER_FMT_FROM_EXT)
+      max_channels = caps.ac_channels & 0x0f;
+
+      if (pplayer->wav_playback &&
+          max_channels != 0 && nchannels > max_channels)

Review Comment:
   `drivers/audio/es8311.c` reports 1 channel for an `AUDIO_TYPE_OUTPUT` query, 
but `es8311_configure()` accepts 2 ("The codec can take stereo audio and play 
only one channel"). esp32s3-korvo-2:audio, esp32s3-lcd-ev:audio and 
esp32s2-kaluga-1:audio use it through `pcm_decode`. A stereo WAV plays on 
master and fails with `ENOTSUP` with this PR. Reproduced with `pcm_decode` over 
`audio_null` with the query reporting 1 channel: master plays the file, this PR 
prints `Error playing file: 138`.
   
   Fix: remove the `max_channels` test and rely on the error check of 
`AUDIOIOC_CONFIGURE`, as for the sample rate.



##########
include/system/nxplayer.h:
##########
@@ -587,7 +605,16 @@ int nxplayer_fill_mp3(int fd, FAR struct ap_buffer_s *apb);
  *
  ****************************************************************************/
 
-int nxplayer_fill_common(int fd, FAR struct ap_buffer_s *apb);
+int nxplayer_fill_common(FAR struct nxplayer_s *pplayer, int fd,
+                         FAR struct ap_buffer_s *apb);
+
+#if defined(CONFIG_AUDIO_FORMAT_PCM) && \
+    defined(CONFIG_NXPLAYER_FMT_FROM_EXT)
+extern const struct nxplayer_dec_ops_s g_wav_dec_ops;
+
+bool nxplayer_is_wav_file(FAR const char *pfilename);
+uint32_t nxplayer_wav_rate_cap(uint32_t rate);

Review Comment:
   `nxplayer_wav_rate_cap()` is public, but its only caller is in 
`nxplayer_wav.c`, which only tests the result against 0.
   
   Fix: make it a static function in `nxplayer_wav.c`, and remove it from the 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to