Moddable-OpenSource / moddable

Tools for developers to create truly open IoT products using standard JavaScript on low cost microcontrollers.
http://www.moddable.com
1.35k stars 238 forks source link

Load an arbitrary file with AudioIn in a simulator. #1390

Closed stc1988 closed 2 months ago

stc1988 commented 3 months ago

Build environment: macOS Moddable SDK version: c40c945 Target device: desktop simulator

Steps to Reproduce

  1. cd $MODDABLE/examples/pins/audioin/replay and add resource that I want to load to manifest.json.
"data": {
    "sim": "$(MODDABLE)/examples/assets/sounds/wilhelm-scream"
}

2 Build and install the app using this build command: mcconfig -d -m

  1. Play bflatmajor

Other information

I confirmed data section of $MODDABLE/build/tmp/mac/mc/debug/replay/manifest_flat.json.

 "data": {
        "sim": [
            "/Users/satoshi/Projects/moddable/examples/assets/sounds/bflatmajor",
            "/Users/satoshi/Projects/moddable/examples/assets/sounds/wilhelm-scream"
        ]
},

Key sim has multiple value and $MODDABLE/build/tmp/mac/mc/debug/replay/resources/sim.wav is bflatmajor.

I’m not sure if the manifest is being loaded correctly, but being able to load arbitrary files would make it more convenient to develop features using AudioIn.

phoddie commented 3 months ago

I think this issue is a feature request to provide a way to override the default WAVE file used by audio input simulator.

Today the only way to do that is to modify the path in $MODDABLE/examples/pins/audioin/replay/manifest.json.

You tried overriding that path by modifying the path in the replay example. That didn't give the expected result because of the way mcconfig merges the manifests. I don't think we should try to change that in mcconfig because it could easily have unintended side-effects that could cause problems with other projects.

One easy approach would be to provide an option to the constructor to pass the name of the resource. There are currently no arguments to the constructor, so this will not cause any compatibility issues:

    constructor(options) {
        this.#wav = new Resource(options?.resource ?? "sim.wav");       

The disadvantage of this approach is that now the calling code includes knowledge of the simulator which is misleading when running on the device:

new AudioIn{{resource: "wilhelm-scream.wav"});

An alternative is to use a config value set in the project manifest:

    constructor(options) {
        this.#wav = new Resource(config.audioInWave ?? "sim.wav");      

What approach do you think makes the most sense?

stc1988 commented 3 months ago

I think this issue is a feature request to provide a way to override the default WAVE file used by audio input simulator.

Sorry for the confusion with the question, but this is a feature request.

One easy approach would be to provide an option to the constructor to pass the name of the resource. There are currently no arguments to the constructor, so this will not cause any compatibility issues: The disadvantage of this approach is that now the calling code includes knowledge of the simulator which is misleading when running on the device:

Yes, I agree. It would be better if there were source code compatibility between the simulator and the device.

Therefore, using a config value might be more convenient, as it allows the project to override the settings.​

phoddie commented 3 months ago

No problem. I agree that the config approach makes the most sense here. I'll commit that.

FYI – I noticed that the replay example didn't play the default sound correctly. That is because the source data is stern, which the audioIn module and the replay example didn't handle correctly. I've got fixes for that as well (not so important as stereo audio input on embedded is relatively rare, but... the example should work correctly.

phoddie commented 2 months ago

Fixed.