From fc83ff2c08fa397000255adfa8d87065a789f8f0 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Sat, 11 Mar 2023 03:23:04 +0100 Subject: [PATCH 01/11] [audio] More capabilities for AudioSink using the AudioServlet AudioServlet can now serve all type of AudioStream multiple times by buffering data in memory or in temporary file. Adding method to ease disposal of temporary file after playing a sound Adding an identifyier to audio stream for further development (allow audio sink to cache computation data) Signed-off-by: Gwendal Roulleau --- .../openhab/core/audio/AudioHTTPServer.java | 8 +- .../org/openhab/core/audio/AudioSink.java | 16 +++ .../org/openhab/core/audio/AudioStream.java | 11 ++ .../core/audio/internal/AudioManagerImpl.java | 9 ++ .../core/audio/internal/AudioServlet.java | 101 +++++++++++++++--- .../internal/webaudio/WebAudioAudioSink.java | 18 ++-- .../core/voice/internal/DialogProcessor.java | 17 ++- .../core/voice/internal/VoiceManagerImpl.java | 10 ++ .../internal/cache/AudioStreamFromCache.java | 9 +- .../voice/internal/cache/TTSLRUCacheImpl.java | 2 +- .../core/cache/lru/LRUMediaCacheEntry.java | 4 + .../org/openhab/core/common/Disposable.java | 28 +++++ 12 files changed, 205 insertions(+), 28 deletions(-) create mode 100644 bundles/org.openhab.core/src/main/java/org/openhab/core/common/Disposable.java diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java index 9788b3acbdd..d7192a38700 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java @@ -40,13 +40,15 @@ public interface AudioHTTPServer { /** * Creates a relative url for a given {@link AudioStream} where it can be requested multiple times within the given * time frame. - * This method only accepts {@link FixedLengthAudioStream}s, since it needs to be able to create multiple concurrent - * streams from it, which isn't possible with a regular {@link AudioStream}. + * This method accepts all {@link AudioStream}s, but it is better to use {@link FixedLengthAudioStream}s since it + * needs to be able to create multiple concurrent streams from it. + * If generic {@link AudioStream} is used, we try to keep this capability by storing it in a small memory buffer, + * e.g {@link ByteArrayAudioStream}, or in a cached file if the stream is too long. * Streams are closed, once they expire. * * @param stream the stream to serve on HTTP * @param seconds number of seconds for which the stream is available through HTTP * @return the relative URL to access the stream starting with a '/' */ - String serve(FixedLengthAudioStream stream, int seconds); + String serve(AudioStream stream, int seconds); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java index f1ecca1d43c..3c68440a508 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java @@ -58,6 +58,9 @@ public interface AudioSink { * * In case the audioStream is null, this should be interpreted as a request to end any currently playing stream. * + * If you call this method and if the sink is synchronous, you should thereafter get rid of a stream implementing + * the {@link org.openhab.core.common.Disposable} interface by calling the dispose method. + * * @param audioStream the audio stream to play or null to keep quiet * @throws UnsupportedAudioFormatException If audioStream format is not supported * @throws UnsupportedAudioStreamException If audioStream is not supported @@ -94,4 +97,17 @@ void process(@Nullable AudioStream audioStream) * @throws IOException if the volume can not be set */ void setVolume(PercentType volume) throws IOException; + + /** + * Tell if the sink is synchronous. + * If true, caller may dispose of the stream immediately after the process method. + * On the contrary, if in the process method, the sink returns before the input stream is entirely consumed, + * then the sink should override this method and return false. + * Please note that by doing so, the sink should then take care itself of the InputStream implementing the + * {@link org.openhab.core.common.Disposable} interface by calling the dispose method when finishing + * reading it. + */ + default boolean isSynchronous() { + return true; + } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioStream.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioStream.java index c122aaf5fc6..560fcc21278 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioStream.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioStream.java @@ -15,6 +15,7 @@ import java.io.InputStream; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; /** * Wrapper for a source of audio data. @@ -37,4 +38,14 @@ public abstract class AudioStream extends InputStream { * @return The supported audio format */ public abstract AudioFormat getFormat(); + + /** + * Usefull for sinks playing the same stream multiple times, + * to avoid already done computation (like reencoding). + * + * @return A string uniquely identifying the stream. + */ + public @Nullable String getId() { + return null; + } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java index 914bedf9115..7b51bab8b42 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java @@ -41,6 +41,7 @@ import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; import org.openhab.core.audio.utils.ToneSynthesizer; +import org.openhab.core.common.Disposable; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; import org.openhab.core.config.core.ParameterOption; @@ -142,8 +143,16 @@ public void play(@Nullable AudioStream audioStream, @Nullable String sinkId, @Nu } try { sink.process(audioStream); + // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance + // to auto dispose: + if (sink.isSynchronous() && audioStream instanceof Disposable disposableAudioStream) { + disposableAudioStream.dispose(); + } } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { logger.warn("Error playing '{}': {}", audioStream, e.getMessage(), e); + } catch (IOException e) { + String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; + logger.warn("Cannot dispose of audio stream {}", fileName, e); } finally { if (volume != null && oldVolume != null) { // restore volume only if it was set before diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index 9bcee8cef5b..e727ee84670 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -12,8 +12,12 @@ */ package org.openhab.core.audio.internal; +import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Files; import java.util.Collections; import java.util.List; import java.util.Map; @@ -22,6 +26,7 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -31,13 +36,17 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.audio.AudioException; import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioHTTPServer; import org.openhab.core.audio.AudioStream; +import org.openhab.core.audio.ByteArrayAudioStream; +import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.FixedLengthAudioStream; +import org.openhab.core.common.Disposable; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; import org.osgi.service.http.whiteboard.propertytypes.HttpWhiteboardServletName; @@ -60,12 +69,16 @@ public class AudioServlet extends HttpServlet implements AudioHTTPServer { private static final List WAV_MIME_TYPES = List.of("audio/wav", "audio/x-wav", "audio/vnd.wave"); + // this one MB buffer will help playing multiple times an AudioStream, if the sink cannot do otherwise + private static final int ONETIME_STREAM_BUFFER_MAX_SIZE = 1048576; + static final String SERVLET_PATH = "/audio"; private final Logger logger = LoggerFactory.getLogger(AudioServlet.class); private final Map oneTimeStreams = new ConcurrentHashMap<>(); private final Map multiTimeStreams = new ConcurrentHashMap<>(); + private final Map currentlyServedResponseByStreamId = new ConcurrentHashMap<>(); private final Map streamTimeouts = new ConcurrentHashMap<>(); @@ -160,12 +173,18 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se .map(String::trim).collect(Collectors.toList()); try (final InputStream stream = prepareInputStream(streamId, resp, acceptedMimeTypes)) { - if (stream == null) { - logger.debug("Received request for invalid stream id at {}", requestURI); - resp.sendError(HttpServletResponse.SC_NOT_FOUND); - } else { - stream.transferTo(resp.getOutputStream()); - resp.flushBuffer(); + try { + currentlyServedResponseByStreamId.computeIfAbsent(streamId, (String) -> new AtomicInteger()) + .incrementAndGet(); + if (stream == null) { + logger.debug("Received request for invalid stream id at {}", requestURI); + resp.sendError(HttpServletResponse.SC_NOT_FOUND); + } else { + stream.transferTo(resp.getOutputStream()); + resp.flushBuffer(); + } + } finally { + currentlyServedResponseByStreamId.get(streamId).decrementAndGet(); } } catch (final AudioException ex) { resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex.getMessage()); @@ -179,10 +198,24 @@ private synchronized void removeTimedOutStreams() { .map(Entry::getKey).collect(Collectors.toList()); toRemove.forEach(streamId -> { - // the stream has expired, we need to remove it! - final FixedLengthAudioStream stream = multiTimeStreams.remove(streamId); - streamTimeouts.remove(streamId); - tryClose(stream); + // the stream has expired, if no one is using it we need to remove it + Integer numberOfStreamServed = currentlyServedResponseByStreamId.getOrDefault(streamId, new AtomicInteger()) + .decrementAndGet(); + if (numberOfStreamServed <= 0) { + final FixedLengthAudioStream stream = multiTimeStreams.remove(streamId); + streamTimeouts.remove(streamId); + tryClose(stream); + if (stream instanceof Disposable disposableStream) { + try { + disposableStream.dispose(); + } catch (IOException e) { + String fileName = disposableStream instanceof FileAudioStream file ? file.toString() + : "unknown"; + logger.warn("Cannot delete temporary file {} for the AudioServlet", fileName, e); + } + } + } + logger.debug("Removed timed out stream {}", streamId); }); } @@ -195,13 +228,42 @@ public String serve(AudioStream stream) { } @Override - public String serve(FixedLengthAudioStream stream, int seconds) { + public String serve(AudioStream stream, int seconds) { String streamId = UUID.randomUUID().toString(); - multiTimeStreams.put(streamId, stream); + if (stream instanceof FixedLengthAudioStream fixedLengthAudioStream) { + multiTimeStreams.put(streamId, fixedLengthAudioStream); + } else { // we prefer a FixedLengthAudioStream, but we can try to make one + try { + multiTimeStreams.put(streamId, createFixedLengthInputStream(stream, streamId)); + } catch (AudioException | IOException e) { + logger.warn("Cannot precache the audio stream to serve it", e); + } + } streamTimeouts.put(streamId, System.nanoTime() + TimeUnit.SECONDS.toNanos(seconds)); return getRelativeURL(streamId); } + private FixedLengthAudioStream createFixedLengthInputStream(AudioStream stream, String streamId) + throws IOException, AudioException { + byte[] dataBytes = stream.readNBytes(ONETIME_STREAM_BUFFER_MAX_SIZE + 1); + FixedLengthAudioStream fixedLengthAudioStreamResult; + if (dataBytes.length <= ONETIME_STREAM_BUFFER_MAX_SIZE) { + // we will use an in memory buffer to avoid disk operation + fixedLengthAudioStreamResult = new ByteArrayAudioStream(dataBytes, stream.getFormat()); + } else { + // in memory max size exceeded, sound is too long, we will use a file + File tempFile = File.createTempFile(streamId, ".snd"); + tempFile.deleteOnExit(); + try (OutputStream outputStream = new FileOutputStream(tempFile)) { + outputStream.write(dataBytes); + stream.transferTo(outputStream); + } + fixedLengthAudioStreamResult = new TemporaryFileAudioStream(tempFile, stream.getFormat()); + } + tryClose(stream); + return fixedLengthAudioStreamResult; + } + Map getMultiTimeStreams() { return Collections.unmodifiableMap(multiTimeStreams); } @@ -213,4 +275,19 @@ Map getOneTimeStreams() { private String getRelativeURL(String streamId) { return SERVLET_PATH + "/" + streamId; } + + private static class TemporaryFileAudioStream extends FileAudioStream implements Disposable { + + private File file; + + public TemporaryFileAudioStream(File file, AudioFormat format) throws AudioException { + super(file, format); + this.file = file; + } + + @Override + public void dispose() throws IOException { + Files.delete(file.toPath()); + } + } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java index 7c9299b5847..2756cd49126 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java @@ -22,7 +22,6 @@ import org.openhab.core.audio.AudioHTTPServer; import org.openhab.core.audio.AudioSink; import org.openhab.core.audio.AudioStream; -import org.openhab.core.audio.FixedLengthAudioStream; import org.openhab.core.audio.URLAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; @@ -49,8 +48,7 @@ public class WebAudioAudioSink implements AudioSink { private final Logger logger = LoggerFactory.getLogger(WebAudioAudioSink.class); private static final Set SUPPORTED_AUDIO_FORMATS = Set.of(AudioFormat.MP3, AudioFormat.WAV); - private static final Set> SUPPORTED_AUDIO_STREAMS = Set - .of(FixedLengthAudioStream.class, URLAudioStream.class); + private static final Set> SUPPORTED_AUDIO_STREAMS = Set.of(AudioStream.class); private AudioHTTPServer audioHTTPServer; private EventPublisher eventPublisher; @@ -75,14 +73,9 @@ public void process(@Nullable AudioStream audioStream) if (audioStream instanceof URLAudioStream urlAudioStream) { // it is an external URL, so we can directly pass this on. sendEvent(urlAudioStream.getURL()); - } else if (audioStream instanceof FixedLengthAudioStream lengthAudioStream) { - // we need to serve it for a while and make it available to multiple clients, hence only - // FixedLengthAudioStreams are supported. - sendEvent(audioHTTPServer.serve(lengthAudioStream, 10)); } else { - throw new UnsupportedAudioStreamException( - "Web audio sink can only handle FixedLengthAudioStreams and URLAudioStreams.", - audioStream.getClass()); + // we need to serve it for a while and make it available to multiple clients + sendEvent(audioHTTPServer.serve(audioStream, 10).toString()); } } catch (IOException e) { logger.debug("Error while closing the audio stream: {}", e.getMessage(), e); @@ -123,4 +116,9 @@ public PercentType getVolume() throws IOException { public void setVolume(final PercentType volume) throws IOException { throw new IOException("Web Audio sink does not support volume level changes."); } + + @Override + public boolean isSynchronous() { + return false; + } } diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java index 2760edbd98e..cd5fba86e8c 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java @@ -25,9 +25,11 @@ import org.openhab.core.audio.AudioException; import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioStream; +import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; import org.openhab.core.audio.utils.ToneSynthesizer; +import org.openhab.core.common.Disposable; import org.openhab.core.events.EventPublisher; import org.openhab.core.i18n.TranslationProvider; import org.openhab.core.items.ItemUtil; @@ -147,7 +149,7 @@ private void initToneSynthesizer(@Nullable String listeningMelodyText) { /** * Starts a persistent dialog - * + * * @throws IllegalStateException if keyword spot service is misconfigured */ public void start() throws IllegalStateException { @@ -380,12 +382,25 @@ protected void say(@Nullable String text) { if (dialogContext.sink().getSupportedStreams().stream().anyMatch(clazz -> clazz.isInstance(audioStream))) { try { dialogContext.sink().process(audioStream); + // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance + // to auto dispose: + if (dialogContext.sink().isSynchronous() + && audioStream instanceof Disposable disposableAudioStream) { + disposableAudioStream.dispose(); + } } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { if (logger.isDebugEnabled()) { logger.debug("Error saying '{}': {}", text, e.getMessage(), e); } else { logger.warn("Error saying '{}': {}", text, e.getMessage()); } + } catch (IOException e) { + String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; + if (logger.isDebugEnabled()) { + logger.debug("Cannot dispose of stream {}", fileName, e); + } else { + logger.warn("Cannot dispose of stream {}", fileName); + } } } else { logger.warn("Failed playing audio stream '{}' as audio doesn't support it.", audioStream); diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java index f08df2250ab..bc3119da1ea 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java @@ -41,8 +41,10 @@ import org.openhab.core.audio.AudioSink; import org.openhab.core.audio.AudioSource; import org.openhab.core.audio.AudioStream; +import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; +import org.openhab.core.common.Disposable; import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -293,6 +295,14 @@ public void say(String text, @Nullable String voiceId, @Nullable String sinkId, } try { sink.process(audioStream); + // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance + // to auto dispose: + if (sink.isSynchronous() && audioStream instanceof Disposable disposableAudioStream) { + disposableAudioStream.dispose(); + } + } catch (IOException e) { + String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; + logger.warn("Cannot dispose of stream {}", fileName, e); } finally { if (volume != null && oldVolume != null) { // restore volume only if it was set before diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/AudioStreamFromCache.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/AudioStreamFromCache.java index 1cc79ea458a..360769944e3 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/AudioStreamFromCache.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/AudioStreamFromCache.java @@ -32,10 +32,12 @@ public class AudioStreamFromCache extends FixedLengthAudioStream { private InputStreamCacheWrapper inputStream; private AudioFormat audioFormat; + private String key; - public AudioStreamFromCache(InputStreamCacheWrapper inputStream, AudioFormatInfo audioFormat) { + public AudioStreamFromCache(InputStreamCacheWrapper inputStream, AudioFormatInfo audioFormat, String key) { this.inputStream = inputStream; this.audioFormat = audioFormat.toAudioFormat(); + this.key = key; } @Override @@ -101,4 +103,9 @@ public synchronized void reset() throws IOException { public boolean markSupported() { return inputStream.markSupported(); } + + @Override + public @Nullable String getId() { + return key; + } } diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/TTSLRUCacheImpl.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/TTSLRUCacheImpl.java index 03ba00db2ad..ace6df0dce6 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/TTSLRUCacheImpl.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/TTSLRUCacheImpl.java @@ -135,7 +135,7 @@ public AudioStream get(CachedTTSService tts, String text, Voice voice, AudioForm // we are sure that the cache is used, and so we can use an AudioStream // implementation that use convenient methods for some client, like getClonedStream() // or mark /reset - return new AudioStreamFromCache(inputStreamCacheWrapper, metadata); + return new AudioStreamFromCache(inputStreamCacheWrapper, metadata, key); } else { // the cache is not used, we can use the original response AudioStream return (AudioStream) fileAndMetadata.getInputStream(); diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/cache/lru/LRUMediaCacheEntry.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/cache/lru/LRUMediaCacheEntry.java index 74c83aaabde..8bf60773cc8 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/cache/lru/LRUMediaCacheEntry.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/cache/lru/LRUMediaCacheEntry.java @@ -25,6 +25,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.common.Disposable; import org.openhab.core.storage.Storage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -233,6 +234,9 @@ protected void closeStreamClient() throws IOException { if (inputStreamLocal != null) { inputStreamLocal.close(); } + if (inputStreamLocal instanceof Disposable disposableStream) { + disposableStream.dispose(); + } } } } finally { diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/common/Disposable.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/Disposable.java new file mode 100644 index 00000000000..b2b4fd9db0d --- /dev/null +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/Disposable.java @@ -0,0 +1,28 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.common; + +import java.io.IOException; + +import org.eclipse.jdt.annotation.NonNullByDefault; + +/** + * For resource needing a callback when they are not needed anymore. + * + * @author Gwendal Roulleau - Initial contribution + */ +@NonNullByDefault +@FunctionalInterface +public interface Disposable { + void dispose() throws IOException; +} From 120bccf9e2360e3fde5f85b5e7073b6fe9e87cf3 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Fri, 17 Mar 2023 18:45:43 +0100 Subject: [PATCH 02/11] [audio] More capabilities for AudioSink using the AudioServlet Clean the map counting the requests. Signed-off-by: Gwendal Roulleau --- .../main/java/org/openhab/core/audio/internal/AudioServlet.java | 1 + 1 file changed, 1 insertion(+) diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index e727ee84670..c4c6689492a 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -204,6 +204,7 @@ private synchronized void removeTimedOutStreams() { if (numberOfStreamServed <= 0) { final FixedLengthAudioStream stream = multiTimeStreams.remove(streamId); streamTimeouts.remove(streamId); + currentlyServedResponseByStreamId.remove(streamId); tryClose(stream); if (stream instanceof Disposable disposableStream) { try { From 8f80976e5b80896b4996f44622ec2abdef5befbf Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Thu, 30 Mar 2023 22:34:31 +0200 Subject: [PATCH 03/11] [audio] More capabilities for AudioSink using the AudioServlet We can now send audio with a Runnable for a delayed task to be executed after. This delayed task includes temporary file deletion and volume restoration. This is a no breaking change / no behaviour modification for other addon AudioSink, as existing AudioSink must explicitly override the old behaviour to use this capability. Add AudioSinkSync / AudioSinkAsync abstract classes to use this capability easily. WebAudioSink now implements this capability, with the help of a modified AudioServlet Signed-off-by: Gwendal Roulleau --- .../openhab/core/audio/AudioHTTPServer.java | 27 ++- .../org/openhab/core/audio/AudioSink.java | 46 ++-- .../openhab/core/audio/AudioSinkAsync.java | 86 +++++++ .../org/openhab/core/audio/AudioSinkSync.java | 63 +++++ .../core/audio/ClonableAudioStream.java | 35 +++ .../core/audio/FixedLengthAudioStream.java | 17 +- .../openhab/core/audio/URLAudioStream.java | 7 +- .../core/audio/internal/AudioManagerImpl.java | 26 +-- .../core/audio/internal/AudioServlet.java | 215 +++++++++++------- .../javasound/JavaSoundAudioSink.java | 3 +- .../internal/webaudio/WebAudioAudioSink.java | 28 +-- .../internal/AbstractAudioServletTest.java | 3 +- .../core/audio/internal/AudioServletTest.java | 51 ++++- .../core/voice/internal/DialogProcessor.java | 17 +- .../core/voice/internal/VoiceManagerImpl.java | 25 +- .../cache/lru/LRUMediaCacheEntryTest.java | 25 +- 16 files changed, 469 insertions(+), 205 deletions(-) create mode 100644 bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java create mode 100644 bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java create mode 100644 bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/ClonableAudioStream.java diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java index d7192a38700..4b897964f8e 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java @@ -40,10 +40,12 @@ public interface AudioHTTPServer { /** * Creates a relative url for a given {@link AudioStream} where it can be requested multiple times within the given * time frame. - * This method accepts all {@link AudioStream}s, but it is better to use {@link FixedLengthAudioStream}s since it + * This method accepts all {@link AudioStream}s, but it is better to use {@link ClonableAudioStream}s since it * needs to be able to create multiple concurrent streams from it. - * If generic {@link AudioStream} is used, we try to keep this capability by storing it in a small memory buffer, - * e.g {@link ByteArrayAudioStream}, or in a cached file if the stream is too long. + * If generic {@link AudioStream} is used, the method tries to add the Clonable capability by storing it in a small + * memory buffer, + * e.g {@link ByteArrayAudioStream}, or in a cached file if the stream reached the buffer capacity, or fails if the + * stream is too long. * Streams are closed, once they expire. * * @param stream the stream to serve on HTTP @@ -51,4 +53,23 @@ public interface AudioHTTPServer { * @return the relative URL to access the stream starting with a '/' */ String serve(AudioStream stream, int seconds); + + /** + * Creates a relative url for a given {@link AudioStream} where it can be requested multiple times within the given + * time frame. + * This method accepts all {@link AudioStream}s, but it is better to use {@link ClonableAudioStream}s since it + * needs to be able to create multiple concurrent streams from it. + * If generic {@link AudioStream} is used, method tries to add the Clonable capability by storing it in a small + * memory buffer, + * e.g {@link ByteArrayAudioStream}, or in a cached file if the stream reached the buffer capacity, or fails if the + * stream is too long. + * Streams are closed, once they expire. + * + * @param stream the stream to serve on HTTP + * @param seconds number of seconds for which the stream is available through HTTP + * @param a Runnable callback for cleaning resources. The AudioHTTPServer will run the callback when the stream is + * not used anymore and timed-out. + * @return the relative URL to access the stream starting with a '/' + */ + String serve(AudioStream stream, int seconds, Runnable callBack); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java index 3c68440a508..3ea7d1c0a37 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java @@ -58,8 +58,8 @@ public interface AudioSink { * * In case the audioStream is null, this should be interpreted as a request to end any currently playing stream. * - * If you call this method and if the sink is synchronous, you should thereafter get rid of a stream implementing - * the {@link org.openhab.core.common.Disposable} interface by calling the dispose method. + * When the process method ends, if the stream implements the {@link org.openhab.core.common.Disposable} interface, + * the sink should hereafter get rid of it by calling the dispose method. * * @param audioStream the audio stream to play or null to keep quiet * @throws UnsupportedAudioFormatException If audioStream format is not supported @@ -68,6 +68,35 @@ public interface AudioSink { void process(@Nullable AudioStream audioStream) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException; + /** + * Processes the passed {@link AudioStream}, and executes the whenFinished Runnable (or stores for later execution + * if the sink is asynchronous). + * + * If the passed {@link AudioStream} is not supported by this instance, an {@link UnsupportedAudioStreamException} + * is thrown. + * + * If the passed {@link AudioStream} has an {@link AudioFormat} not supported by this instance, + * an {@link UnsupportedAudioFormatException} is thrown. + * + * In case the audioStream is null, this should be interpreted as a request to end any currently playing stream. + * + * When the process method ends, if the stream implements the {@link org.openhab.core.common.Disposable} interface, + * the sink should hereafter get rid of it by calling the dispose method. + * + * @param audioStream the audio stream to play or null to keep quiet + * @param whenFinished A Runnable to run when the sound is finished playing. + * @throws UnsupportedAudioFormatException If audioStream format is not supported + * @throws UnsupportedAudioStreamException If audioStream is not supported + */ + default void process(@Nullable AudioStream audioStream, Runnable whenFinished) + throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + try { + process(audioStream); + } finally { + whenFinished.run(); + } + } + /** * Gets a set containing all supported audio formats * @@ -97,17 +126,4 @@ void process(@Nullable AudioStream audioStream) * @throws IOException if the volume can not be set */ void setVolume(PercentType volume) throws IOException; - - /** - * Tell if the sink is synchronous. - * If true, caller may dispose of the stream immediately after the process method. - * On the contrary, if in the process method, the sink returns before the input stream is entirely consumed, - * then the sink should override this method and return false. - * Please note that by doing so, the sink should then take care itself of the InputStream implementing the - * {@link org.openhab.core.common.Disposable} interface by calling the dispose method when finishing - * reading it. - */ - default boolean isSynchronous() { - return true; - } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java new file mode 100644 index 00000000000..04784349be2 --- /dev/null +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java @@ -0,0 +1,86 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.audio; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.common.Disposable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Definition of an audio output like headphones, a speaker or for writing to + * a file / clip. + * This version is asynchronous: when the process() method returns, the {@link AudioStream} + * may or may not be played, and we don't know when the delayed task will be executed. + * CAUTION : It is the responsibility of the implementing AudioSink class to call the runDelayedTask + * method when playing is done. + * + * @author Gwendal Roulleau - Initial contribution + */ +@NonNullByDefault +public abstract class AudioSinkAsync implements AudioSink { + + private final Logger logger = LoggerFactory.getLogger(AudioSinkAsync.class); + + private final Map runnableByAudioStream = new HashMap<>(); + + @Override + public void process(@Nullable AudioStream audioStream, Runnable whenFinished) + throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + + try { + if (audioStream != null) { + runnableByAudioStream.put(audioStream, whenFinished); + } + process(audioStream); + } finally { + if (audioStream == null) { + // No need to delay the post process task + whenFinished.run(); + } + } + } + + /** + * Will run the delayed task stored previously. + * + * @param audioStream The AudioStream is the key to find the delayed Runnable task in the storage. + */ + protected void runDelayed(AudioStream audioStream) { + Runnable delayedTask = runnableByAudioStream.remove(audioStream); + + if (delayedTask != null) { + delayedTask.run(); + } + + // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance + // to auto dispose: + if (audioStream instanceof Disposable disposableAudioStream) { + try { + disposableAudioStream.dispose(); + } catch (IOException e) { + String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; + if (logger.isDebugEnabled()) { + logger.debug("Cannot dispose of stream {}", fileName, e); + } else { + logger.warn("Cannot dispose of stream {}, reason {}", fileName, e.getMessage()); + } + } + } + } +} diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java new file mode 100644 index 00000000000..d6a1354cd94 --- /dev/null +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java @@ -0,0 +1,63 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.audio; + +import java.io.IOException; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.common.Disposable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Definition of an audio output like headphones, a speaker or for writing to + * a file / clip. + * This version is synchronous: when the process() method returns, + * the source is considered played, and could be disposed. + * Any delayed tasks can then be performed, such as volume restoration + * + * @author Gwendal Roulleau - Initial contribution + */ +@NonNullByDefault +public abstract class AudioSinkSync implements AudioSink { + + private final Logger logger = LoggerFactory.getLogger(AudioSinkSync.class); + + @Override + public void process(@Nullable AudioStream audioStream, Runnable whenFinished) + throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + + try { + process(audioStream); + } finally { + + whenFinished.run(); + + // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance + // to auto dispose: + if (audioStream instanceof Disposable disposableAudioStream) { + try { + disposableAudioStream.dispose(); + } catch (IOException e) { + String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; + if (logger.isDebugEnabled()) { + logger.debug("Cannot dispose of stream {}", fileName, e); + } else { + logger.warn("Cannot dispose of stream {}, reason {}", fileName, e.getMessage()); + } + } + } + } + } +} diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/ClonableAudioStream.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/ClonableAudioStream.java new file mode 100644 index 00000000000..111e1c1d2d9 --- /dev/null +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/ClonableAudioStream.java @@ -0,0 +1,35 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.audio; + +import java.io.InputStream; + +import org.eclipse.jdt.annotation.NonNullByDefault; + +/** + * This is an {@link AudioStream}, that can be cloned + * + * @author Gwendal Roulleau - Initial contribution, separation from FixedLengthAudioStream + */ +@NonNullByDefault +public abstract class ClonableAudioStream extends AudioStream { + + /** + * Returns a new, fully independent stream instance, which can be read and closed without impacting the original + * instance. + * + * @return a new input stream that can be consumed by the caller + * @throws AudioException if stream cannot be created + */ + public abstract InputStream getClonedStream() throws AudioException; +} diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FixedLengthAudioStream.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FixedLengthAudioStream.java index 6e47d35b054..4a737d50f99 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FixedLengthAudioStream.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FixedLengthAudioStream.java @@ -12,18 +12,16 @@ */ package org.openhab.core.audio; -import java.io.InputStream; - import org.eclipse.jdt.annotation.NonNullByDefault; /** - * This is an {@link AudioStream}, which can provide information about its absolute length and is able to provide - * cloned streams. + * This is a {@link ClonableAudioStream}, which can also provide information about its absolute length. * * @author Kai Kreuzer - Initial contribution + * @author Gwendal Roulleau - Separate getClonedStream into its own class */ @NonNullByDefault -public abstract class FixedLengthAudioStream extends AudioStream { +public abstract class FixedLengthAudioStream extends ClonableAudioStream { /** * Provides the length of the stream in bytes. @@ -31,13 +29,4 @@ public abstract class FixedLengthAudioStream extends AudioStream { * @return absolute length in bytes */ public abstract long length(); - - /** - * Returns a new, fully independent stream instance, which can be read and closed without impacting the original - * instance. - * - * @return a new input stream that can be consumed by the caller - * @throws AudioException if stream cannot be created - */ - public abstract InputStream getClonedStream() throws AudioException; } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/URLAudioStream.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/URLAudioStream.java index fef7e76f579..e43ab639760 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/URLAudioStream.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/URLAudioStream.java @@ -40,7 +40,7 @@ * @author Christoph Weitkamp - Refactored use of filename extension */ @NonNullByDefault -public class URLAudioStream extends AudioStream { +public class URLAudioStream extends ClonableAudioStream { private static final Pattern PLS_STREAM_PATTERN = Pattern.compile("^File[0-9]=(.+)$"); @@ -154,4 +154,9 @@ public void close() throws IOException { public String toString() { return url; } + + @Override + public InputStream getClonedStream() throws AudioException { + return new URLAudioStream(url); + } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java index 7b51bab8b42..75f966b3fcc 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java @@ -41,7 +41,6 @@ import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; import org.openhab.core.audio.utils.ToneSynthesizer; -import org.openhab.core.common.Disposable; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; import org.openhab.core.config.core.ParameterOption; @@ -141,28 +140,23 @@ public void play(@Nullable AudioStream audioStream, @Nullable String sinkId, @Nu e.getMessage(), e); } } - try { - sink.process(audioStream); - // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance - // to auto dispose: - if (sink.isSynchronous() && audioStream instanceof Disposable disposableAudioStream) { - disposableAudioStream.dispose(); - } - } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { - logger.warn("Error playing '{}': {}", audioStream, e.getMessage(), e); - } catch (IOException e) { - String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; - logger.warn("Cannot dispose of audio stream {}", fileName, e); - } finally { - if (volume != null && oldVolume != null) { + + final PercentType oldVolumeFinal = oldVolume; + Runnable toRunWhenProcessFinished = () -> { + if (volume != null && oldVolumeFinal != null) { // restore volume only if it was set before try { - sink.setVolume(oldVolume); + sink.setVolume(oldVolumeFinal); } catch (IOException e) { logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), e.getMessage(), e); } } + }; + try { + sink.process(audioStream, toRunWhenProcessFinished); + } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { + logger.warn("Error playing '{}': {}", audioStream, e.getMessage(), e); } } else { logger.warn("Failed playing audio stream '{}' as no audio sink was found.", audioStream); diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index c4c6689492a..bcc4714bfca 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -25,6 +25,8 @@ import java.util.Objects; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -36,7 +38,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.audio.AudioException; @@ -44,9 +45,11 @@ import org.openhab.core.audio.AudioHTTPServer; import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.ByteArrayAudioStream; +import org.openhab.core.audio.ClonableAudioStream; import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.FixedLengthAudioStream; import org.openhab.core.common.Disposable; +import org.openhab.core.common.ThreadPoolManager; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; import org.osgi.service.http.whiteboard.propertytypes.HttpWhiteboardServletName; @@ -69,27 +72,29 @@ public class AudioServlet extends HttpServlet implements AudioHTTPServer { private static final List WAV_MIME_TYPES = List.of("audio/wav", "audio/x-wav", "audio/vnd.wave"); - // this one MB buffer will help playing multiple times an AudioStream, if the sink cannot do otherwise + // A 1MB in memory buffer will help playing multiple times an AudioStream, if the sink cannot do otherwise private static final int ONETIME_STREAM_BUFFER_MAX_SIZE = 1048576; + // 5MB max for a file buffer + private static final int ONETIME_STREAM_FILE_MAX_SIZE = 5242880; static final String SERVLET_PATH = "/audio"; private final Logger logger = LoggerFactory.getLogger(AudioServlet.class); - private final Map oneTimeStreams = new ConcurrentHashMap<>(); - private final Map multiTimeStreams = new ConcurrentHashMap<>(); - private final Map currentlyServedResponseByStreamId = new ConcurrentHashMap<>(); + private final Map servedStreams = new ConcurrentHashMap<>(); - private final Map streamTimeouts = new ConcurrentHashMap<>(); + private final ScheduledExecutorService threadPool = ThreadPoolManager + .getScheduledPool(ThreadPoolManager.THREAD_POOL_NAME_COMMON); + @Nullable + ScheduledFuture periodicCleaner; + + private static final Runnable doNothing = () -> { + }; @Deactivate protected synchronized void deactivate() { - multiTimeStreams.values().forEach(this::tryClose); - multiTimeStreams.clear(); - streamTimeouts.clear(); - - oneTimeStreams.values().forEach(this::tryClose); - oneTimeStreams.clear(); + servedStreams.values().stream().map(streamServed -> streamServed.audioStream).forEach(this::tryClose); + servedStreams.clear(); } private void tryClose(@Nullable AudioStream stream) { @@ -101,29 +106,18 @@ private void tryClose(@Nullable AudioStream stream) { } } - private @Nullable InputStream prepareInputStream(final String streamId, final HttpServletResponse resp, + private InputStream prepareInputStream(final StreamServed servedStream, final HttpServletResponse resp, List acceptedMimeTypes) throws AudioException { - final AudioStream stream; - final boolean multiAccess; - if (oneTimeStreams.containsKey(streamId)) { - stream = oneTimeStreams.remove(streamId); - multiAccess = false; - } else if (multiTimeStreams.containsKey(streamId)) { - stream = multiTimeStreams.get(streamId); - multiAccess = true; - } else { - return null; - } - logger.debug("Stream to serve is {}", streamId); + logger.debug("Stream to serve is {}", servedStream.id); // try to set the content-type, if possible final String mimeType; - if (AudioFormat.CODEC_MP3.equals(stream.getFormat().getCodec())) { + if (AudioFormat.CODEC_MP3.equals(servedStream.audioStream.getFormat().getCodec())) { mimeType = "audio/mpeg"; - } else if (AudioFormat.CONTAINER_WAVE.equals(stream.getFormat().getContainer())) { + } else if (AudioFormat.CONTAINER_WAVE.equals(servedStream.audioStream.getFormat().getContainer())) { mimeType = WAV_MIME_TYPES.stream().filter(acceptedMimeTypes::contains).findFirst().orElse("audio/wav"); - } else if (AudioFormat.CONTAINER_OGG.equals(stream.getFormat().getContainer())) { + } else if (AudioFormat.CONTAINER_OGG.equals(servedStream.audioStream.getFormat().getContainer())) { mimeType = "audio/ogg"; } else { mimeType = null; @@ -133,16 +127,17 @@ private void tryClose(@Nullable AudioStream stream) { } // try to set the content-length, if possible - if (stream instanceof FixedLengthAudioStream audioStream) { - final long size = audioStream.length(); + if (servedStream.audioStream instanceof FixedLengthAudioStream fixedLengthServedStream) { + final long size = fixedLengthServedStream.length(); resp.setContentLength((int) size); } - if (multiAccess) { + if (servedStream.multiTimeStream + && servedStream.audioStream instanceof ClonableAudioStream clonableAudioStream) { // we need to care about concurrent access and have a separate stream for each thread - return ((FixedLengthAudioStream) stream).getClonedStream(); + return clonableAudioStream.getClonedStream(); } else { - return stream; + return servedStream.audioStream; } } @@ -159,7 +154,6 @@ private String substringBefore(String str, String separator) { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { - removeTimedOutStreams(); String requestURI = req.getRequestURI(); if (requestURI == null) { @@ -172,111 +166,156 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se List acceptedMimeTypes = Stream.of(Objects.requireNonNullElse(req.getHeader("Accept"), "").split(",")) .map(String::trim).collect(Collectors.toList()); - try (final InputStream stream = prepareInputStream(streamId, resp, acceptedMimeTypes)) { - try { - currentlyServedResponseByStreamId.computeIfAbsent(streamId, (String) -> new AtomicInteger()) - .incrementAndGet(); - if (stream == null) { - logger.debug("Received request for invalid stream id at {}", requestURI); - resp.sendError(HttpServletResponse.SC_NOT_FOUND); - } else { - stream.transferTo(resp.getOutputStream()); - resp.flushBuffer(); - } + StreamServed servedStream = servedStreams.get(streamId); + if (servedStream == null) { + logger.debug("Received request for invalid stream id at {}", requestURI); + resp.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } + + // we count the number of active process using the input stream + AtomicInteger currentlyServedStream = servedStream.currentlyServedStream; + if (currentlyServedStream.incrementAndGet() == 1 || servedStream.multiTimeStream) { + try (final InputStream stream = prepareInputStream(servedStream, resp, acceptedMimeTypes)) { + stream.transferTo(resp.getOutputStream()); + resp.flushBuffer(); + } catch (final AudioException ex) { + resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex.getMessage()); } finally { - currentlyServedResponseByStreamId.get(streamId).decrementAndGet(); + currentlyServedStream.decrementAndGet(); } - } catch (final AudioException ex) { - resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex.getMessage()); + } else { + logger.debug("Received request for already consumed stream id at {}", requestURI); + resp.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } + + // we can immediately dispose and remove, if it is a one time stream + if (!servedStream.multiTimeStream) { + servedStream.callback.run(); + servedStreams.remove(streamId); + logger.debug("Removed timed out stream {}", streamId); } } private synchronized void removeTimedOutStreams() { // Build list of expired streams. long now = System.nanoTime(); - final List toRemove = streamTimeouts.entrySet().stream().filter(e -> e.getValue() < now) + final List toRemove = servedStreams.entrySet().stream() + .filter(e -> e.getValue().timeout < now && e.getValue().currentlyServedStream.get() <= 0) .map(Entry::getKey).collect(Collectors.toList()); toRemove.forEach(streamId -> { - // the stream has expired, if no one is using it we need to remove it - Integer numberOfStreamServed = currentlyServedResponseByStreamId.getOrDefault(streamId, new AtomicInteger()) - .decrementAndGet(); - if (numberOfStreamServed <= 0) { - final FixedLengthAudioStream stream = multiTimeStreams.remove(streamId); - streamTimeouts.remove(streamId); - currentlyServedResponseByStreamId.remove(streamId); - tryClose(stream); - if (stream instanceof Disposable disposableStream) { - try { - disposableStream.dispose(); - } catch (IOException e) { - String fileName = disposableStream instanceof FileAudioStream file ? file.toString() - : "unknown"; - logger.warn("Cannot delete temporary file {} for the AudioServlet", fileName, e); - } - } + // the stream has expired and no one is using it, we need to remove it! + StreamServed streamServed = servedStreams.remove(streamId); + if (streamServed != null) { + tryClose(streamServed.audioStream); + // we can notify the caller of the stream consumption + streamServed.callback.run(); + logger.debug("Removed timed out stream {}", streamId); } - - logger.debug("Removed timed out stream {}", streamId); }); + + // Because the callback should be executed as soon as possible, + // we cannot wait for the next doGet to perform a clean. So we have to schedule a periodic cleaner. + ScheduledFuture periodicCleanerLocal = periodicCleaner; + if (!servedStreams.isEmpty()) { + if (periodicCleanerLocal == null || periodicCleanerLocal.isDone()) { + // reschedule a clean + periodicCleaner = threadPool.scheduleWithFixedDelay(this::removeTimedOutStreams, 5, 5, + TimeUnit.SECONDS); + } + } else if (periodicCleanerLocal != null) { // no more stream to serve, shut the periodic cleaning thread: + periodicCleanerLocal.cancel(true); + periodicCleaner = null; + } } @Override public String serve(AudioStream stream) { String streamId = UUID.randomUUID().toString(); - oneTimeStreams.put(streamId, stream); + // Because we cannot wait indefinitely before executing the callback, + // even if this is a one time stream, we set a timeout just in case. + long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + StreamServed streamToServe = new StreamServed(streamId, stream, new AtomicInteger(), timeOut, false, doNothing); + servedStreams.put(streamId, streamToServe); + + // try to clean, or a least launch the periodic cleanse: + removeTimedOutStreams(); + return getRelativeURL(streamId); } @Override public String serve(AudioStream stream, int seconds) { + return serve(stream, seconds, doNothing); + } + + @Override + public String serve(AudioStream stream, int seconds, Runnable callBack) { + String streamId = UUID.randomUUID().toString(); - if (stream instanceof FixedLengthAudioStream fixedLengthAudioStream) { - multiTimeStreams.put(streamId, fixedLengthAudioStream); - } else { // we prefer a FixedLengthAudioStream, but we can try to make one + ClonableAudioStream clonableAudioStream = null; + if (stream instanceof ClonableAudioStream clonableAudioStreamDetected) { + clonableAudioStream = clonableAudioStreamDetected; + } else { // we prefer a ClonableAudioStream, but we can try to make one try { - multiTimeStreams.put(streamId, createFixedLengthInputStream(stream, streamId)); + clonableAudioStream = createClonableInputStream(stream, streamId); } catch (AudioException | IOException e) { logger.warn("Cannot precache the audio stream to serve it", e); + return getRelativeURL(streamId); } } - streamTimeouts.put(streamId, System.nanoTime() + TimeUnit.SECONDS.toNanos(seconds)); + long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(seconds); + StreamServed streamToServe = new StreamServed(streamId, clonableAudioStream, new AtomicInteger(), timeOut, true, + callBack); + servedStreams.put(streamId, streamToServe); + + // try to clean, or a least launch the periodic cleanse: + removeTimedOutStreams(); + return getRelativeURL(streamId); } - private FixedLengthAudioStream createFixedLengthInputStream(AudioStream stream, String streamId) + private ClonableAudioStream createClonableInputStream(AudioStream stream, String streamId) throws IOException, AudioException { byte[] dataBytes = stream.readNBytes(ONETIME_STREAM_BUFFER_MAX_SIZE + 1); - FixedLengthAudioStream fixedLengthAudioStreamResult; + ClonableAudioStream clonableAudioStreamResult; if (dataBytes.length <= ONETIME_STREAM_BUFFER_MAX_SIZE) { // we will use an in memory buffer to avoid disk operation - fixedLengthAudioStreamResult = new ByteArrayAudioStream(dataBytes, stream.getFormat()); + clonableAudioStreamResult = new ByteArrayAudioStream(dataBytes, stream.getFormat()); } else { // in memory max size exceeded, sound is too long, we will use a file File tempFile = File.createTempFile(streamId, ".snd"); tempFile.deleteOnExit(); try (OutputStream outputStream = new FileOutputStream(tempFile)) { + // copy already read data to file : outputStream.write(dataBytes); - stream.transferTo(outputStream); + // copy the remaining stream data to a file. + byte[] buf = new byte[8192]; + int length; + // but with a limit + int fileSize = 0; + while ((length = stream.read(buf)) != -1 && fileSize < ONETIME_STREAM_FILE_MAX_SIZE) { + outputStream.write(buf, 0, length); + fileSize += length; + } } - fixedLengthAudioStreamResult = new TemporaryFileAudioStream(tempFile, stream.getFormat()); + clonableAudioStreamResult = new TemporaryFileAudioStream(tempFile, stream.getFormat()); } tryClose(stream); - return fixedLengthAudioStreamResult; + return clonableAudioStreamResult; } - Map getMultiTimeStreams() { - return Collections.unmodifiableMap(multiTimeStreams); - } - - Map getOneTimeStreams() { - return Collections.unmodifiableMap(oneTimeStreams); + Map getServedStreams() { + return Collections.unmodifiableMap(servedStreams); } private String getRelativeURL(String streamId) { return SERVLET_PATH + "/" + streamId; } + // Using a FileAudioStream implementing Disposable allows file deletion when it is not used anymore private static class TemporaryFileAudioStream extends FileAudioStream implements Disposable { private File file; @@ -291,4 +330,8 @@ public void dispose() throws IOException { Files.delete(file.toPath()); } } + + protected static record StreamServed(String id, AudioStream audioStream, AtomicInteger currentlyServedStream, + long timeout, boolean multiTimeStream, Runnable callback) { + } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java index 606b20e0195..bd6f0ac5e5f 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java @@ -32,6 +32,7 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioSink; +import org.openhab.core.audio.AudioSinkSync; import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.URLAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; @@ -55,7 +56,7 @@ */ @NonNullByDefault @Component(service = AudioSink.class, immediate = true) -public class JavaSoundAudioSink implements AudioSink { +public class JavaSoundAudioSink extends AudioSinkSync { private static final Logger LOGGER = LoggerFactory.getLogger(JavaSoundAudioSink.class); diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java index 2756cd49126..857b406a268 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java @@ -21,6 +21,7 @@ import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioHTTPServer; import org.openhab.core.audio.AudioSink; +import org.openhab.core.audio.AudioSinkAsync; import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.URLAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; @@ -43,7 +44,7 @@ */ @NonNullByDefault @Component(service = AudioSink.class, immediate = true) -public class WebAudioAudioSink implements AudioSink { +public class WebAudioAudioSink extends AudioSinkAsync { private final Logger logger = LoggerFactory.getLogger(WebAudioAudioSink.class); @@ -68,17 +69,21 @@ public void process(@Nullable AudioStream audioStream) logger.debug("Web Audio sink does not support stopping the currently playing stream."); return; } - try (AudioStream stream = audioStream) { - logger.debug("Received audio stream of format {}", audioStream.getFormat()); - if (audioStream instanceof URLAudioStream urlAudioStream) { + logger.debug("Received audio stream of format {}", audioStream.getFormat()); + if (audioStream instanceof URLAudioStream urlAudioStream) { + 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()); - } else { - // we need to serve it for a while and make it available to multiple clients - sendEvent(audioHTTPServer.serve(audioStream, 10).toString()); + } catch (IOException e) { + logger.debug("Error while closing the audio stream: {}", e.getMessage(), e); } - } catch (IOException e) { - logger.debug("Error while closing the audio stream: {}", e.getMessage(), e); + } else { + // we will let the HTTP servlet run the delayed task when finished with the stream + Runnable delayedTask = () -> this.runDelayed(audioStream); + // we need to serve it for a while and make it available to multiple clients + sendEvent(audioHTTPServer.serve(audioStream, 10, delayedTask).toString()); } } @@ -116,9 +121,4 @@ public PercentType getVolume() throws IOException { public void setVolume(final PercentType volume) throws IOException { throw new IOException("Web Audio sink does not support volume level changes."); } - - @Override - public boolean isSynchronous() { - return false; - } } diff --git a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java index 14224cb2ac6..458b6e47cf3 100644 --- a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java +++ b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java @@ -33,7 +33,6 @@ import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.ByteArrayAudioStream; -import org.openhab.core.audio.FixedLengthAudioStream; import org.openhab.core.test.TestPortUtil; import org.openhab.core.test.TestServer; import org.openhab.core.test.java.JavaTest; @@ -126,7 +125,7 @@ protected String serveStream(AudioStream stream, @Nullable Integer timeInterval) String path; if (timeInterval != null) { - path = audioServlet.serve((FixedLengthAudioStream) stream, timeInterval); + path = audioServlet.serve(stream, timeInterval); } else { path = audioServlet.serve(stream); } diff --git a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java index 13e4957b5a6..8740a358941 100644 --- a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java +++ b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java @@ -14,10 +14,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.*; import java.io.File; import java.util.concurrent.TimeUnit; @@ -29,8 +27,10 @@ import org.junit.jupiter.api.Test; import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioStream; +import org.openhab.core.audio.ByteArrayAudioStream; import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.FixedLengthAudioStream; +import org.openhab.core.audio.internal.AudioServlet.StreamServed; import org.openhab.core.audio.internal.utils.BundledSoundFileHandler; /** @@ -128,7 +128,7 @@ public void onlyOneRequestToOneTimeStreamsCanBeMade() throws Exception { } @Test - public void requestToMultitimeStreamCannotBeDoneAfterTheTimeoutOfTheStreamHasExipred() throws Exception { + public void requestToMultitimeStreamCannotBeDoneAfterTheTimeoutOfTheStreamHasExpired() throws Exception { final int streamTimeout = 3; AudioStream audioStream = getByteArrayAudioStream(testByteArray, AudioFormat.CONTAINER_NONE, @@ -151,8 +151,8 @@ public void requestToMultitimeStreamCannotBeDoneAfterTheTimeoutOfTheStreamHasExi assertThat("The response media type was not as expected", response.getMediaType(), is(MEDIA_TYPE_AUDIO_MPEG)); - assertThat("The audio stream was not added to the multitime streams", - audioServlet.getMultiTimeStreams().containsValue(audioStream), is(true)); + assertThat("The audio stream was not added to the multitime streams", audioServlet.getServedStreams() + .values().stream().map(StreamServed::audioStream).toList().contains(audioStream), is(true)); } waitForAssert(() -> { @@ -161,14 +161,37 @@ public void requestToMultitimeStreamCannotBeDoneAfterTheTimeoutOfTheStreamHasExi } catch (Exception e) { throw new IllegalStateException(e); } - assertThat("The audio stream was not removed from multitime streams", - audioServlet.getMultiTimeStreams().containsValue(audioStream), is(false)); + assertThat("The audio stream was not removed from multitime streams", audioServlet.getServedStreams() + .values().stream().map(StreamServed::audioStream).toList().contains(audioStream), is(false)); }); response = getHttpRequest(url).send(); assertThat("The response status was not as expected", response.getStatus(), is(HttpStatus.NOT_FOUND_404)); } + @Test + public void oneTimeStreamIsRecreatedAsAClonable() throws Exception { + AudioStream audioStream = mock(AudioStream.class); + AudioFormat audioFormat = mock(AudioFormat.class); + when(audioStream.getFormat()).thenReturn(audioFormat); + when(audioFormat.getCodec()).thenReturn(AudioFormat.CODEC_MP3); + when(audioStream.readNBytes(anyInt())).thenReturn(testByteArray); + + String url = serveStream(audioStream, 10); + String uuid = url.substring(url.lastIndexOf("/") + 1); + StreamServed servedStream = audioServlet.getServedStreams().get(uuid); + + // does not contain directly the stream because it is now a new stream wrapper + assertThat(servedStream.audioStream(), not(audioStream)); + // it is now a ByteArrayAudioStream wrapper : + assertThat(servedStream.audioStream(), instanceOf(ByteArrayAudioStream.class)); + + ContentResponse response = getHttpRequest(url).send(); + assertThat("The response content was not as expected", response.getContent(), is(testByteArray)); + + verify(audioStream).close(); + } + @Test public void oneTimeStreamIsClosedAndRemovedAfterServed() throws Exception { AudioStream audioStream = mock(AudioStream.class); @@ -177,11 +200,14 @@ public void oneTimeStreamIsClosedAndRemovedAfterServed() throws Exception { when(audioFormat.getCodec()).thenReturn(AudioFormat.CODEC_MP3); String url = serveStream(audioStream); + assertThat(audioServlet.getServedStreams().values().stream().map(StreamServed::audioStream).toList(), + contains(audioStream)); getHttpRequest(url).send(); verify(audioStream).close(); - assertThat(audioServlet.getOneTimeStreams().values(), not(contains(audioStream))); + assertThat(audioServlet.getServedStreams().values().stream().map(StreamServed::audioStream).toList(), + not(contains(audioStream))); } @Test @@ -198,6 +224,8 @@ public void multiTimeStreamIsClosedAfterExpired() throws Exception { when(audioFormat.getCodec()).thenReturn(AudioFormat.CODEC_MP3); String url = serveStream(audioStream, 2); + assertThat(audioServlet.getServedStreams().values().stream().map(StreamServed::audioStream).toList(), + contains(audioStream)); waitForAssert(() -> { try { @@ -210,7 +238,8 @@ public void multiTimeStreamIsClosedAfterExpired() throws Exception { }); verify(audioStream).close(); - assertThat(audioServlet.getMultiTimeStreams().values(), not(contains(audioStream))); + assertThat(audioServlet.getServedStreams().values().stream().map(StreamServed::audioStream).toList(), + not(contains(audioStream))); verify(clonedStream, times(cloneCounter.get())).close(); } diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java index cd5fba86e8c..2760edbd98e 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java @@ -25,11 +25,9 @@ import org.openhab.core.audio.AudioException; import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioStream; -import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; import org.openhab.core.audio.utils.ToneSynthesizer; -import org.openhab.core.common.Disposable; import org.openhab.core.events.EventPublisher; import org.openhab.core.i18n.TranslationProvider; import org.openhab.core.items.ItemUtil; @@ -149,7 +147,7 @@ private void initToneSynthesizer(@Nullable String listeningMelodyText) { /** * Starts a persistent dialog - * + * * @throws IllegalStateException if keyword spot service is misconfigured */ public void start() throws IllegalStateException { @@ -382,25 +380,12 @@ protected void say(@Nullable String text) { if (dialogContext.sink().getSupportedStreams().stream().anyMatch(clazz -> clazz.isInstance(audioStream))) { try { dialogContext.sink().process(audioStream); - // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance - // to auto dispose: - if (dialogContext.sink().isSynchronous() - && audioStream instanceof Disposable disposableAudioStream) { - disposableAudioStream.dispose(); - } } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { if (logger.isDebugEnabled()) { logger.debug("Error saying '{}': {}", text, e.getMessage(), e); } else { logger.warn("Error saying '{}': {}", text, e.getMessage()); } - } catch (IOException e) { - String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; - if (logger.isDebugEnabled()) { - logger.debug("Cannot dispose of stream {}", fileName, e); - } else { - logger.warn("Cannot dispose of stream {}", fileName); - } } } else { logger.warn("Failed playing audio stream '{}' as audio doesn't support it.", audioStream); diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java index bc3119da1ea..a0697add376 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java @@ -41,10 +41,8 @@ import org.openhab.core.audio.AudioSink; import org.openhab.core.audio.AudioSource; import org.openhab.core.audio.AudioStream; -import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; -import org.openhab.core.common.Disposable; import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -293,27 +291,22 @@ public void say(String text, @Nullable String voiceId, @Nullable String sinkId, e.getMessage(), e); } } - try { - sink.process(audioStream); - // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance - // to auto dispose: - if (sink.isSynchronous() && audioStream instanceof Disposable disposableAudioStream) { - disposableAudioStream.dispose(); - } - } catch (IOException e) { - String fileName = audioStream instanceof FileAudioStream file ? file.toString() : "unknown"; - logger.warn("Cannot dispose of stream {}", fileName, e); - } finally { - if (volume != null && oldVolume != null) { + + final PercentType oldVolumeFinal = oldVolume; + Runnable toRunWhenProcessFinished = () -> { + if (volume != null && oldVolumeFinal != null) { // restore volume only if it was set before try { - sink.setVolume(oldVolume); + sink.setVolume(oldVolumeFinal); } catch (IOException e) { logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), e.getMessage(), e); } } - } + }; + + sink.process(audioStream, toRunWhenProcessFinished); + } catch (TTSException | UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { if (logger.isDebugEnabled()) { logger.debug("Error saying '{}': {}", text, e.getMessage(), e); diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java index 078f80ea043..619ee0c0923 100644 --- a/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java @@ -26,6 +26,8 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import org.apache.commons.lang3.mutable.Mutable; +import org.apache.commons.lang3.mutable.MutableObject; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.junit.jupiter.api.BeforeEach; @@ -191,10 +193,21 @@ public void loadTwoThreadsAtTheSameTimeFromTheSameSupplierTest() throws IOExcept InputStream actualAudioStream2 = lruMediaCacheEntry.getInputStream(); // read bytes from the two stream concurrently + Mutable<@Nullable IOException> exceptionCatched = new MutableObject<>(); List parallelAudioStreamList = Arrays.asList(actualAudioStream1, actualAudioStream2); - List bytesResultList = parallelAudioStreamList.parallelStream().map(this::readSafe) - .collect(Collectors.toList()); + List bytesResultList = parallelAudioStreamList.parallelStream().map(stream -> { + try { + return stream.readAllBytes(); + } catch (IOException e) { + exceptionCatched.setValue(e); + return new byte[0]; + } + }).collect(Collectors.toList()); + IOException possibleException = exceptionCatched.getValue(); + if (possibleException != null) { + throw possibleException; + } assertArrayEquals(randomData, bytesResultList.get(0)); assertArrayEquals(randomData, bytesResultList.get(1)); @@ -208,14 +221,6 @@ public void loadTwoThreadsAtTheSameTimeFromTheSameSupplierTest() throws IOExcept verifyNoMoreInteractions(ttsServiceMock); } - private byte[] readSafe(InputStream InputStream) { - try { - return InputStream.readAllBytes(); - } catch (IOException e) { - return new byte[0]; - } - } - private byte[] getRandomData(int length) { Random random = new Random(); byte[] randomBytes = new byte[length]; From fcd81bd52d7516b76b7b560276fc868b6fc51198 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Sat, 1 Apr 2023 10:20:41 +0200 Subject: [PATCH 04/11] [audio] More capabilities for AudioSink using the AudioServlet Add disposable capability to FileAudioStream Signed-off-by: Gwendal Roulleau --- .../openhab/core/audio/FileAudioStream.java | 17 +++++++++++++++- .../core/audio/internal/AudioServlet.java | 20 +------------------ 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FileAudioStream.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FileAudioStream.java index 481d62353aa..7247742aaa6 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FileAudioStream.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FileAudioStream.java @@ -18,10 +18,12 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.audio.utils.AudioStreamUtils; import org.openhab.core.audio.utils.AudioWaveUtils; +import org.openhab.core.common.Disposable; /** * This is an AudioStream from an audio file @@ -31,7 +33,7 @@ * @author Christoph Weitkamp - Refactored use of filename extension */ @NonNullByDefault -public class FileAudioStream extends FixedLengthAudioStream { +public class FileAudioStream extends FixedLengthAudioStream implements Disposable { public static final String WAV_EXTENSION = "wav"; public static final String MP3_EXTENSION = "mp3"; @@ -42,16 +44,22 @@ public class FileAudioStream extends FixedLengthAudioStream { private final AudioFormat audioFormat; private InputStream inputStream; private final long length; + private final boolean isTemporaryFile; public FileAudioStream(File file) throws AudioException { this(file, getAudioFormat(file)); } public FileAudioStream(File file, AudioFormat format) throws AudioException { + this(file, format, false); + } + + public FileAudioStream(File file, AudioFormat format, boolean isTemporaryFile) throws AudioException { this.file = file; this.inputStream = getInputStream(file); this.audioFormat = format; this.length = file.length(); + this.isTemporaryFile = isTemporaryFile; } private static AudioFormat getAudioFormat(File file) throws AudioException { @@ -125,4 +133,11 @@ public synchronized void reset() throws IOException { public InputStream getClonedStream() throws AudioException { return getInputStream(file); } + + @Override + public void dispose() throws IOException { + if (isTemporaryFile) { + Files.delete(file.toPath()); + } + } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index bcc4714bfca..ef64c898cd9 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -17,7 +17,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.nio.file.Files; import java.util.Collections; import java.util.List; import java.util.Map; @@ -48,7 +47,6 @@ import org.openhab.core.audio.ClonableAudioStream; import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.FixedLengthAudioStream; -import org.openhab.core.common.Disposable; import org.openhab.core.common.ThreadPoolManager; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; @@ -301,7 +299,7 @@ private ClonableAudioStream createClonableInputStream(AudioStream stream, String fileSize += length; } } - clonableAudioStreamResult = new TemporaryFileAudioStream(tempFile, stream.getFormat()); + clonableAudioStreamResult = new FileAudioStream(tempFile, stream.getFormat(), true); } tryClose(stream); return clonableAudioStreamResult; @@ -315,22 +313,6 @@ private String getRelativeURL(String streamId) { return SERVLET_PATH + "/" + streamId; } - // Using a FileAudioStream implementing Disposable allows file deletion when it is not used anymore - private static class TemporaryFileAudioStream extends FileAudioStream implements Disposable { - - private File file; - - public TemporaryFileAudioStream(File file, AudioFormat format) throws AudioException { - super(file, format); - this.file = file; - } - - @Override - public void dispose() throws IOException { - Files.delete(file.toPath()); - } - } - protected static record StreamServed(String id, AudioStream audioStream, AtomicInteger currentlyServedStream, long timeout, boolean multiTimeStream, Runnable callback) { } From db3d1efe92253ff97e572269f973902b2ae37287 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Sun, 2 Apr 2023 10:30:24 +0200 Subject: [PATCH 05/11] [audio] More capabilities for AudioSink using the AudioServlet Apply code review. Add enhanced generic utility volume change method. Signed-off-by: Gwendal Roulleau --- .../openhab/core/audio/AudioHTTPServer.java | 5 +- .../org/openhab/core/audio/AudioSink.java | 6 +- .../openhab/core/audio/AudioSinkAsync.java | 6 +- .../org/openhab/core/audio/AudioSinkSync.java | 7 +- .../core/audio/internal/AudioManagerImpl.java | 35 +------- .../core/audio/internal/AudioServlet.java | 44 +++++----- .../internal/webaudio/WebAudioAudioSink.java | 6 +- .../core/audio/utils/AudioSinkUtils.java | 83 +++++++++++++++++++ .../core/voice/internal/VoiceManagerImpl.java | 37 +-------- 9 files changed, 134 insertions(+), 95 deletions(-) create mode 100644 bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java index 4b897964f8e..f8379a0ae0d 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java @@ -12,6 +12,8 @@ */ package org.openhab.core.audio; +import java.io.IOException; + import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.audio.internal.AudioServlet; @@ -70,6 +72,7 @@ public interface AudioHTTPServer { * @param a Runnable callback for cleaning resources. The AudioHTTPServer will run the callback when the stream is * not used anymore and timed-out. * @return the relative URL to access the stream starting with a '/' + * @throws IOException when the stream is not a {@link ClonableAudioStream} and we cannot get or store it on disk. */ - String serve(AudioStream stream, int seconds, Runnable callBack); + String serve(AudioStream stream, int seconds, Runnable callBack) throws IOException; } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java index 3ea7d1c0a37..0dd90dd58a8 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java @@ -88,12 +88,14 @@ void process(@Nullable AudioStream audioStream) * @throws UnsupportedAudioFormatException If audioStream format is not supported * @throws UnsupportedAudioStreamException If audioStream is not supported */ - default void process(@Nullable AudioStream audioStream, Runnable whenFinished) + default void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { try { process(audioStream); } finally { - whenFinished.run(); + if (whenFinished != null) { + whenFinished.run(); + } } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java index 04784349be2..aff4ec0a931 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java @@ -40,16 +40,16 @@ public abstract class AudioSinkAsync implements AudioSink { private final Map runnableByAudioStream = new HashMap<>(); @Override - public void process(@Nullable AudioStream audioStream, Runnable whenFinished) + public void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { try { - if (audioStream != null) { + if (audioStream != null && whenFinished != null) { runnableByAudioStream.put(audioStream, whenFinished); } process(audioStream); } finally { - if (audioStream == null) { + if (audioStream == null && whenFinished != null) { // No need to delay the post process task whenFinished.run(); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java index d6a1354cd94..59e278151fe 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java @@ -35,14 +35,15 @@ public abstract class AudioSinkSync implements AudioSink { private final Logger logger = LoggerFactory.getLogger(AudioSinkSync.class); @Override - public void process(@Nullable AudioStream audioStream, Runnable whenFinished) + public void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { try { process(audioStream); } finally { - - whenFinished.run(); + if (whenFinished != null) { + whenFinished.run(); + } // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance // to auto dispose: diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java index 75f966b3fcc..db075890576 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java @@ -40,6 +40,7 @@ import org.openhab.core.audio.URLAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; +import org.openhab.core.audio.utils.AudioSinkUtils; import org.openhab.core.audio.utils.ToneSynthesizer; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -122,39 +123,9 @@ public void play(@Nullable AudioStream audioStream, @Nullable String sinkId) { public void play(@Nullable AudioStream audioStream, @Nullable String sinkId, @Nullable PercentType volume) { AudioSink sink = getSink(sinkId); if (sink != null) { - PercentType oldVolume = null; - // set notification sound volume - if (volume != null) { - try { - // get current volume - oldVolume = sink.getVolume(); - } catch (IOException e) { - logger.debug("An exception occurred while getting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - - try { - sink.setVolume(volume); - } catch (IOException e) { - logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - } - - final PercentType oldVolumeFinal = oldVolume; - Runnable toRunWhenProcessFinished = () -> { - if (volume != null && oldVolumeFinal != null) { - // restore volume only if it was set before - try { - sink.setVolume(oldVolumeFinal); - } catch (IOException e) { - logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - } - }; + Runnable volumeRestoration = AudioSinkUtils.handleVolumeCommand(volume, sink, logger); try { - sink.process(audioStream, toRunWhenProcessFinished); + sink.process(audioStream, volumeRestoration); } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { logger.warn("Error playing '{}': {}", audioStream, e.getMessage(), e); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index ef64c898cd9..bcbaa045003 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -86,9 +86,6 @@ public class AudioServlet extends HttpServlet implements AudioHTTPServer { @Nullable ScheduledFuture periodicCleaner; - private static final Runnable doNothing = () -> { - }; - @Deactivate protected synchronized void deactivate() { servedStreams.values().stream().map(streamServed -> streamServed.audioStream).forEach(this::tryClose); @@ -190,7 +187,10 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se // we can immediately dispose and remove, if it is a one time stream if (!servedStream.multiTimeStream) { - servedStream.callback.run(); + Runnable callback = servedStream.callback; + if (callback != null) { + callback.run(); + } servedStreams.remove(streamId); logger.debug("Removed timed out stream {}", streamId); } @@ -209,7 +209,10 @@ private synchronized void removeTimedOutStreams() { if (streamServed != null) { tryClose(streamServed.audioStream); // we can notify the caller of the stream consumption - streamServed.callback.run(); + Runnable callback = streamServed.callback; + if (callback != null) { + callback.run(); + } logger.debug("Removed timed out stream {}", streamId); } }); @@ -235,7 +238,7 @@ public String serve(AudioStream stream) { // Because we cannot wait indefinitely before executing the callback, // even if this is a one time stream, we set a timeout just in case. long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); - StreamServed streamToServe = new StreamServed(streamId, stream, new AtomicInteger(), timeOut, false, doNothing); + StreamServed streamToServe = new StreamServed(streamId, stream, new AtomicInteger(), timeOut, false, null); servedStreams.put(streamId, streamToServe); // try to clean, or a least launch the periodic cleanse: @@ -246,23 +249,23 @@ public String serve(AudioStream stream) { @Override public String serve(AudioStream stream, int seconds) { - return serve(stream, seconds, doNothing); + try { + return serve(stream, seconds, null); + } catch (IOException e) { + logger.warn("Cannot precache the audio stream to serve it", e); + return getRelativeURL("error"); + } } @Override - public String serve(AudioStream stream, int seconds, Runnable callBack) { + public String serve(AudioStream stream, int seconds, @Nullable Runnable callBack) throws IOException { String streamId = UUID.randomUUID().toString(); ClonableAudioStream clonableAudioStream = null; if (stream instanceof ClonableAudioStream clonableAudioStreamDetected) { clonableAudioStream = clonableAudioStreamDetected; } else { // we prefer a ClonableAudioStream, but we can try to make one - try { - clonableAudioStream = createClonableInputStream(stream, streamId); - } catch (AudioException | IOException e) { - logger.warn("Cannot precache the audio stream to serve it", e); - return getRelativeURL(streamId); - } + clonableAudioStream = createClonableInputStream(stream, streamId); } long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(seconds); StreamServed streamToServe = new StreamServed(streamId, clonableAudioStream, new AtomicInteger(), timeOut, true, @@ -275,8 +278,7 @@ public String serve(AudioStream stream, int seconds, Runnable callBack) { return getRelativeURL(streamId); } - private ClonableAudioStream createClonableInputStream(AudioStream stream, String streamId) - throws IOException, AudioException { + private ClonableAudioStream createClonableInputStream(AudioStream stream, String streamId) throws IOException { byte[] dataBytes = stream.readNBytes(ONETIME_STREAM_BUFFER_MAX_SIZE + 1); ClonableAudioStream clonableAudioStreamResult; if (dataBytes.length <= ONETIME_STREAM_BUFFER_MAX_SIZE) { @@ -293,13 +295,17 @@ private ClonableAudioStream createClonableInputStream(AudioStream stream, String byte[] buf = new byte[8192]; int length; // but with a limit - int fileSize = 0; + int fileSize = ONETIME_STREAM_BUFFER_MAX_SIZE + 1; while ((length = stream.read(buf)) != -1 && fileSize < ONETIME_STREAM_FILE_MAX_SIZE) { outputStream.write(buf, 0, length); fileSize += length; } } - clonableAudioStreamResult = new FileAudioStream(tempFile, stream.getFormat(), true); + try { + clonableAudioStreamResult = new FileAudioStream(tempFile, stream.getFormat(), true); + } catch (AudioException e) { // this is in fact a FileNotFoundException and should not happen + throw new IOException("Cannot found the cache file we just created.", e); + } } tryClose(stream); return clonableAudioStreamResult; @@ -314,6 +320,6 @@ private String getRelativeURL(String streamId) { } protected static record StreamServed(String id, AudioStream audioStream, AtomicInteger currentlyServedStream, - long timeout, boolean multiTimeStream, Runnable callback) { + long timeout, boolean multiTimeStream, @Nullable Runnable callback) { } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java index 857b406a268..270103e0ce1 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java @@ -83,7 +83,11 @@ public void process(@Nullable AudioStream audioStream) // we will let the HTTP servlet run the delayed task when finished with the stream Runnable delayedTask = () -> this.runDelayed(audioStream); // we need to serve it for a while and make it available to multiple clients - sendEvent(audioHTTPServer.serve(audioStream, 10, delayedTask).toString()); + try { + sendEvent(audioHTTPServer.serve(audioStream, 10, delayedTask).toString()); + } catch (IOException e) { + logger.warn("Cannot precache the audio stream to serve it", e); + } } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java new file mode 100644 index 00000000000..1aa904f6376 --- /dev/null +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java @@ -0,0 +1,83 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.audio.utils; + +import java.io.IOException; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.audio.AudioSink; +import org.openhab.core.library.types.PercentType; +import org.slf4j.Logger; + +/** + * Some utility methods for sink + * + * @author Gwendal Roulleau - Initial contribution + * + */ +@NonNullByDefault +public class AudioSinkUtils { + + /** + * Handle a volume command change and returns a Runnable to restore it. + * + * @param volume The volume to set + * @param sink The sink to set the volume to + * @param logger to log error to + * @return A runnable to restore the volume to its previous value, or null if no change is required + */ + public static @Nullable Runnable handleVolumeCommand(@Nullable PercentType volume, AudioSink sink, Logger logger) { + boolean volumeChanged = false; + PercentType oldVolume = null; + + if (volume == null) { + return null; + } + + // set notification sound volume + try { + // get current volume + oldVolume = sink.getVolume(); + } catch (IOException e) { + logger.debug("An exception occurred while getting the volume of sink '{}' : {}", sink.getId(), + e.getMessage(), e); + } + + if (!volume.equals(oldVolume) || oldVolume == null) { + try { + sink.setVolume(volume); + volumeChanged = true; + } catch (IOException e) { + logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), + e.getMessage(), e); + } + } + + final PercentType oldVolumeFinal = oldVolume; + Runnable toRunWhenProcessFinished = null; + // restore volume only if it was set before + if (volumeChanged && oldVolumeFinal != null) { + toRunWhenProcessFinished = () -> { + try { + sink.setVolume(oldVolumeFinal); + } catch (IOException | UnsupportedOperationException e) { + logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), + e.getMessage(), e); + } + }; + } + + return toRunWhenProcessFinished; + } +} diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java index a0697add376..62f00d4d754 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java @@ -12,7 +12,6 @@ */ package org.openhab.core.voice.internal; -import java.io.IOException; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; @@ -43,6 +42,7 @@ import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; +import org.openhab.core.audio.utils.AudioSinkUtils; import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -273,40 +273,9 @@ public void say(String text, @Nullable String voiceId, @Nullable String sinkId, "Failed playing audio stream '" + audioStream + "' as audio sink doesn't support it"); } - PercentType oldVolume = null; - // set notification sound volume - if (volume != null) { - try { - // get current volume - oldVolume = sink.getVolume(); - } catch (IOException e) { - logger.debug("An exception occurred while getting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - - try { - sink.setVolume(volume); - } catch (IOException e) { - logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - } - - final PercentType oldVolumeFinal = oldVolume; - Runnable toRunWhenProcessFinished = () -> { - if (volume != null && oldVolumeFinal != null) { - // restore volume only if it was set before - try { - sink.setVolume(oldVolumeFinal); - } catch (IOException e) { - logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - } - }; - - sink.process(audioStream, toRunWhenProcessFinished); + Runnable volumeRestoration = AudioSinkUtils.handleVolumeCommand(volume, sink, logger); + sink.process(audioStream, volumeRestoration); } catch (TTSException | UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { if (logger.isDebugEnabled()) { logger.debug("Error saying '{}': {}", text, e.getMessage(), e); From f491a541f0567dafb0af9ab1a84e9475e000ffca Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Sun, 2 Apr 2023 13:57:05 +0200 Subject: [PATCH 06/11] [audio] More capabilities for AudioSink using the AudioServlet JavaSoundAudioSink is in fact asynchronous Signed-off-by: Gwendal Roulleau --- .../audio/internal/javasound/JavaSoundAudioSink.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java index bd6f0ac5e5f..e9fca650224 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java @@ -32,7 +32,7 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioSink; -import org.openhab.core.audio.AudioSinkSync; +import org.openhab.core.audio.AudioSinkAsync; import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.URLAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; @@ -56,7 +56,7 @@ */ @NonNullByDefault @Component(service = AudioSink.class, immediate = true) -public class JavaSoundAudioSink extends AudioSinkSync { +public class JavaSoundAudioSink extends AudioSinkAsync { private static final Logger LOGGER = LoggerFactory.getLogger(JavaSoundAudioSink.class); @@ -105,7 +105,7 @@ public synchronized void process(final @Nullable AudioStream audioStream) try { // we start a new continuous stream and store its handle streamPlayer = new Player(audioStream); - playInThread(streamPlayer); + playInThread(streamPlayer, () -> runDelayed(audioStream)); } catch (JavaLayerException e) { LOGGER.error("An exception occurred while playing url audio stream : '{}'", e.getMessage()); } @@ -114,7 +114,7 @@ public synchronized void process(final @Nullable AudioStream audioStream) } else { // we are playing some normal file (no url stream) try { - playInThread(new Player(audioStream)); + playInThread(new Player(audioStream), () -> runDelayed(audioStream)); } catch (JavaLayerException e) { LOGGER.error("An exception occurred while playing audio : '{}'", e.getMessage()); } @@ -122,7 +122,7 @@ public synchronized void process(final @Nullable AudioStream audioStream) } } - private void playInThread(final @Nullable Player player) { + private void playInThread(final @Nullable Player player, Runnable toRunAfter) { // run in new thread threadFactory.newThread(() -> { if (player != null) { @@ -132,6 +132,7 @@ private void playInThread(final @Nullable Player player) { LOGGER.error("An exception occurred while playing audio : '{}'", e.getMessage()); } finally { player.close(); + toRunAfter.run(); } } }).start(); From 5f996af40c036e08fae0d6c9b741a6ff1d500c6e Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Mon, 3 Apr 2023 18:12:52 +0200 Subject: [PATCH 07/11] [audio] More capabilities for AudioSink using the AudioServlet Adding (approximative, better than nothing) sound duration computation method for MP3 and WAV. Use this sound duration computation to guess when the async sound is finished and when to do the post process (i.e. volume restoration) Signed-off-by: Gwendal Roulleau --- .../core/audio/internal/AudioServlet.java | 20 ++++-- .../core/audio/utils/AudioStreamUtils.java | 66 +++++++++++++++++++ .../core/audio/internal/AudioServletTest.java | 3 + 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index bcbaa045003..b4d2676d6c4 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -28,6 +28,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -47,6 +48,7 @@ import org.openhab.core.audio.ClonableAudioStream; import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.FixedLengthAudioStream; +import org.openhab.core.audio.utils.AudioStreamUtils; import org.openhab.core.common.ThreadPoolManager; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; @@ -172,7 +174,12 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se AtomicInteger currentlyServedStream = servedStream.currentlyServedStream; if (currentlyServedStream.incrementAndGet() == 1 || servedStream.multiTimeStream) { try (final InputStream stream = prepareInputStream(servedStream, resp, acceptedMimeTypes)) { - stream.transferTo(resp.getOutputStream()); + Long endOfPlayTimestamp = AudioStreamUtils.transferAndAnalyzeLength(stream, resp.getOutputStream(), + servedStream.audioStream.getFormat()); + // update timeout with the sound duration : + if (endOfPlayTimestamp != null) { + servedStream.timeout.set(Math.max(servedStream.timeout.get(), endOfPlayTimestamp)); + } resp.flushBuffer(); } catch (final AudioException ex) { resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex.getMessage()); @@ -200,7 +207,7 @@ private synchronized void removeTimedOutStreams() { // Build list of expired streams. long now = System.nanoTime(); final List toRemove = servedStreams.entrySet().stream() - .filter(e -> e.getValue().timeout < now && e.getValue().currentlyServedStream.get() <= 0) + .filter(e -> e.getValue().timeout.get() < now && e.getValue().currentlyServedStream.get() <= 0) .map(Entry::getKey).collect(Collectors.toList()); toRemove.forEach(streamId -> { @@ -238,7 +245,8 @@ public String serve(AudioStream stream) { // Because we cannot wait indefinitely before executing the callback, // even if this is a one time stream, we set a timeout just in case. long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); - StreamServed streamToServe = new StreamServed(streamId, stream, new AtomicInteger(), timeOut, false, null); + StreamServed streamToServe = new StreamServed(streamId, stream, new AtomicInteger(), new AtomicLong(timeOut), + false, null); servedStreams.put(streamId, streamToServe); // try to clean, or a least launch the periodic cleanse: @@ -268,8 +276,8 @@ public String serve(AudioStream stream, int seconds, @Nullable Runnable callBack clonableAudioStream = createClonableInputStream(stream, streamId); } long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(seconds); - StreamServed streamToServe = new StreamServed(streamId, clonableAudioStream, new AtomicInteger(), timeOut, true, - callBack); + StreamServed streamToServe = new StreamServed(streamId, clonableAudioStream, new AtomicInteger(), + new AtomicLong(timeOut), true, callBack); servedStreams.put(streamId, streamToServe); // try to clean, or a least launch the periodic cleanse: @@ -320,6 +328,6 @@ private String getRelativeURL(String streamId) { } protected static record StreamServed(String id, AudioStream audioStream, AtomicInteger currentlyServedStream, - long timeout, boolean multiTimeStream, @Nullable Runnable callback) { + AtomicLong timeout, boolean multiTimeStream, @Nullable Runnable callback) { } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java index 04f6a80fe36..be7402cebc7 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java @@ -12,7 +12,21 @@ */ package org.openhab.core.audio.utils; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import javazoom.jl.decoder.Bitstream; +import javazoom.jl.decoder.BitstreamException; +import javazoom.jl.decoder.Header; + +import javax.sound.sampled.AudioInputStream; +import javax.sound.sampled.AudioSystem; +import javax.sound.sampled.UnsupportedAudioFileException; + import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.audio.AudioFormat; /** * Some general filename and extension utilities. @@ -66,4 +80,56 @@ public static String getExtension(String filename) { public static boolean isExtension(String filename, String extension) { return !extension.isEmpty() && getExtension(filename).equals(extension); } + + /** + * Transfers data from an input stream to an output stream and computes on the fly its duration + * + * @param in the input stream giving audio data ta play + * @param out the output stream receiving data to play + * @return the timestamp (from System.nanoTime) when the sound should be fully played. Returns null if computing + * time fails. + * @throws IOException if reading from the stream or writing to the stream failed + */ + public static @Nullable Long transferAndAnalyzeLength(InputStream in, OutputStream out, AudioFormat audioFormat) + throws IOException { + // take some data from the stream beginning + byte[] dataBytes = in.readNBytes(8192); + + // beginning sound timestamp : + long startTime = System.nanoTime(); + // copy already read data to the output stream : + out.write(dataBytes); + // transfer everything else + Long dataTransferedLength = dataBytes.length + in.transferTo(out); + + if (dataTransferedLength > 0) { + if (AudioFormat.CODEC_PCM_SIGNED.equals(audioFormat.getCodec())) { + try (AudioInputStream audioInputStream = AudioSystem + .getAudioInputStream(new ByteArrayInputStream(dataBytes))) { + int frameSize = audioInputStream.getFormat().getFrameSize(); + float frameRate = audioInputStream.getFormat().getFrameRate(); + long computedDuration = Float.valueOf((dataTransferedLength / (frameSize * frameRate)) * 1000000000) + .longValue(); + return startTime + computedDuration; + } catch (IOException | UnsupportedAudioFileException e) { + return null; + } + } else if (AudioFormat.CODEC_MP3.equals(audioFormat.getCodec())) { + // not precise, no VBR, but better than nothing + Bitstream bitstream = new Bitstream(new ByteArrayInputStream(dataBytes)); + try { + Header h = bitstream.readFrame(); + if (h != null) { + long computedDuration = Float.valueOf(h.total_ms(dataTransferedLength.intValue()) * 1000000) + .longValue(); + return startTime + computedDuration; + } + } catch (BitstreamException ex) { + return null; + } + } + } + + return null; + } } diff --git a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java index 8740a358941..30430242b04 100644 --- a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java +++ b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java @@ -198,6 +198,7 @@ public void oneTimeStreamIsClosedAndRemovedAfterServed() throws Exception { AudioFormat audioFormat = mock(AudioFormat.class); when(audioStream.getFormat()).thenReturn(audioFormat); when(audioFormat.getCodec()).thenReturn(AudioFormat.CODEC_MP3); + when(audioStream.readNBytes(anyInt())).thenReturn(new byte[] { 1, 2, 3 }); String url = serveStream(audioStream); assertThat(audioServlet.getServedStreams().values().stream().map(StreamServed::audioStream).toList(), @@ -221,6 +222,8 @@ public void multiTimeStreamIsClosedAfterExpired() throws Exception { cloneCounter.getAndIncrement(); return clonedStream; }); + when(audioStream.readNBytes(anyInt())).thenReturn(new byte[] { 1, 2, 3 }); + when(clonedStream.readNBytes(anyInt())).thenReturn(new byte[] { 1, 2, 3 }); when(audioFormat.getCodec()).thenReturn(AudioFormat.CODEC_MP3); String url = serveStream(audioStream, 2); From 952efc5bce6876a92207251960e8593a74a59f21 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Sat, 8 Apr 2023 15:50:57 +0200 Subject: [PATCH 08/11] [audio] More capabilities for AudioSink using the AudioServlet Apply code review Signed-off-by: Gwendal Roulleau --- .../java/org/openhab/core/audio/internal/AudioServlet.java | 7 ++++--- .../java/org/openhab/core/audio/utils/AudioSinkUtils.java | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index b4d2676d6c4..371186de669 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -305,14 +305,15 @@ private ClonableAudioStream createClonableInputStream(AudioStream stream, String // but with a limit int fileSize = ONETIME_STREAM_BUFFER_MAX_SIZE + 1; while ((length = stream.read(buf)) != -1 && fileSize < ONETIME_STREAM_FILE_MAX_SIZE) { - outputStream.write(buf, 0, length); - fileSize += length; + int lengthToWrite = Math.min(length, ONETIME_STREAM_FILE_MAX_SIZE - fileSize); + outputStream.write(buf, 0, lengthToWrite); + fileSize += lengthToWrite; } } try { clonableAudioStreamResult = new FileAudioStream(tempFile, stream.getFormat(), true); } catch (AudioException e) { // this is in fact a FileNotFoundException and should not happen - throw new IOException("Cannot found the cache file we just created.", e); + throw new IOException("Cannot find the cache file we just created.", e); } } tryClose(stream); diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java index 1aa904f6376..84f7f21d300 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java @@ -49,7 +49,7 @@ public class AudioSinkUtils { try { // get current volume oldVolume = sink.getVolume(); - } catch (IOException e) { + } catch (IOException | UnsupportedOperationException e) { logger.debug("An exception occurred while getting the volume of sink '{}' : {}", sink.getId(), e.getMessage(), e); } @@ -58,7 +58,7 @@ public class AudioSinkUtils { try { sink.setVolume(volume); volumeChanged = true; - } catch (IOException e) { + } catch (IOException | UnsupportedOperationException e) { logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), e.getMessage(), e); } From 041f4663fdf67cc1bb8a3b31355eb1706e99dbc7 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Sun, 14 May 2023 11:59:56 +0200 Subject: [PATCH 09/11] [audio] More capabilities for AudioSink using the AudioServlet Apply SAT review Signed-off-by: Gwendal Roulleau --- .../src/main/java/org/openhab/core/audio/AudioSinkAsync.java | 1 - .../src/main/java/org/openhab/core/audio/AudioSinkSync.java | 1 - .../java/org/openhab/core/audio/internal/AudioServlet.java | 3 --- 3 files changed, 5 deletions(-) diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java index aff4ec0a931..2f00f19f357 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java @@ -42,7 +42,6 @@ public abstract class AudioSinkAsync implements AudioSink { @Override public void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { - try { if (audioStream != null && whenFinished != null) { runnableByAudioStream.put(audioStream, whenFinished); diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java index 59e278151fe..cc6105bd0ca 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java @@ -37,7 +37,6 @@ public abstract class AudioSinkSync implements AudioSink { @Override public void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { - try { process(audioStream); } finally { diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index 371186de669..0452f2f9b04 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -105,7 +105,6 @@ private void tryClose(@Nullable AudioStream stream) { private InputStream prepareInputStream(final StreamServed servedStream, final HttpServletResponse resp, List acceptedMimeTypes) throws AudioException { - logger.debug("Stream to serve is {}", servedStream.id); // try to set the content-type, if possible @@ -151,7 +150,6 @@ private String substringBefore(String str, String separator) { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { - String requestURI = req.getRequestURI(); if (requestURI == null) { resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "requestURI is null"); @@ -267,7 +265,6 @@ public String serve(AudioStream stream, int seconds) { @Override public String serve(AudioStream stream, int seconds, @Nullable Runnable callBack) throws IOException { - String streamId = UUID.randomUUID().toString(); ClonableAudioStream clonableAudioStream = null; if (stream instanceof ClonableAudioStream clonableAudioStreamDetected) { From b8a0a5bb45907494fd29c51ceb164e32b94e1e6f Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Sat, 27 May 2023 09:07:38 +0200 Subject: [PATCH 10/11] [audio] More capabilities for AudioSink using the AudioServlet Applying code review advices (CompletableFuture and others) Signed-off-by: Gwendal Roulleau --- .../openhab/core/audio/AudioHTTPServer.java | 44 ++++---- .../org/openhab/core/audio/AudioManager.java | 11 ++ .../org/openhab/core/audio/AudioSink.java | 27 ++--- .../openhab/core/audio/AudioSinkAsync.java | 65 ++++++++--- .../org/openhab/core/audio/AudioSinkSync.java | 41 +++++-- .../org/openhab/core/audio/StreamServed.java | 30 +++++ .../core/audio/internal/AudioManagerImpl.java | 54 ++++++++- .../core/audio/internal/AudioServlet.java | 104 +++++++++--------- .../javasound/JavaSoundAudioSink.java | 29 ++--- .../internal/webaudio/WebAudioAudioSink.java | 10 +- .../core/audio/utils/AudioSinkUtils.java | 64 ++--------- .../core/audio/utils/AudioSinkUtilsImpl.java | 91 +++++++++++++++ .../core/audio/utils/AudioStreamUtils.java | 66 ----------- .../internal/AbstractAudioServletTest.java | 5 +- .../core/audio/internal/AudioServletTest.java | 2 +- .../core/voice/internal/VoiceManagerImpl.java | 11 +- 16 files changed, 393 insertions(+), 261 deletions(-) create mode 100644 bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/StreamServed.java create mode 100644 bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtilsImpl.java diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java index f8379a0ae0d..2cb85ee5833 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioHTTPServer.java @@ -13,6 +13,7 @@ package org.openhab.core.audio; import java.io.IOException; +import java.util.concurrent.CompletableFuture; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.audio.internal.AudioServlet; @@ -36,43 +37,48 @@ public interface AudioHTTPServer { * * @param stream the stream to serve on HTTP * @return the relative URL to access the stream starting with a '/' + * @deprecated Use {@link AudioHTTPServer#serve(AudioStream, int, boolean, CompletableFuture)} */ + @Deprecated String serve(AudioStream stream); /** * Creates a relative url for a given {@link AudioStream} where it can be requested multiple times within the given * time frame. - * This method accepts all {@link AudioStream}s, but it is better to use {@link ClonableAudioStream}s since it - * needs to be able to create multiple concurrent streams from it. - * If generic {@link AudioStream} is used, the method tries to add the Clonable capability by storing it in a small - * memory buffer, - * e.g {@link ByteArrayAudioStream}, or in a cached file if the stream reached the buffer capacity, or fails if the - * stream is too long. + * This method accepts all {@link AudioStream}s, but it is better to use {@link ClonableAudioStream}s. If generic + * {@link AudioStream} is used, the method tries to add the Clonable capability by storing it in a small memory + * buffer, e.g {@link ByteArrayAudioStream}, or in a cached file if the stream reached the buffer capacity, + * or fails if the stream is too long. * Streams are closed, once they expire. * * @param stream the stream to serve on HTTP * @param seconds number of seconds for which the stream is available through HTTP * @return the relative URL to access the stream starting with a '/' + * @deprecated Use {@link AudioHTTPServer#serve(AudioStream, int, boolean, CompletableFuture)} */ + @Deprecated String serve(AudioStream stream, int seconds); /** - * Creates a relative url for a given {@link AudioStream} where it can be requested multiple times within the given - * time frame. - * This method accepts all {@link AudioStream}s, but it is better to use {@link ClonableAudioStream}s since it - * needs to be able to create multiple concurrent streams from it. - * If generic {@link AudioStream} is used, method tries to add the Clonable capability by storing it in a small - * memory buffer, - * e.g {@link ByteArrayAudioStream}, or in a cached file if the stream reached the buffer capacity, or fails if the - * stream is too long. + * Creates a relative url for a given {@link AudioStream} where it can be requested one or multiple times within the + * given time frame. + * This method accepts all {@link AudioStream}s, but if multiTimeStream is set to true it is better to use + * {@link ClonableAudioStream}s. Otherwise, if a generic {@link AudioStream} is used, the method will then try + * to add the Clonable capability by storing it in a small memory buffer, e.g {@link ByteArrayAudioStream}, or in a + * cached file if the stream reached the buffer capacity, or fails to render the sound completely if the stream is + * too long. + * A {@link CompletableFuture} is used to inform the caller that the playback ends in order to clean + * resources and run delayed task, such as restoring volume. * Streams are closed, once they expire. * * @param stream the stream to serve on HTTP - * @param seconds number of seconds for which the stream is available through HTTP - * @param a Runnable callback for cleaning resources. The AudioHTTPServer will run the callback when the stream is - * not used anymore and timed-out. - * @return the relative URL to access the stream starting with a '/' + * @param seconds number of seconds for which the stream is available through HTTP. The stream will be deleted only + * if not started, so you can set a duration shorter than the track's duration. + * @param multiTimeStream set to true if this stream should be played multiple time, and thus needs to be made + * Cloneable if it is not already. + * @return information about the {@link StreamServed}, including the relative URL to access the stream starting with + * a '/', and a CompletableFuture to know when the playback ends. * @throws IOException when the stream is not a {@link ClonableAudioStream} and we cannot get or store it on disk. */ - String serve(AudioStream stream, int seconds, Runnable callBack) throws IOException; + StreamServed serve(AudioStream stream, int seconds, boolean multiTimeStream) throws IOException; } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioManager.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioManager.java index a60e5fe4172..d640c0f3996 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioManager.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioManager.java @@ -252,4 +252,15 @@ public interface AudioManager { * @return ids of matching sinks */ Set getSinkIds(String pattern); + + /** + * Handles a volume command change and returns a Runnable to restore it. + * Returning a Runnable allows us to have a no-op Runnable if changing volume back is not needed, and conveniently + * keeping it as one liner usable in a chain for the caller. + * + * @param volume The volume to set + * @param sink The sink to set the volume to + * @return A runnable to restore the volume to its previous value, or no-operation if no change is required. + */ + Runnable handleVolumeCommand(@Nullable PercentType volume, AudioSink sink); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java index 0dd90dd58a8..d3f40e601f8 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java @@ -15,6 +15,7 @@ import java.io.IOException; import java.util.Locale; import java.util.Set; +import java.util.concurrent.CompletableFuture; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -58,19 +59,21 @@ public interface AudioSink { * * In case the audioStream is null, this should be interpreted as a request to end any currently playing stream. * - * When the process method ends, if the stream implements the {@link org.openhab.core.common.Disposable} interface, - * the sink should hereafter get rid of it by calling the dispose method. + * When the stream is not needed anymore, if the stream implements the {@link org.openhab.core.common.Disposable} + * interface, the sink should hereafter get rid of it by calling the dispose method. * * @param audioStream the audio stream to play or null to keep quiet * @throws UnsupportedAudioFormatException If audioStream format is not supported * @throws UnsupportedAudioStreamException If audioStream is not supported + * @deprecated Use {@link AudioSink#processAndComplete(AudioStream)} */ + @Deprecated void process(@Nullable AudioStream audioStream) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException; /** - * Processes the passed {@link AudioStream}, and executes the whenFinished Runnable (or stores for later execution - * if the sink is asynchronous). + * Processes the passed {@link AudioStream}, and returns a CompletableFuture that should complete when the sound is + * fully played. It is the sink responsibility to complete this future. * * If the passed {@link AudioStream} is not supported by this instance, an {@link UnsupportedAudioStreamException} * is thrown. @@ -80,23 +83,17 @@ void process(@Nullable AudioStream audioStream) * * In case the audioStream is null, this should be interpreted as a request to end any currently playing stream. * - * When the process method ends, if the stream implements the {@link org.openhab.core.common.Disposable} interface, - * the sink should hereafter get rid of it by calling the dispose method. + * When the stream is not needed anymore, if the stream implements the {@link org.openhab.core.common.Disposable} + * interface, the sink should hereafter get rid of it by calling the dispose method. * * @param audioStream the audio stream to play or null to keep quiet - * @param whenFinished A Runnable to run when the sound is finished playing. * @throws UnsupportedAudioFormatException If audioStream format is not supported * @throws UnsupportedAudioStreamException If audioStream is not supported */ - default void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) + default CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { - try { - process(audioStream); - } finally { - if (whenFinished != null) { - whenFinished.run(); - } - } + process(audioStream); + return CompletableFuture.completedFuture(null); } /** diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java index 2f00f19f357..cbc2962abe6 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java @@ -15,6 +15,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.CompletableFuture; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -25,10 +26,10 @@ /** * Definition of an audio output like headphones, a speaker or for writing to * a file / clip. - * This version is asynchronous: when the process() method returns, the {@link AudioStream} - * may or may not be played, and we don't know when the delayed task will be executed. - * CAUTION : It is the responsibility of the implementing AudioSink class to call the runDelayedTask - * method when playing is done. + * Helper class for asynchronous sink : when the process() method returns, the {@link AudioStream} + * may or may not be played. It is the responsibility of the implementing AudioSink class to + * complete the CompletableFuture when playing is done. Any delayed tasks will then be performed, such as volume + * restoration. * * @author Gwendal Roulleau - Initial contribution */ @@ -37,38 +38,66 @@ public abstract class AudioSinkAsync implements AudioSink { private final Logger logger = LoggerFactory.getLogger(AudioSinkAsync.class); - private final Map runnableByAudioStream = new HashMap<>(); + private final Map> runnableByAudioStream = new HashMap<>(); @Override - public void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) + public CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + CompletableFuture<@Nullable Void> completableFuture = new CompletableFuture<@Nullable Void>(); try { - if (audioStream != null && whenFinished != null) { - runnableByAudioStream.put(audioStream, whenFinished); + if (audioStream != null) { + runnableByAudioStream.put(audioStream, completableFuture); } - process(audioStream); + processAsynchronously(audioStream); + return completableFuture; } finally { - if (audioStream == null && whenFinished != null) { + if (audioStream == null) { // No need to delay the post process task - whenFinished.run(); + runnableByAudioStream.remove(audioStream); + completableFuture.complete(null); } } } + @Override + public void process(@Nullable AudioStream audioStream) + throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + processAsynchronously(audioStream); + } + /** - * Will run the delayed task stored previously. + * Processes the passed {@link AudioStream} asynchronously. This method is expected to return before the stream is + * fully played. This is the sink responsibility to call the {@link AudioSinkAsync#playbackFinished(AudioStream)} + * when it is. + * + * If the passed {@link AudioStream} is not supported by this instance, an {@link UnsupportedAudioStreamException} + * is thrown. * - * @param audioStream The AudioStream is the key to find the delayed Runnable task in the storage. + * If the passed {@link AudioStream} has an {@link AudioFormat} not supported by this instance, + * an {@link UnsupportedAudioFormatException} is thrown. + * + * In case the audioStream is null, this should be interpreted as a request to end any currently playing stream. + * + * @param audioStream the audio stream to play or null to keep quiet + * @throws UnsupportedAudioFormatException If audioStream format is not supported + * @throws UnsupportedAudioStreamException If audioStream is not supported */ - protected void runDelayed(AudioStream audioStream) { - Runnable delayedTask = runnableByAudioStream.remove(audioStream); + protected abstract void processAsynchronously(@Nullable AudioStream audioStream) + throws UnsupportedAudioFormatException, UnsupportedAudioStreamException; - if (delayedTask != null) { - delayedTask.run(); + /** + * Will complete the future previously returned, allowing the core to run delayed task. + * + * @param audioStream The AudioStream is the key to find the delayed CompletableFuture in the storage. + */ + protected void playbackFinished(AudioStream audioStream) { + CompletableFuture<@Nullable Void> completableFuture = runnableByAudioStream.remove(audioStream); + if (completableFuture != null) { + completableFuture.complete(null); } // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance - // to auto dispose: + // to auto dispose. if (audioStream instanceof Disposable disposableAudioStream) { try { disposableAudioStream.dispose(); diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java index cc6105bd0ca..e0318385083 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java @@ -13,6 +13,7 @@ package org.openhab.core.audio; import java.io.IOException; +import java.util.concurrent.CompletableFuture; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -23,9 +24,9 @@ /** * Definition of an audio output like headphones, a speaker or for writing to * a file / clip. - * This version is synchronous: when the process() method returns, + * Helper class for synchronous sink : when the process() method returns, * the source is considered played, and could be disposed. - * Any delayed tasks can then be performed, such as volume restoration + * Any delayed tasks can then be performed, such as volume restoration. * * @author Gwendal Roulleau - Initial contribution */ @@ -35,17 +36,12 @@ public abstract class AudioSinkSync implements AudioSink { private final Logger logger = LoggerFactory.getLogger(AudioSinkSync.class); @Override - public void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFinished) + public CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { try { - process(audioStream); + processSynchronously(audioStream); } finally { - if (whenFinished != null) { - whenFinished.run(); - } - - // if the stream is not needed anymore, then we should call back the AudioStream to let it a chance - // to auto dispose: + // as the stream is not needed anymore, we should dispose of it if (audioStream instanceof Disposable disposableAudioStream) { try { disposableAudioStream.dispose(); @@ -59,5 +55,30 @@ public void process(@Nullable AudioStream audioStream, @Nullable Runnable whenFi } } } + return CompletableFuture.completedFuture(null); } + + @Override + public void process(@Nullable AudioStream audioStream) + throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + processSynchronously(audioStream); + } + + /** + * Processes the passed {@link AudioStream} and returns only when the playback is ended. + * + * If the passed {@link AudioStream} is not supported by this instance, an {@link UnsupportedAudioStreamException} + * is thrown. + * + * If the passed {@link AudioStream} has an {@link AudioFormat} not supported by this instance, + * an {@link UnsupportedAudioFormatException} is thrown. + * + * In case the audioStream is null, this should be interpreted as a request to end any currently playing stream. + * + * @param audioStream the audio stream to play or null to keep quiet + * @throws UnsupportedAudioFormatException If audioStream format is not supported + * @throws UnsupportedAudioStreamException If audioStream is not supported + */ + protected abstract void processSynchronously(@Nullable AudioStream audioStream) + throws UnsupportedAudioFormatException, UnsupportedAudioStreamException; } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/StreamServed.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/StreamServed.java new file mode 100644 index 00000000000..35bf5946957 --- /dev/null +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/StreamServed.java @@ -0,0 +1,30 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.audio; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; + +/** + * Streams served by the AudioHTTPServer. + * + * @author Gwendal Roulleau - Initial contribution + */ +@NonNullByDefault +public record StreamServed(String url, AudioStream audioStream, AtomicInteger currentlyServedStream, AtomicLong timeout, + boolean multiTimeStream, CompletableFuture<@Nullable Void> playEnd) { +} diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java index db075890576..066d1edbf1f 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java @@ -40,7 +40,6 @@ import org.openhab.core.audio.URLAudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; -import org.openhab.core.audio.utils.AudioSinkUtils; import org.openhab.core.audio.utils.ToneSynthesizer; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -123,10 +122,12 @@ public void play(@Nullable AudioStream audioStream, @Nullable String sinkId) { public void play(@Nullable AudioStream audioStream, @Nullable String sinkId, @Nullable PercentType volume) { AudioSink sink = getSink(sinkId); if (sink != null) { - Runnable volumeRestoration = AudioSinkUtils.handleVolumeCommand(volume, sink, logger); + Runnable volumeRestauration = null; try { - sink.process(audioStream, volumeRestoration); + volumeRestauration = handleVolumeCommand(volume, sink); + sink.processAndComplete(audioStream).thenRun(volumeRestauration); } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { + volumeRestauration.run(); logger.warn("Error playing '{}': {}", audioStream, e.getMessage(), e); } } else { @@ -325,6 +326,53 @@ public Set getSinkIds(String pattern) { return null; } + @Override + public Runnable handleVolumeCommand(@Nullable PercentType volume, AudioSink sink) { + boolean volumeChanged = false; + PercentType oldVolume = null; + + Runnable toRunWhenProcessFinished = () -> { + }; + + if (volume == null) { + return toRunWhenProcessFinished; + } + + // set notification sound volume + try { + // get current volume + oldVolume = sink.getVolume(); + } catch (IOException | UnsupportedOperationException e) { + logger.debug("An exception occurred while getting the volume of sink '{}' : {}", sink.getId(), + e.getMessage(), e); + } + + if (!volume.equals(oldVolume) || oldVolume == null) { + try { + sink.setVolume(volume); + volumeChanged = true; + } catch (IOException | UnsupportedOperationException e) { + logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), + e.getMessage(), e); + } + } + + final PercentType oldVolumeFinal = oldVolume; + // restore volume only if it was set before + if (volumeChanged && oldVolumeFinal != null) { + toRunWhenProcessFinished = () -> { + try { + sink.setVolume(oldVolumeFinal); + } catch (IOException | UnsupportedOperationException e) { + logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), + e.getMessage(), e); + } + }; + } + + return toRunWhenProcessFinished; + } + @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC) protected void addAudioSource(AudioSource audioSource) { this.audioSources.put(audioSource.getId(), audioSource); diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java index 0452f2f9b04..e7acbb867ee 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java @@ -23,6 +23,7 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -48,10 +49,13 @@ import org.openhab.core.audio.ClonableAudioStream; import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.FixedLengthAudioStream; -import org.openhab.core.audio.utils.AudioStreamUtils; +import org.openhab.core.audio.StreamServed; +import org.openhab.core.audio.utils.AudioSinkUtils; import org.openhab.core.common.ThreadPoolManager; +import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; +import org.osgi.service.component.annotations.Reference; import org.osgi.service.http.whiteboard.propertytypes.HttpWhiteboardServletName; import org.osgi.service.http.whiteboard.propertytypes.HttpWhiteboardServletPattern; import org.slf4j.Logger; @@ -88,9 +92,17 @@ public class AudioServlet extends HttpServlet implements AudioHTTPServer { @Nullable ScheduledFuture periodicCleaner; + private AudioSinkUtils audioSinkUtils; + + @Activate + public AudioServlet(@Reference AudioSinkUtils audioSinkUtils) { + super(); + this.audioSinkUtils = audioSinkUtils; + } + @Deactivate protected synchronized void deactivate() { - servedStreams.values().stream().map(streamServed -> streamServed.audioStream).forEach(this::tryClose); + servedStreams.values().stream().map(streamServed -> streamServed.audioStream()).forEach(this::tryClose); servedStreams.clear(); } @@ -103,17 +115,17 @@ private void tryClose(@Nullable AudioStream stream) { } } - private InputStream prepareInputStream(final StreamServed servedStream, final HttpServletResponse resp, + private InputStream prepareInputStream(final StreamServed streamServed, final HttpServletResponse resp, List acceptedMimeTypes) throws AudioException { - logger.debug("Stream to serve is {}", servedStream.id); + logger.debug("Stream to serve is {}", streamServed.url()); // try to set the content-type, if possible final String mimeType; - if (AudioFormat.CODEC_MP3.equals(servedStream.audioStream.getFormat().getCodec())) { + if (AudioFormat.CODEC_MP3.equals(streamServed.audioStream().getFormat().getCodec())) { mimeType = "audio/mpeg"; - } else if (AudioFormat.CONTAINER_WAVE.equals(servedStream.audioStream.getFormat().getContainer())) { + } else if (AudioFormat.CONTAINER_WAVE.equals(streamServed.audioStream().getFormat().getContainer())) { mimeType = WAV_MIME_TYPES.stream().filter(acceptedMimeTypes::contains).findFirst().orElse("audio/wav"); - } else if (AudioFormat.CONTAINER_OGG.equals(servedStream.audioStream.getFormat().getContainer())) { + } else if (AudioFormat.CONTAINER_OGG.equals(streamServed.audioStream().getFormat().getContainer())) { mimeType = "audio/ogg"; } else { mimeType = null; @@ -123,17 +135,17 @@ private InputStream prepareInputStream(final StreamServed servedStream, final Ht } // try to set the content-length, if possible - if (servedStream.audioStream instanceof FixedLengthAudioStream fixedLengthServedStream) { + if (streamServed.audioStream() instanceof FixedLengthAudioStream fixedLengthServedStream) { final long size = fixedLengthServedStream.length(); resp.setContentLength((int) size); } - if (servedStream.multiTimeStream - && servedStream.audioStream instanceof ClonableAudioStream clonableAudioStream) { + if (streamServed.multiTimeStream() + && streamServed.audioStream() instanceof ClonableAudioStream clonableAudioStream) { // we need to care about concurrent access and have a separate stream for each thread return clonableAudioStream.getClonedStream(); } else { - return servedStream.audioStream; + return streamServed.audioStream(); } } @@ -169,14 +181,14 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se } // we count the number of active process using the input stream - AtomicInteger currentlyServedStream = servedStream.currentlyServedStream; - if (currentlyServedStream.incrementAndGet() == 1 || servedStream.multiTimeStream) { + AtomicInteger currentlyServedStream = servedStream.currentlyServedStream(); + if (currentlyServedStream.incrementAndGet() == 1 || servedStream.multiTimeStream()) { try (final InputStream stream = prepareInputStream(servedStream, resp, acceptedMimeTypes)) { - Long endOfPlayTimestamp = AudioStreamUtils.transferAndAnalyzeLength(stream, resp.getOutputStream(), - servedStream.audioStream.getFormat()); + Long endOfPlayTimestamp = audioSinkUtils.transferAndAnalyzeLength(stream, resp.getOutputStream(), + servedStream.audioStream().getFormat()); // update timeout with the sound duration : if (endOfPlayTimestamp != null) { - servedStream.timeout.set(Math.max(servedStream.timeout.get(), endOfPlayTimestamp)); + servedStream.timeout().set(Math.max(servedStream.timeout().get(), endOfPlayTimestamp)); } resp.flushBuffer(); } catch (final AudioException ex) { @@ -191,12 +203,9 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se } // we can immediately dispose and remove, if it is a one time stream - if (!servedStream.multiTimeStream) { - Runnable callback = servedStream.callback; - if (callback != null) { - callback.run(); - } + if (!servedStream.multiTimeStream()) { servedStreams.remove(streamId); + servedStream.playEnd().complete(null); logger.debug("Removed timed out stream {}", streamId); } } @@ -205,19 +214,16 @@ private synchronized void removeTimedOutStreams() { // Build list of expired streams. long now = System.nanoTime(); final List toRemove = servedStreams.entrySet().stream() - .filter(e -> e.getValue().timeout.get() < now && e.getValue().currentlyServedStream.get() <= 0) + .filter(e -> e.getValue().timeout().get() < now && e.getValue().currentlyServedStream().get() <= 0) .map(Entry::getKey).collect(Collectors.toList()); toRemove.forEach(streamId -> { // the stream has expired and no one is using it, we need to remove it! StreamServed streamServed = servedStreams.remove(streamId); if (streamServed != null) { - tryClose(streamServed.audioStream); + tryClose(streamServed.audioStream()); // we can notify the caller of the stream consumption - Runnable callback = streamServed.callback; - if (callback != null) { - callback.run(); - } + streamServed.playEnd().complete(null); logger.debug("Removed timed out stream {}", streamId); } }); @@ -239,24 +245,20 @@ private synchronized void removeTimedOutStreams() { @Override public String serve(AudioStream stream) { - String streamId = UUID.randomUUID().toString(); - // Because we cannot wait indefinitely before executing the callback, - // even if this is a one time stream, we set a timeout just in case. - long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); - StreamServed streamToServe = new StreamServed(streamId, stream, new AtomicInteger(), new AtomicLong(timeOut), - false, null); - servedStreams.put(streamId, streamToServe); - - // try to clean, or a least launch the periodic cleanse: - removeTimedOutStreams(); - - return getRelativeURL(streamId); + try { + // In case the stream is never played, we cannot wait indefinitely before executing the callback. + // so we set a timeout (even if this is a one time stream). + return serve(stream, 10, false).url(); + } catch (IOException e) { + logger.warn("Cannot precache the audio stream to serve it", e); + return getRelativeURL("error"); + } } @Override public String serve(AudioStream stream, int seconds) { try { - return serve(stream, seconds, null); + return serve(stream, seconds, true).url(); } catch (IOException e) { logger.warn("Cannot precache the audio stream to serve it", e); return getRelativeURL("error"); @@ -264,23 +266,23 @@ public String serve(AudioStream stream, int seconds) { } @Override - public String serve(AudioStream stream, int seconds, @Nullable Runnable callBack) throws IOException { + public StreamServed serve(AudioStream originalStream, int seconds, boolean multiTimeStream) throws IOException { String streamId = UUID.randomUUID().toString(); - ClonableAudioStream clonableAudioStream = null; - if (stream instanceof ClonableAudioStream clonableAudioStreamDetected) { - clonableAudioStream = clonableAudioStreamDetected; - } else { // we prefer a ClonableAudioStream, but we can try to make one - clonableAudioStream = createClonableInputStream(stream, streamId); + AudioStream audioStream = originalStream; + if (!(originalStream instanceof ClonableAudioStream) && multiTimeStream) { + // we we can try to make a Cloneable stream as it is needed + audioStream = createClonableInputStream(originalStream, streamId); } long timeOut = System.nanoTime() + TimeUnit.SECONDS.toNanos(seconds); - StreamServed streamToServe = new StreamServed(streamId, clonableAudioStream, new AtomicInteger(), - new AtomicLong(timeOut), true, callBack); + CompletableFuture<@Nullable Void> playEnd = new CompletableFuture<@Nullable Void>(); + StreamServed streamToServe = new StreamServed(getRelativeURL(streamId), audioStream, new AtomicInteger(), + new AtomicLong(timeOut), multiTimeStream, playEnd); servedStreams.put(streamId, streamToServe); // try to clean, or a least launch the periodic cleanse: removeTimedOutStreams(); - return getRelativeURL(streamId); + return streamToServe; } private ClonableAudioStream createClonableInputStream(AudioStream stream, String streamId) throws IOException { @@ -324,8 +326,4 @@ Map getServedStreams() { private String getRelativeURL(String streamId) { return SERVLET_PATH + "/" + streamId; } - - protected static record StreamServed(String id, AudioStream audioStream, AtomicInteger currentlyServedStream, - AtomicLong timeout, boolean multiTimeStream, @Nullable Runnable callback) { - } } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java index e9fca650224..16e8b36789d 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java @@ -80,7 +80,7 @@ protected void activate(BundleContext context) { } @Override - public synchronized void process(final @Nullable AudioStream audioStream) + public synchronized void processAsynchronously(final @Nullable AudioStream audioStream) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { if (audioStream != null && !AudioFormat.CODEC_MP3.equals(audioStream.getFormat().getCodec())) { AudioPlayer audioPlayer = new AudioPlayer(audioStream); @@ -104,8 +104,7 @@ public synchronized void process(final @Nullable AudioStream audioStream) } else { try { // we start a new continuous stream and store its handle - streamPlayer = new Player(audioStream); - playInThread(streamPlayer, () -> runDelayed(audioStream)); + playInThread(audioStream, true); } catch (JavaLayerException e) { LOGGER.error("An exception occurred while playing url audio stream : '{}'", e.getMessage()); } @@ -114,7 +113,7 @@ public synchronized void process(final @Nullable AudioStream audioStream) } else { // we are playing some normal file (no url stream) try { - playInThread(new Player(audioStream), () -> runDelayed(audioStream)); + playInThread(audioStream, false); } catch (JavaLayerException e) { LOGGER.error("An exception occurred while playing audio : '{}'", e.getMessage()); } @@ -122,18 +121,20 @@ public synchronized void process(final @Nullable AudioStream audioStream) } } - private void playInThread(final @Nullable Player player, Runnable toRunAfter) { + private void playInThread(final AudioStream audioStream, boolean store) throws JavaLayerException { // run in new thread + Player streamPlayerFinal = new Player(audioStream); + if (store) { // we store its handle in case we want to interrupt it. + streamPlayer = streamPlayerFinal; + } threadFactory.newThread(() -> { - if (player != null) { - try { - player.play(); - } catch (Exception e) { - LOGGER.error("An exception occurred while playing audio : '{}'", e.getMessage()); - } finally { - player.close(); - toRunAfter.run(); - } + try { + streamPlayerFinal.play(); + } catch (Exception e) { + LOGGER.error("An exception occurred while playing audio : '{}'", e.getMessage()); + } finally { + streamPlayerFinal.close(); + playbackFinished(audioStream); } }).start(); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java index 270103e0ce1..36e492ee772 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/webaudio/WebAudioAudioSink.java @@ -23,6 +23,7 @@ import org.openhab.core.audio.AudioSink; 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; @@ -61,7 +62,7 @@ public WebAudioAudioSink(@Reference AudioHTTPServer audioHTTPServer, @Reference } @Override - public void process(@Nullable AudioStream audioStream) + public void processAsynchronously(@Nullable AudioStream audioStream) throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { if (audioStream == null) { // in case the audioStream is null, this should be interpreted as a request to end any currently playing @@ -80,11 +81,12 @@ public void process(@Nullable AudioStream audioStream) logger.debug("Error while closing the audio stream: {}", e.getMessage(), e); } } else { - // we will let the HTTP servlet run the delayed task when finished with the stream - Runnable delayedTask = () -> this.runDelayed(audioStream); // we need to serve it for a while and make it available to multiple clients try { - sendEvent(audioHTTPServer.serve(audioStream, 10, delayedTask).toString()); + 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); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java index 84f7f21d300..b5a96e10811 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtils.java @@ -13,12 +13,12 @@ package org.openhab.core.audio.utils; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; -import org.openhab.core.audio.AudioSink; -import org.openhab.core.library.types.PercentType; -import org.slf4j.Logger; +import org.openhab.core.audio.AudioFormat; /** * Some utility methods for sink @@ -27,57 +27,17 @@ * */ @NonNullByDefault -public class AudioSinkUtils { +public interface AudioSinkUtils { /** - * Handle a volume command change and returns a Runnable to restore it. + * Transfers data from an input stream to an output stream and computes on the fly its duration * - * @param volume The volume to set - * @param sink The sink to set the volume to - * @param logger to log error to - * @return A runnable to restore the volume to its previous value, or null if no change is required + * @param in the input stream giving audio data ta play + * @param out the output stream receiving data to play + * @return the timestamp (from System.nanoTime) when the sound should be fully played. Returns null if computing + * time fails. + * @throws IOException if reading from the stream or writing to the stream failed */ - public static @Nullable Runnable handleVolumeCommand(@Nullable PercentType volume, AudioSink sink, Logger logger) { - boolean volumeChanged = false; - PercentType oldVolume = null; - - if (volume == null) { - return null; - } - - // set notification sound volume - try { - // get current volume - oldVolume = sink.getVolume(); - } catch (IOException | UnsupportedOperationException e) { - logger.debug("An exception occurred while getting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - - if (!volume.equals(oldVolume) || oldVolume == null) { - try { - sink.setVolume(volume); - volumeChanged = true; - } catch (IOException | UnsupportedOperationException e) { - logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - } - - final PercentType oldVolumeFinal = oldVolume; - Runnable toRunWhenProcessFinished = null; - // restore volume only if it was set before - if (volumeChanged && oldVolumeFinal != null) { - toRunWhenProcessFinished = () -> { - try { - sink.setVolume(oldVolumeFinal); - } catch (IOException | UnsupportedOperationException e) { - logger.debug("An exception occurred while setting the volume of sink '{}' : {}", sink.getId(), - e.getMessage(), e); - } - }; - } - - return toRunWhenProcessFinished; - } + @Nullable + Long transferAndAnalyzeLength(InputStream in, OutputStream out, AudioFormat audioFormat) throws IOException; } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtilsImpl.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtilsImpl.java new file mode 100644 index 00000000000..f49da0ce8e0 --- /dev/null +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtilsImpl.java @@ -0,0 +1,91 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.audio.utils; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import javazoom.jl.decoder.Bitstream; +import javazoom.jl.decoder.BitstreamException; +import javazoom.jl.decoder.Header; + +import javax.sound.sampled.AudioInputStream; +import javax.sound.sampled.AudioSystem; +import javax.sound.sampled.UnsupportedAudioFileException; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.audio.AudioFormat; +import org.osgi.service.component.annotations.Component; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Some utility methods for sink + * + * @author Gwendal Roulleau - Initial contribution + * + */ +@NonNullByDefault +@Component +public class AudioSinkUtilsImpl implements AudioSinkUtils { + + private final Logger logger = LoggerFactory.getLogger(AudioSinkUtilsImpl.class); + + @Override + public @Nullable Long transferAndAnalyzeLength(InputStream in, OutputStream out, AudioFormat audioFormat) + throws IOException { + // take some data from the stream beginning + byte[] dataBytes = in.readNBytes(8192); + + // beginning sound timestamp : + long startTime = System.nanoTime(); + // copy already read data to the output stream : + out.write(dataBytes); + // transfer everything else + Long dataTransferedLength = dataBytes.length + in.transferTo(out); + + if (dataTransferedLength > 0) { + if (AudioFormat.CODEC_PCM_SIGNED.equals(audioFormat.getCodec())) { + try (AudioInputStream audioInputStream = AudioSystem + .getAudioInputStream(new ByteArrayInputStream(dataBytes))) { + int frameSize = audioInputStream.getFormat().getFrameSize(); + float frameRate = audioInputStream.getFormat().getFrameRate(); + long computedDuration = Float.valueOf((dataTransferedLength / (frameSize * frameRate)) * 1000000000) + .longValue(); + return startTime + computedDuration; + } catch (IOException | UnsupportedAudioFileException e) { + logger.debug("Cannot compute the duration of input stream", e); + return null; + } + } else if (AudioFormat.CODEC_MP3.equals(audioFormat.getCodec())) { + // not precise, no VBR, but better than nothing + Bitstream bitstream = new Bitstream(new ByteArrayInputStream(dataBytes)); + try { + Header h = bitstream.readFrame(); + if (h != null) { + long computedDuration = Float.valueOf(h.total_ms(dataTransferedLength.intValue()) * 1000000) + .longValue(); + return startTime + computedDuration; + } + } catch (BitstreamException ex) { + logger.debug("Cannot compute the duration of input stream", ex); + return null; + } + } + } + + return null; + } +} diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java index be7402cebc7..04f6a80fe36 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioStreamUtils.java @@ -12,21 +12,7 @@ */ package org.openhab.core.audio.utils; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import javazoom.jl.decoder.Bitstream; -import javazoom.jl.decoder.BitstreamException; -import javazoom.jl.decoder.Header; - -import javax.sound.sampled.AudioInputStream; -import javax.sound.sampled.AudioSystem; -import javax.sound.sampled.UnsupportedAudioFileException; - import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; -import org.openhab.core.audio.AudioFormat; /** * Some general filename and extension utilities. @@ -80,56 +66,4 @@ public static String getExtension(String filename) { public static boolean isExtension(String filename, String extension) { return !extension.isEmpty() && getExtension(filename).equals(extension); } - - /** - * Transfers data from an input stream to an output stream and computes on the fly its duration - * - * @param in the input stream giving audio data ta play - * @param out the output stream receiving data to play - * @return the timestamp (from System.nanoTime) when the sound should be fully played. Returns null if computing - * time fails. - * @throws IOException if reading from the stream or writing to the stream failed - */ - public static @Nullable Long transferAndAnalyzeLength(InputStream in, OutputStream out, AudioFormat audioFormat) - throws IOException { - // take some data from the stream beginning - byte[] dataBytes = in.readNBytes(8192); - - // beginning sound timestamp : - long startTime = System.nanoTime(); - // copy already read data to the output stream : - out.write(dataBytes); - // transfer everything else - Long dataTransferedLength = dataBytes.length + in.transferTo(out); - - if (dataTransferedLength > 0) { - if (AudioFormat.CODEC_PCM_SIGNED.equals(audioFormat.getCodec())) { - try (AudioInputStream audioInputStream = AudioSystem - .getAudioInputStream(new ByteArrayInputStream(dataBytes))) { - int frameSize = audioInputStream.getFormat().getFrameSize(); - float frameRate = audioInputStream.getFormat().getFrameRate(); - long computedDuration = Float.valueOf((dataTransferedLength / (frameSize * frameRate)) * 1000000000) - .longValue(); - return startTime + computedDuration; - } catch (IOException | UnsupportedAudioFileException e) { - return null; - } - } else if (AudioFormat.CODEC_MP3.equals(audioFormat.getCodec())) { - // not precise, no VBR, but better than nothing - Bitstream bitstream = new Bitstream(new ByteArrayInputStream(dataBytes)); - try { - Header h = bitstream.readFrame(); - if (h != null) { - long computedDuration = Float.valueOf(h.total_ms(dataTransferedLength.intValue()) * 1000000) - .longValue(); - return startTime + computedDuration; - } - } catch (BitstreamException ex) { - return null; - } - } - } - - return null; - } } diff --git a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java index 458b6e47cf3..c70545adc4e 100644 --- a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java +++ b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java @@ -33,6 +33,8 @@ import org.openhab.core.audio.AudioFormat; import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.ByteArrayAudioStream; +import org.openhab.core.audio.utils.AudioSinkUtils; +import org.openhab.core.audio.utils.AudioSinkUtilsImpl; import org.openhab.core.test.TestPortUtil; import org.openhab.core.test.TestServer; import org.openhab.core.test.java.JavaTest; @@ -61,10 +63,11 @@ public abstract class AbstractAudioServletTest extends JavaTest { public @Mock @NonNullByDefault({}) HttpService httpServiceMock; public @Mock @NonNullByDefault({}) HttpContext httpContextMock; + public AudioSinkUtils audioSinkUtils = new AudioSinkUtilsImpl(); @BeforeEach public void setupServerAndClient() { - audioServlet = new AudioServlet(); + audioServlet = new AudioServlet(audioSinkUtils); ServletHolder servletHolder = new ServletHolder(audioServlet); diff --git a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java index 30430242b04..6f7200604a6 100644 --- a/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java +++ b/bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java @@ -30,7 +30,7 @@ import org.openhab.core.audio.ByteArrayAudioStream; import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.FixedLengthAudioStream; -import org.openhab.core.audio.internal.AudioServlet.StreamServed; +import org.openhab.core.audio.StreamServed; import org.openhab.core.audio.internal.utils.BundledSoundFileHandler; /** diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java index 62f00d4d754..fbb20b9c40f 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java @@ -42,7 +42,6 @@ import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.UnsupportedAudioFormatException; import org.openhab.core.audio.UnsupportedAudioStreamException; -import org.openhab.core.audio.utils.AudioSinkUtils; import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -229,6 +228,7 @@ public void say(String text, @Nullable String voiceId, @Nullable String sinkId) public void say(String text, @Nullable String voiceId, @Nullable String sinkId, @Nullable PercentType volume) { Objects.requireNonNull(text, "Text cannot be said as it is null."); + Runnable volumeRestauration = null; try { TTSService tts = null; Voice voice = null; @@ -272,16 +272,17 @@ public void say(String text, @Nullable String voiceId, @Nullable String sinkId, throw new TTSException( "Failed playing audio stream '" + audioStream + "' as audio sink doesn't support it"); } - - Runnable volumeRestoration = AudioSinkUtils.handleVolumeCommand(volume, sink, logger); - - sink.process(audioStream, volumeRestoration); + volumeRestauration = audioManager.handleVolumeCommand(volume, sink); + sink.processAndComplete(audioStream).thenRun(volumeRestauration); } catch (TTSException | UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { if (logger.isDebugEnabled()) { logger.debug("Error saying '{}': {}", text, e.getMessage(), e); } else { logger.warn("Error saying '{}': {}", text, e.getMessage()); } + if (volumeRestauration != null) { + volumeRestauration.run(); + } } } From 39e77705d7de1b440894f89379139026a58ee345 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Wed, 14 Jun 2023 14:46:42 +0200 Subject: [PATCH 11/11] [audio] More capabilities for AudioSink using the AudioServlet Apply code review Signed-off-by: Gwendal Roulleau --- .../org/openhab/core/audio/AudioSink.java | 14 +++++++---- .../openhab/core/audio/AudioSinkAsync.java | 25 +++++++++---------- .../org/openhab/core/audio/AudioSinkSync.java | 7 +++--- .../core/audio/internal/AudioManagerImpl.java | 15 ++++------- .../javasound/JavaSoundAudioSink.java | 4 +-- .../core/voice/internal/VoiceManagerImpl.java | 15 +++++------ 6 files changed, 38 insertions(+), 42 deletions(-) diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java index d3f40e601f8..adc542a5ce4 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSink.java @@ -87,12 +87,16 @@ void process(@Nullable AudioStream audioStream) * interface, the sink should hereafter get rid of it by calling the dispose method. * * @param audioStream the audio stream to play or null to keep quiet - * @throws UnsupportedAudioFormatException If audioStream format is not supported - * @throws UnsupportedAudioStreamException If audioStream is not supported + * @return A future completed when the sound is fully played. The method can instead complete with + * UnsupportedAudioFormatException if the audioStream format is not supported, or + * UnsupportedAudioStreamException If audioStream is not supported */ - default CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) - throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { - process(audioStream); + default CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) { + try { + process(audioStream); + } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { + return CompletableFuture.failedFuture(e); + } return CompletableFuture.completedFuture(null); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java index cbc2962abe6..c88d20c93c9 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkAsync.java @@ -38,25 +38,24 @@ public abstract class AudioSinkAsync implements AudioSink { private final Logger logger = LoggerFactory.getLogger(AudioSinkAsync.class); - private final Map> runnableByAudioStream = new HashMap<>(); + protected final Map> runnableByAudioStream = new HashMap<>(); @Override - public CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) - throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + public CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) { CompletableFuture<@Nullable Void> completableFuture = new CompletableFuture<@Nullable Void>(); + if (audioStream != null) { + runnableByAudioStream.put(audioStream, completableFuture); + } try { - if (audioStream != null) { - runnableByAudioStream.put(audioStream, completableFuture); - } processAsynchronously(audioStream); - return completableFuture; - } finally { - if (audioStream == null) { - // No need to delay the post process task - runnableByAudioStream.remove(audioStream); - completableFuture.complete(null); - } + } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { + completableFuture.completeExceptionally(e); + } + if (audioStream == null) { + // No need to delay the post process task + completableFuture.complete(null); } + return completableFuture; } @Override diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java index e0318385083..97c888f7468 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioSinkSync.java @@ -36,10 +36,12 @@ public abstract class AudioSinkSync implements AudioSink { private final Logger logger = LoggerFactory.getLogger(AudioSinkSync.class); @Override - public CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) - throws UnsupportedAudioFormatException, UnsupportedAudioStreamException { + public CompletableFuture<@Nullable Void> processAndComplete(@Nullable AudioStream audioStream) { try { processSynchronously(audioStream); + return CompletableFuture.completedFuture(null); + } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { + return CompletableFuture.failedFuture(e); } finally { // as the stream is not needed anymore, we should dispose of it if (audioStream instanceof Disposable disposableAudioStream) { @@ -55,7 +57,6 @@ public abstract class AudioSinkSync implements AudioSink { } } } - return CompletableFuture.completedFuture(null); } @Override diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java index 066d1edbf1f..eb79b9344f5 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioManagerImpl.java @@ -38,8 +38,6 @@ import org.openhab.core.audio.AudioStream; import org.openhab.core.audio.FileAudioStream; import org.openhab.core.audio.URLAudioStream; -import org.openhab.core.audio.UnsupportedAudioFormatException; -import org.openhab.core.audio.UnsupportedAudioStreamException; import org.openhab.core.audio.utils.ToneSynthesizer; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -122,14 +120,11 @@ public void play(@Nullable AudioStream audioStream, @Nullable String sinkId) { public void play(@Nullable AudioStream audioStream, @Nullable String sinkId, @Nullable PercentType volume) { AudioSink sink = getSink(sinkId); if (sink != null) { - Runnable volumeRestauration = null; - try { - volumeRestauration = handleVolumeCommand(volume, sink); - sink.processAndComplete(audioStream).thenRun(volumeRestauration); - } catch (UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { - volumeRestauration.run(); - logger.warn("Error playing '{}': {}", audioStream, e.getMessage(), e); - } + Runnable restoreVolume = handleVolumeCommand(volume, sink); + sink.processAndComplete(audioStream).exceptionally((exception) -> { + logger.warn("Error playing '{}': {}", audioStream, exception.getMessage(), exception); + return null; + }).thenRun(restoreVolume); } else { logger.warn("Failed playing audio stream '{}' as no audio sink was found.", audioStream); } diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java index 16e8b36789d..41208b514f6 100644 --- a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java +++ b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/javasound/JavaSoundAudioSink.java @@ -13,7 +13,6 @@ package org.openhab.core.audio.internal.javasound; import java.io.IOException; -import java.math.BigDecimal; import java.nio.charset.StandardCharsets; import java.util.Locale; import java.util.Scanner; @@ -87,6 +86,7 @@ public synchronized void processAsynchronously(final @Nullable AudioStream audio audioPlayer.start(); try { audioPlayer.join(); + playbackFinished(audioStream); } catch (InterruptedException e) { LOGGER.error("Playing audio has been interrupted."); } @@ -177,7 +177,7 @@ public PercentType getVolume() throws IOException { return true; }); if (volumes[0] != null) { - return new PercentType(new BigDecimal(volumes[0] * 100f)); + return new PercentType(Math.round(volumes[0] * 100f)); } else { LOGGER.warn("Cannot determine master volume level - assuming 100%"); return PercentType.HUNDRED; diff --git a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java index fbb20b9c40f..de770481a69 100644 --- a/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java +++ b/bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java @@ -40,8 +40,6 @@ import org.openhab.core.audio.AudioSink; import org.openhab.core.audio.AudioSource; import org.openhab.core.audio.AudioStream; -import org.openhab.core.audio.UnsupportedAudioFormatException; -import org.openhab.core.audio.UnsupportedAudioStreamException; import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.config.core.ConfigOptionProvider; import org.openhab.core.config.core.ConfigurableService; @@ -228,7 +226,6 @@ public void say(String text, @Nullable String voiceId, @Nullable String sinkId) public void say(String text, @Nullable String voiceId, @Nullable String sinkId, @Nullable PercentType volume) { Objects.requireNonNull(text, "Text cannot be said as it is null."); - Runnable volumeRestauration = null; try { TTSService tts = null; Voice voice = null; @@ -272,17 +269,17 @@ public void say(String text, @Nullable String voiceId, @Nullable String sinkId, throw new TTSException( "Failed playing audio stream '" + audioStream + "' as audio sink doesn't support it"); } - volumeRestauration = audioManager.handleVolumeCommand(volume, sink); - sink.processAndComplete(audioStream).thenRun(volumeRestauration); - } catch (TTSException | UnsupportedAudioFormatException | UnsupportedAudioStreamException e) { + Runnable restoreVolume = audioManager.handleVolumeCommand(volume, sink); + sink.processAndComplete(audioStream).exceptionally(exception -> { + logger.warn("Error playing '{}': {}", audioStream, exception.getMessage(), exception); + return null; + }).thenRun(restoreVolume); + } catch (TTSException e) { if (logger.isDebugEnabled()) { logger.debug("Error saying '{}': {}", text, e.getMessage(), e); } else { logger.warn("Error saying '{}': {}", text, e.getMessage()); } - if (volumeRestauration != null) { - volumeRestauration.run(); - } } }