Proposed API updates to audio functions about morse-decoder HOT 12 OPEN

Hello Chris,

What I had in my mind was something similar to your proposed solution, though your solution is more enthusiastically thought out than mine. Yet, you are way more familiar with the WebAudio API than I am.

let pausedTime = 0;

const createSource = () => {
  // recreate source
};

const play = async () => {
  await render;
  if (pausedTime > 0) {
    createSource();
  }
  source.start(context.currentTime, pausedTime);
  timeout = setTimeout(() => { stop(); }, (totalTime - pausedTime) * 1000);
};

const pause = () => {
  pausedTime = context.currentTime;
  stop(false);
};

const stop = (dispose = true) => {
  clearTimeout(timeout);
  timeout = 0;
  source.stop(0);
  if (dispose) {
    pausedTime = 0;
  }
};

Other than these, I completely agree with renaming onended. But also maybe change methods to like onsuspended, onrunning, and onclosed to comply with the audio states and add onready. We can also implement getCurrentTime() and getTotalTime().

from morse-decoder.

Looks really perfect, that's some neat progress, @chris--jones.

The only problem I noticed is, if someone doesn't pass audio options, then the function doesn't work?

// Doesn't work
a = morse.audio('SOS');

// Works
a = morse.audio('SOS', {
  audio: {
   wpm: 20,
   frequency: 550,  // value in hertz
   onstopped: function () { // event that fires when the tone stops playing
     console.log('ended');
   },
});

I believe that if the input isn't too lengthy, disposal isn't a problem. While the idea of a timeout makes sense, I'm unsure about how to determine its duration.

For seek, according to this comment, we need to create a new buffer node. But although this looks easy, I think it is a bit tough? I tried something like below, but it isn't working.

const play = async (offset = 0) => {
    var _a;
    if (!sourceStarted) {
      sourceStarted = true;
      currentTimeOffset = (context === null || context === void 0 ? void 0 : context.currentTime) || 0;
      await initAudio();
      if (audioBuffer && offset > audioBuffer.duration) {
        return;
      }
      source.start(0, offset);
      (_a = options.audio.onstarted) === null || _a === void 0 ? void 0 : _a.bind(source)();
    }
    else if ((context === null || context === void 0 ? void 0 : context.state) === 'suspended') {
      context.resume();
    }
};

const seek = (seekSeconds) => {
    stop();
    play(seekSeconds);
};

Note: I mistakenly closed the issue with Github keyboard shortcuts.

from morse-decoder.

Looking ahead, maybe we should also drop the ES5 support. With the 4.0.0, we might even get rid of some extra stuff and remove the dist folder. What do you think?

I think at the very least we could possibly offer an ES6 only build which would cut down on the bloat, I'm not sure if this is easy to do conditionally, but I'd hope we could set up GitHub actions and automate it.

it's interesting that the Web Audio tool hasn't changed much since 2014.

I think there have been a few fixes and minor changes, and a decent amount has been proposed, but some of these things seem to stay in proposed state for years :) https://webaudio.github.io/web-audio-api/

from morse-decoder.

Perfect, these GitHub Actions are really useful. I need to delve deeper into them. Feel free to send a PR any time.

from morse-decoder.

Sorry, haven't forgotten about this - just started a new job and have been flat out.

from morse-decoder.

Thanks for the input @ozdemirburak - I actually wasn't familiar with the suspend/resume, I will do some experimenting 😄

I had one other question though, I noticed that we have this line to trigger the stop:
timeout = setTimeout(() => { stop() }, totalTime * 1000);

I think this was done because originally stop was called specifying a scheduled time oscillator.stop(context.currentTime + t);
and then the stop method also called oscillator.stop(0).

In my experimentation - neither are required, the oscillator will simply stop on its own and the provided stop method in the morse-decoder api may be used if stopping sooner is required.

I'm also seeing this warning in the browser, although it's seemingly not causing any issues.

I will attempt to fix this along the way as well.

from morse-decoder.

Yes, you're right; we don't need them.

I don't really know how to fix it if there's no user action. So, probably okay to ignore that error. And as you said, it's still working, even without an interaction (const helloAudio = morse.audio('Hello world')).

from morse-decoder.

WIP: https://github.com/chris--jones/morse-decoder/tree/feature/audio-pause

I sort of fixed the audio context problem by initialising it, if needed, in the play function. This won't fix it if play is triggered outside of a user interaction, but it's less likely to occur in this configuration.

I have to amend my original design a little bit - I forgot totalTime was pre-calculated (no function needed, this is ready on initialisation). You can suspend and resume the audioContext to pause and play the audio, however the currentTime keeps progressing even after audio is stopped; I've worked around this for now by tracking an offset when play is triggered.

Working:

• play and pause / stop and play again
• start/stop/onstatechange events
• getCurrentTime() / totalTime

To Do:

• dispose the audio buffer - I'm thinking of doing this on a timeout that resets if you call another audio function, I already changed the render promise to a function that renders the buffer only if not already rendered. I'm also wondering if any of this is needed because I don't actually know if the audio takes up a significant amount of resources or not.
• seek or set current time

from morse-decoder.

The only problem I noticed is, if someone doesn't pass audio options, then the function doesn't work?

Ah yes, sorry I moved the audio config into a separate section as now we've got some new audio options and events that aren't oscillator related. I also realised this is after I suggested to you that we shouldn't introduce breaking changes without good reason, but I think this grouping of options makes a bit more sense😅

I do want your opinion on the options structure though. We could always put the oscillator specific config back in oscillator section and leave the other audio options outside of it for example.
In any case I've yet to do regression testing to make sure that sensible defaults all load and everything works without specifying config.

I believe that if the input isn't too lengthy, disposal isn't a problem. While the idea of a timeout makes sense, I'm unsure about how to determine its duration.

I'm tempted to either not implement automatic disposal (I think we should still offer the dispose method though to clean up resource usage), or just dispose after every playback.
The more I experiment with this the more I'm thinking we should rely on existing audio api functionality as much as possible to not over-complicate the API or re-invent the wheel. I was pleasantly surprised that I could remove the timeout logic for example and that the AudioContext already supplied suspend and resume.
Where I'm going with this is that some components seem designed to only be used once like the Oscillator node (see also https://blog.szynalski.com/2014/04/web-audio-api/).

For seek, according to this comment, we need to create a new buffer node. But although this looks easy, I think it is a bit tough? I tried something like below, but it isn't working.

I'll have a look at that next, it shouldn't be that tricky!

from morse-decoder.

We could always put the oscillator specific config back in oscillator section and leave the other audio options outside of it for example.

Sure this sounds reasonable, and I think your example would be a great solution, clean enough.

Looking ahead, maybe we should also drop the ES5 support. With the 4.0.0, we might even get rid of some extra stuff and remove the dist folder. What do you think?

The source you provided is perfect. And, I agree, we shouldn't make things harder than they need to be. I think offering an option for automatic disposal would be excellent. Additionally, in the end, a solution without using timeout would be awesome.

By the way, it's interesting that the Web Audio API hasn't changed much since 2014.

Thanks a lot.

from morse-decoder.

I just realised our target in the tsconfig is already ES2017, so we're already past ES6 support (2015)
Targeting esnext saves about 1kb of code and may be done conditionally by overriding the tsconfig in rollup.config.mjs (we could use an env var to produce multiple target outputs.

import typescript from '@rollup/plugin-typescript';

export default {
  input: './src/index.ts',
  output: {
    file: './src/index.js',
    format: 'cjs'
  },
  plugins: [typescript({ target: "esnext"})]
};

Here's an example of a GitHub actions setup that produces the release build and would push the asset if the release is tagged: https://github.com/chris--jones/morse-decoder/actions/runs/6681331395/job/18155374660

from morse-decoder.

Not a problem, congratulations on the new job, @chris--jones.

from morse-decoder.