Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speaker relative path not working with external controller #6591

Closed
gabryelreyes opened this issue Jul 29, 2024 · 7 comments · Fixed by #6605
Closed

Speaker relative path not working with external controller #6591

gabryelreyes opened this issue Jul 29, 2024 · 7 comments · Fixed by #6605
Assignees
Milestone

Comments

@gabryelreyes
Copy link
Contributor

gabryelreyes commented Jul 29, 2024

Describe the Bug

Using a external controller: using the Speaker to play a WAV file that is found relative to the controller executable results in the file not to being found.

This should be possible according to the documentation: https://cyberbotics.com/doc/reference/speaker#description

├── ExternalController
│   ├── ExternalController.exe
│   └── Sounds
│       ├── CoolSound.wav

Root cause

robot->controllerDir(); returns an empty string, as mControllerDir is not updated if the controller is <extern>.
This results in QFile::exists(mControllerDir + filename) returning false for the WAV file.

Steps to Reproduce

  1. Create a world with robot
    • It shall have an <extern> controller.
    • It shall have a Speaker device.
  2. Write an external controller for the robot.
  3. Have the controller play a WAV file with a relative path to the file.
    • Example: pathToFile = "sounds/CoolSound.wav"

Expected behavior

Expected to hear the sound.

Observed behavior

WARNING: DEF ROBOT robot > Speaker "speaker": Sound file 'Sounds/CoolSound.wav' not found. The sound file should be defined relatively to the controller, or absolutely.`

System

  • Operating System: Windows 11
  • Graphics Card: Nvidia GeForce Mx550
@gabryelreyes gabryelreyes self-assigned this Jul 29, 2024
@gabryelreyes
Copy link
Contributor Author

gabryelreyes commented Jul 29, 2024

mControllerDir cannot be updated in the case of external controllers given that it may be located in a remote machine.
The only possibilty for the sounds to reliably work would be for them to be located relative to the world file, as this would ensure the files are in the same machine as the simulation or as URLs to be downloaded.

@omichel
Copy link
Member

omichel commented Jul 29, 2024

Otherwise, we can also modify the protocol to send the sound file to the server together with the speaker sound play command.

@gabryelreyes
Copy link
Contributor Author

I do not know how easy/difficult it is to implement such a feature to handle files that are a couple hundred of kBytes. Caching would be required, as if it was a web resource.

@omichel
Copy link
Member

omichel commented Jul 29, 2024

Yes, if implementing such a feature, we should also implement some caching functionality.

@nhjschulz
Copy link
Contributor

nhjschulz commented Jul 30, 2024

Proposal for external controller sound support:

Add a new speaker API to upload sound data to speaker devices from external controller. Speakers will then cache the data and a later call to play the file will work, regardless of what the path was (as long it is the same string used during the upload).

On controller side it will look like this:

... 
const char[] LOCAL_SOUND_FILE="c:/foo/bar/x.wav";

wb_speaker_upload_sound( left, right, LOCAL_SOUND_FILE, balance);
....
wb_speaker_play_sound(... LOCAL_SOUND_FILE, ....);

The upload sound API will stream the data to the Webots instance together with the filename for caching.
This may result in large messages if we embed the content of eventually several wav files inside it.

Will this work or is there a message length limit to consider ?
Is backward compatibiliy between controllers and Webots version an issue if we extend the speaker protocol ?

@gabryelreyes gabryelreyes added this to the R2024a milestone Jul 30, 2024
@omichel
Copy link
Member

omichel commented Jul 30, 2024

Yes, that would work. However, I believe we can probably implement this without modifying the API: When passing a sound file to wb_speaker_play_sound, we could simply upload this sound file to the Webots server if it was not already uploaded and play it. This means that we have to maintain, in the libController, a list of sound files which have already been uploaded to avoid uploading them several times.

@gabryelreyes gabryelreyes mentioned this issue Jul 30, 2024
48 tasks
@nhjschulz
Copy link
Contributor

@omichel Ok, I'll embed the sound data upload logic into existing wb_speaker_play_sound() as suggested above..

nhjschulz added a commit to nhjschulz/webots that referenced this issue Jul 31, 2024
Stream sound data from controller to Webots if the sound file path
is accesible by the controller process. This allows external
controller to use sounds even if the file  pathis not accessible
from Webots.

 * Controller: Try to load sound data on controller side and stream
   it to webots as part of the C_SPEAKER_PLAY_SOUND message. (speaker.c)
 * Webots: Read controller uploaded sound data in C_SPEAKER_PLAY_SOUND
   message (WbSpeaker). Use former file based load method if no data
   was streamed from controller.
 * Updated speaker.md to explain when wb_speaker_play_sound()
   streams loads the sound data itself already.
nhjschulz added a commit to nhjschulz/webots that referenced this issue Jul 31, 2024
Cache sound data in speaker decice not WbSoundSource.
WbSoundSource gets deleted on play stop.
nhjschulz added a commit to nhjschulz/webots that referenced this issue Aug 1, 2024
Stream sound data from controller to Webots if the sound file path
is accesible by the controller process. This allows external
controller to use sounds even if the file path is not accessible
from Webots.

 * Controller: Try to load sound data on controller side and stream
   it to webots as part of the C_SPEAKER_PLAY_SOUND message. (speaker.c)
 * Webots: Read controller uploaded sound data in C_SPEAKER_PLAY_SOUND
   message (WbSpeaker). Use former file based load method if no data
   was streamed from controller.
 * Streamed sound data is cached inside WbSpeaker instance as controller
   provides the data only on first use of a sound file.
 * Updated speaker.md to explain when wb_speaker_play_sound()
   streams loads the sound data itself already.
 * Prevent copy/assignment of WbSoundClip as it now hold a heap
   allocated member (mDevice).
@gabryelreyes gabryelreyes linked a pull request Aug 2, 2024 that will close this issue
gabryelreyes added a commit that referenced this issue Aug 5, 2024
* Improve sound support for external controller (#6591)

Stream sound data from controller to Webots if the sound file path
is accesible by the controller process. This allows external
controller to use sounds even if the file path is not accessible
from Webots.

 * Controller: Try to load sound data on controller side and stream
   it to webots as part of the C_SPEAKER_PLAY_SOUND message. (speaker.c)
 * Webots: Read controller uploaded sound data in C_SPEAKER_PLAY_SOUND
   message (WbSpeaker). Use former file based load method if no data
   was streamed from controller.
 * Streamed sound data is cached inside WbSpeaker instance as controller
   provides the data only on first use of a sound file.
 * Updated speaker.md to explain when wb_speaker_play_sound()
   streams loads the sound data itself already.
 * Prevent copy/assignment of WbSoundClip as it now hold a heap
   allocated member (mDevice).

* WbSpeaker.cpp - fix source indentation

* Fix comment typo in speaker.c

* Address cppcheck issues

* Added changelog entry for this PR

Fixed ubuntu tester src formatting issue.

* Fix formatting issue in WbSpeaker.cpp

* Fixed bug radar.py (#6606)

* fix bug radar.py

Fixed the bug that when using Python to write a controller, using getTargets() could not correctly obtain multiple target data detected by radar

* Fixed the bug in obtaining radar detection target information#6606

Fixed the bug that when using Python to write a controller, using getTargets() could not correctly obtain multiple target data detected by radar

---------

Co-authored-by: Olivier Michel <Olivier.Michel@cyberbotics.com>

* Added changelog entry for this PR

Fixed ubuntu tester src formatting issue.

* Addressed code review comments

#6605

* Incorporated review feedback

Added improvement provided by CoolSpy3.

* Update src/controller/c/speaker.c

---------

Co-authored-by: Olivier Michel <Olivier.Michel@cyberbotics.com>
Co-authored-by: Gabryel Reyes <66941456+gabryelreyes@users.noreply.github.com>
Co-authored-by: lonely-poppy <77427326+lonely-poppy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants