blue-debug opened a new pull request, #3700:
URL: https://github.com/apache/nuttx-apps/pull/3700

   ## Summary
   
   This PR contains one commit doing two related things: it reworks the
   nxplayer setup path so that `play` and `playraw` share it, which fixes a
   playback regression, and it adds a `tone` command on top of the reworked
   path.
   
   ### 1. Fix: `play` never issues AUDIOIOC_CONFIGURE
   
   Playback is silent on a Raspberry Pi Pico 2 with the
   `raspberrypi-pico-2:spisd` configuration. Reported and bisected to two
   commits from apache/nuttx#18348 on the dev mailing list:
   https://www.mail-archive.com/[email protected]/msg14859.html
   This PR fixes the `nuttx-apps` side; the `nuttx` side is apache/nuttx#19686.
   
   `nxplayer_playfile()` calls `nxplayer_playinternal()` directly with zeroed
   `nchannels`/`bpsamp`/`samprate`, while the fallback defaults added by
   09a3bc44b4 ("Add playraw command") live in `nxplayer_playraw()`. As a
   result `play` never issues `AUDIOIOC_CONFIGURE` for any format whose
   parameters the application cannot determine on its own.
   
   That used to be harmless, because the audio upper half only tracked a
   `started` flag and `AUDIOIOC_START` worked either way. It now requires
   `AUDIO_STATE_PREPARED`, which only `audio_configure()` sets:
   
   ```
     before:  OPEN ---------------------> RUNNING     (configure optional)
     after:   OPEN --configure--> PREPARED --> RUNNING
   ```
   
   Formats whose header the application parses itself are unaffected, since
   `pre_parse` fills in the three values and the defaults are never needed.
   WAV is the one case left where the parsing lives in the kernel:
   `pcm_decode` reads the RIFF header from the first enqueued buffer and
   configures the lower half directly, which does not pass through the upper
   half. So the device stays in `AUDIO_STATE_OPEN` and `AUDIOIOC_START` is
   rejected even though the hardware is already programmed correctly.
   
   Move the file open, format detection and `pre_parse` call from
   `nxplayer_playinternal()` into `nxplayer_playraw()`, ahead of the default
   fill, and let `nxplayer_playfile()` go through it. Both entry points now
   share one path, so `play` picks up the defaults that `playraw` already had.
   The `pre_parse` return value is checked as well; it was previously ignored,
   and a header that fails to parse left the three out-parameters at zero and
   produced the same failure.
   
   ### 2. New `tone` command
   
   With the setup path no longer tied to a file, the same machinery can drive
   a generated stream, so this adds a `tone` command:
   
   ```
       nxplayer> device pcm0p
       nxplayer> tone 48000 20
   ```
   
   Arguments are `samplerate duration pitchfreq`, all optional, defaulting to
   48000 Hz, 10 s and 440 Hz respectively.
   
   `nxplayer_playtone()` stores the tone parameters in the player context and
   enters the same `nxplayer_playinternal()` flow used for files, declaring the
   format as 2 channels / 16 bits / the requested sample rate. In the play
   thread, `nxplayer_filltone()` takes the place of the file read and
   synthesises a 16-bit stereo sine wave with `sinf()` directly into each audio
   pipeline buffer, marking the last one with `AUDIO_APB_FINAL` when the
   requested duration is reached.
   
   This is useful for bring-up and for narrowing down playback problems, since
   it exercises the audio device without involving a filesystem or any format
   decoding: if a tone plays but a file does not, the problem is above the
   driver.
   
   depends-on: apache/nuttx/pull/19686
   
   ## Impact
   
   - `play` now issues `AUDIOIOC_CONFIGURE` in all cases. The values may be
     defaults rather than the real ones, which is harmless: a lower half that
     derives the format itself overrides them before the stream starts.
   - `nxplayer_playraw()` gains `filefmt` and `subfmt` parameters, so its
     prototype in `include/system/nxplayer.h` changes and out-of-tree callers
     need updating. In-tree callers are updated in the same commit.
   - `nxplayer_playinternal()` no longer takes a filename.
   - New public function `nxplayer_playtone()`, new `struct nxplayer_tone_s`
     member in `struct nxplayer_s`, and a new `tone` command. 
`nxplayer_filltone()`
     uses `sinf()`, so a toolchain with libm is required for that path.
   - A `pre_parse` failure is now reported instead of silently proceeding with
     zeroed format parameters.
   
   ## Testing
   
   Runtime verification was kindly done by the reporter on
   `raspberrypi-pico-2:spisd`, together with the companion nuttx change:
   https://www.mail-archive.com/[email protected]/msg14859.html
   
   Before:
   
   ```
   nsh> nxplayer
   nxplayer> play /mnt/sd0/test.wav
   ```
   
   Nothing happens - no I2S clock or data on the pins.
   
   After: playback runs.
   
   Local checks on Ubuntu 22.04.5 LTS x86_64:
   
   - `tools/checkpatch.sh` - clean


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