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.

Sign up for GitHub

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

[webaudio] Allow stopping play #3766

Merged
merged 1 commit into from
Nov 1, 2023
Merged

[webaudio] Allow stopping play #3766

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 10 additions & 21 deletions ....core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.openhab.core.audio.AudioSinkAsync;
import org.openhab.core.audio.AudioStream;
import org.openhab.core.audio.StreamServed;
import org.openhab.core.audio.URLAudioStream;
import org.openhab.core.audio.UnsupportedAudioFormatException;
import org.openhab.core.audio.UnsupportedAudioStreamException;
import org.openhab.core.events.EventPublisher;
Expand Down Expand Up @@ -67,29 +66,19 @@ public void processAsynchronously(@Nullable AudioStream audioStream)
if (audioStream == null) {
// in case the audioStream is null, this should be interpreted as a request to end any currently playing
// stream.
logger.debug("Web Audio sink does not support stopping the currently playing stream.");
sendEvent("");
return;
}
logger.debug("Received audio stream of format {}", audioStream.getFormat());
if (audioStream instanceof URLAudioStream urlAudioStream) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can we remove this different handling here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can we remove this different handling here?

I should have explained it better, sorry.

The two cases, as it is now :

  • if we recognize an URLAudioStream, we send the URL to the webui, and it will play it directly. It can be (and probably will ?) on another network host / internet. So potential CORS issue.
  • If it is not an URLAudioStream, we don't have the choice : we set up the audio servlet to serve the sound. We then send the audioservlet URL to the webui, and the webui will call it to get the sound. From the webui point of view, it is local, so no CORS issue.

By removing the first case, we ensure that the webui will always use the openHAB audio servlet to get the sound.
The URLAudioStream will be used as any other kind of stream : the audioservlet will get the audio sound and act as a proxy.

But there is still an issue with infinite stream. webUI (and webaudio API as it is used) does not support unlimited stream. I'm not knowledgeable enough of the webaudio API for this kind of modification (and it seems pretty hard).

I forgot to mention what happens. The webui will try to fully load the sound before playing it (no streaming support for the kind of play method we use here). Of course an infinite stream has no end, and therefore the sound will never start playing. Incredibly, streaming audio is not standard with the webaudio API.

try (AudioStream stream = urlAudioStream) {
// in this case only, we need to close the stream by ourself in a try with block,
// because nothing will consume it
// it is an external URL, so we can directly pass this on.
sendEvent(urlAudioStream.getURL());
} catch (IOException e) {
logger.debug("Error while closing the audio stream: {}", e.getMessage(), e);
}
} else {
// we need to serve it for a while and make it available to multiple clients
try {
StreamServed servedStream = audioHTTPServer.serve(audioStream, 10, true);
// we will let the HTTP servlet run the delayed task when finished with the stream
servedStream.playEnd().thenRun(() -> this.playbackFinished(audioStream));
sendEvent(servedStream.url());
} catch (IOException e) {
logger.warn("Cannot precache the audio stream to serve it", e);
}

// we need to serve it for a while and make it available to multiple clients
try {
StreamServed servedStream = audioHTTPServer.serve(audioStream, 10, true);
// we will let the HTTP servlet run the delayed task when finished with the stream
servedStream.playEnd().thenRun(() -> this.playbackFinished(audioStream));
sendEvent(servedStream.url());
} catch (IOException e) {
logger.warn("Cannot precache the audio stream to serve it", e);
}
}

Expand Down