[GitHub] [incubator-nuttx] yunkya2 commented on a change in pull request #3568: audio/pcm_decode.c: skip extra chunk of wav header

[GitBox <gi...@apache.org>]

yunkya2 commented on a change in pull request #3568:
URL: https://github.com/apache/incubator-nuttx/pull/3568#discussion_r615485533



##########
File path: audio/pcm_decode.c
##########
@@ -356,17 +363,45 @@ static bool pcm_parsewav(FAR struct pcm_decode_s *priv, uint8_t *data)
   localwav.fmt.align     = pcm_leuint16(wav->fmt.align);
   localwav.fmt.bpsamp    = pcm_leuint16(wav->fmt.bpsamp);
 
-  localwav.data.chunkid  = pcm_leuint32(wav->data.chunkid);
-  localwav.data.chunklen = pcm_leuint32(wav->data.chunklen);
+  /* Find the data chunk */
+
+  dchunk = &wav->data;
+
+  while (len < ret)

Review comment:
       The condition `len < ret` is ok  because:
   - The initial value of `ret` (= `sizeof(struct wav_header_s)`) already contains the size of `struct wav_datachunk_s`.
     In the case of no extra chunk, we can just compare with `ret` and the length of the incoming data `len`. 
   - The case of `len == ret` is invalid because it means the incoming data only have wav header and there is no data body to be transferred.
   
   But, your `for` loop seems better because we don't need to compare the values twice. I'd like to modify the loop to yours except the error condition 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