[audio] Improve audio duration computation (#3675)

Allows the use of a Sizeable interface (for AudioStream that we know the length of, but we cannot clone). We can then improve the duration detection, for example for the pulseaudio sink (PR coming after). We can also give the length information to sink in more cases. Add the support of the mark / reset methods to some common AudioStream. We then allow more stream analysis for sink requiring it (Stream analysis often requires to get back in time after consuming a few bytes) Signed-off-by: Gwendal Roulleau <[email protected]>
openhab · Jul 2, 2023 · 3ec1457 · 3ec1457
1 parent 5ceaa64
commit 3ec1457
Show file tree

Hide file tree

Showing 13 changed files with 118 additions and 34 deletions.
diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioFormat.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/AudioFormat.java
@@ -484,7 +484,7 @@ public String toString() {
                 + (bigEndian != null ? "bigEndian=" + bigEndian + ", " : "")
                 + (bitDepth != null ? "bitDepth=" + bitDepth + ", " : "")
                 + (bitRate != null ? "bitRate=" + bitRate + ", " : "")
-                + (frequency != null ? "frequency=" + frequency : "") + (channels != null ? "channels=" + channels : "")
-                + "]";
+                + (frequency != null ? "frequency=" + frequency + ", " : "")
+                + (channels != null ? "channels=" + channels : "") + "]";
     }
 }
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
@@ -31,7 +31,7 @@ public interface AudioHTTPServer {
     /**
      * Creates a relative url for a given {@link AudioStream} where it can be requested a single time.
      * Note that the HTTP header only contains "Content-length", if the passed stream is an instance of
-     * {@link FixedLengthAudioStream}.
+     * {@link SizeableAudioStream}.
      * If the client that requests the url expects this header field to be present, make sure to pass such an instance.
      * Streams are closed after having been served.
      *

diff --git a/...les/org.openhab.core.audio/src/main/java/org/openhab/core/audio/ByteArrayAudioStream.java b/...les/org.openhab.core.audio/src/main/java/org/openhab/core/audio/ByteArrayAudioStream.java
@@ -19,7 +19,8 @@
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
 /**
- * This is an implementation of a {@link FixedLengthAudioStream}, which is based on a simple byte array.
+ * This is an implementation of an {@link AudioStream} with known length and a clone method, which is based on a simple
+ * byte array.
  *
  * @author Kai Kreuzer - Initial contribution
  */
@@ -60,4 +61,19 @@ public long length() {
     public InputStream getClonedStream() {
         return new ByteArrayAudioStream(bytes, format);
     }
+
+    @Override
+    public synchronized void mark(int readlimit) {
+        stream.mark(readlimit);
+    }
+
+    @Override
+    public synchronized void reset() throws IOException {
+        stream.reset();
+    }
+
+    @Override
+    public boolean markSupported() {
+        return true;
+    }
 }
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
@@ -17,12 +17,12 @@
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
 /**
- * This is an {@link AudioStream}, that can be cloned
+ * This is for an {@link AudioStream}, that can be cloned
  *
- * @author Gwendal Roulleau - Initial contribution, separation from FixedLengthAudioStream
+ * @author Gwendal Roulleau - Initial contribution, separation from {@link FixedLengthAudioStream}
  */
 @NonNullByDefault
-public abstract class ClonableAudioStream extends AudioStream {
+public interface ClonableAudioStream {
 
     /**
      * Returns a new, fully independent stream instance, which can be read and closed without impacting the original
@@ -31,5 +31,5 @@ public abstract class ClonableAudioStream extends AudioStream {
      * @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;
+    public InputStream getClonedStream() throws AudioException;
 }
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
@@ -42,9 +42,11 @@ public class FileAudioStream extends FixedLengthAudioStream implements Disposabl
 
     private final File file;
     private final AudioFormat audioFormat;
-    private InputStream inputStream;
+    private FileInputStream inputStream;
     private final long length;
     private final boolean isTemporaryFile;
+    private int markedOffset = 0;
+    private int alreadyRead = 0;
 
     public FileAudioStream(File file) throws AudioException {
         this(file, getAudioFormat(file));
@@ -87,7 +89,7 @@ private static AudioFormat parseWavFormat(File file) throws AudioException {
         }
     }
 
-    private static InputStream getInputStream(File file) throws AudioException {
+    private static FileInputStream getInputStream(File file) throws AudioException {
         try {
             return new FileInputStream(file);
         } catch (FileNotFoundException e) {
@@ -102,7 +104,9 @@ public AudioFormat getFormat() {
 
     @Override
     public int read() throws IOException {
-        return inputStream.read();
+        int read = inputStream.read();
+        alreadyRead++;
+        return read;
     }
 
     @Override
@@ -124,11 +128,23 @@ public synchronized void reset() throws IOException {
         }
         try {
             inputStream = getInputStream(file);
+            inputStream.skipNBytes(markedOffset);
+            alreadyRead = markedOffset;
         } catch (AudioException e) {
             throw new IOException("Cannot reset file input stream: " + e.getMessage(), e);
         }
     }
 
+    @Override
+    public synchronized void mark(int readlimit) {
+        markedOffset = alreadyRead;
+    }
+
+    @Override
+    public boolean markSupported() {
+        return true;
+    }
+
     @Override
     public InputStream getClonedStream() throws AudioException {
         return getInputStream(file);

diff --git a/...s/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FixedLengthAudioStream.java b/...s/org.openhab.core.audio/src/main/java/org/openhab/core/audio/FixedLengthAudioStream.java
@@ -15,18 +15,15 @@
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
 /**
- * This is a {@link ClonableAudioStream}, which can also provide information about its absolute length.
+ * This is a {@link AudioStream}, which can also provide information about its absolute length and get cloned.
  *
  * @author Kai Kreuzer - Initial contribution
- * @author Gwendal Roulleau - Separate getClonedStream into its own class
+ * @author Gwendal Roulleau - Separate getClonedStream and length into their own interface.
+ * @deprecated You should consider using {@link ClonableAudioStream} and/or {@link SizeableAudioStream} to detect audio
+ *             stream capabilities
  */
 @NonNullByDefault
-public abstract class FixedLengthAudioStream extends ClonableAudioStream {
+@Deprecated
+public abstract class FixedLengthAudioStream extends AudioStream implements SizeableAudioStream, ClonableAudioStream {
 
-    /**
-     * Provides the length of the stream in bytes.
-     *
-     * @return absolute length in bytes
-     */
-    public abstract long length();
 }
diff --git a/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/SizeableAudioStream.java b/bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/SizeableAudioStream.java
@@ -0,0 +1,31 @@
+/**
+ * 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 org.eclipse.jdt.annotation.NonNullByDefault;
+
+/**
+ * This is for an {@link AudioStream}, which size is known
+ *
+ * @author Gwendal Roulleau - Initial contribution, separation from {@link FixedLengthAudioStream}
+ */
+@NonNullByDefault
+public interface SizeableAudioStream {
+
+    /**
+     * Provides the length of the stream in bytes.
+     *
+     * @return absolute length in bytes
+     */
+    public long length();
+}
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
@@ -40,7 +40,7 @@
  * @author Christoph Weitkamp - Refactored use of filename extension
  */
 @NonNullByDefault
-public class URLAudioStream extends ClonableAudioStream {
+public class URLAudioStream extends AudioStream implements ClonableAudioStream {
 
     private static final Pattern PLS_STREAM_PATTERN = Pattern.compile("^File[0-9]=(.+)$");
 

diff --git a/...es/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java b/...es/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java
@@ -48,7 +48,7 @@
 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.audio.SizeableAudioStream;
 import org.openhab.core.audio.StreamServed;
 import org.openhab.core.audio.utils.AudioSinkUtils;
 import org.openhab.core.common.ThreadPoolManager;
@@ -135,8 +135,8 @@ private InputStream prepareInputStream(final StreamServed streamServed, final Ht
         }
 
         // try to set the content-length, if possible
-        if (streamServed.audioStream() instanceof FixedLengthAudioStream fixedLengthServedStream) {
-            final long size = fixedLengthServedStream.length();
+        if (streamServed.audioStream() instanceof SizeableAudioStream sizeableServedStream) {
+            final long size = sizeableServedStream.length();
             resp.setContentLength((int) size);
         }
 
@@ -285,9 +285,9 @@ public StreamServed serve(AudioStream originalStream, int seconds, boolean multi
         return streamToServe;
     }
 
-    private ClonableAudioStream createClonableInputStream(AudioStream stream, String streamId) throws IOException {
+    private AudioStream createClonableInputStream(AudioStream stream, String streamId) throws IOException {
         byte[] dataBytes = stream.readNBytes(ONETIME_STREAM_BUFFER_MAX_SIZE + 1);
-        ClonableAudioStream clonableAudioStreamResult;
+        AudioStream clonableAudioStreamResult;
         if (dataBytes.length <= ONETIME_STREAM_BUFFER_MAX_SIZE) {
             // we will use an in memory buffer to avoid disk operation
             clonableAudioStreamResult = new ByteArrayAudioStream(dataBytes, stream.getFormat());

diff --git a/...org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtilsImpl.java b/...org.openhab.core.audio/src/main/java/org/openhab/core/audio/utils/AudioSinkUtilsImpl.java
@@ -66,11 +66,20 @@ public class AudioSinkUtilsImpl implements AudioSinkUtils {
                             .longValue();
                     return startTime + computedDuration;
                 } catch (IOException | UnsupportedAudioFileException e) {
-                    logger.debug("Cannot compute the duration of input stream", e);
+                    logger.debug("Cannot compute the duration of input stream with method java stream sound analysis",
+                            e);
+                    Integer bitRate = audioFormat.getBitRate();
+                    if (bitRate != null && bitRate != 0) {
+                        long computedDuration = Float.valueOf((1f * dataTransferedLength / bitRate) * 1000000000)
+                                .longValue();
+                        return startTime + computedDuration;
+                    } else {
+                        logger.debug("Cannot compute the duration of input stream by using audio format information");
+                    }
                     return null;
                 }
             } else if (AudioFormat.CODEC_MP3.equals(audioFormat.getCodec())) {
-                // not precise, no VBR, but better than nothing
+                // not accurate, no VBR support, but better than nothing
                 Bitstream bitstream = new Bitstream(new ByteArrayInputStream(dataBytes));
                 try {
                     Header h = bitstream.readFrame();

diff --git a/...rg.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java b/...rg.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AudioServletTest.java
@@ -29,7 +29,6 @@
 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.StreamServed;
 import org.openhab.core.audio.internal.utils.BundledSoundFileHandler;
 
@@ -214,7 +213,7 @@ public void oneTimeStreamIsClosedAndRemovedAfterServed() throws Exception {
     @Test
     public void multiTimeStreamIsClosedAfterExpired() throws Exception {
         AtomicInteger cloneCounter = new AtomicInteger();
-        FixedLengthAudioStream audioStream = mock(FixedLengthAudioStream.class);
+        ByteArrayAudioStream audioStream = mock(ByteArrayAudioStream.class);
         AudioStream clonedStream = mock(AudioStream.class);
         AudioFormat audioFormat = mock(AudioFormat.class);
         when(audioStream.getFormat()).thenReturn(audioFormat);
@@ -250,7 +249,7 @@ public void multiTimeStreamIsClosedAfterExpired() throws Exception {
     @Test
     public void streamsAreClosedOnDeactivate() throws Exception {
         AudioStream oneTimeStream = mock(AudioStream.class);
-        FixedLengthAudioStream multiTimeStream = mock(FixedLengthAudioStream.class);
+        ByteArrayAudioStream multiTimeStream = mock(ByteArrayAudioStream.class);
 
         serveStream(oneTimeStream);
         serveStream(multiTimeStream, 10);

diff --git a/....openhab.core.audio/src/test/java/org/openhab/core/audio/internal/fake/AudioSinkFake.java b/....openhab.core.audio/src/test/java/org/openhab/core/audio/internal/fake/AudioSinkFake.java
@@ -21,7 +21,7 @@
 import org.openhab.core.audio.AudioFormat;
 import org.openhab.core.audio.AudioSink;
 import org.openhab.core.audio.AudioStream;
-import org.openhab.core.audio.FixedLengthAudioStream;
+import org.openhab.core.audio.ByteArrayAudioStream;
 import org.openhab.core.audio.URLAudioStream;
 import org.openhab.core.audio.UnsupportedAudioFormatException;
 import org.openhab.core.audio.UnsupportedAudioStreamException;
@@ -49,8 +49,8 @@ public class AudioSinkFake implements AudioSink {
     public boolean isUnsupportedAudioStreamExceptionExpected;
 
     private static final Set<AudioFormat> SUPPORTED_AUDIO_FORMATS = Set.of(AudioFormat.MP3, AudioFormat.WAV);
-    private static final Set<Class<? extends AudioStream>> SUPPORTED_AUDIO_STREAMS = Set
-            .of(FixedLengthAudioStream.class, URLAudioStream.class);
+    private static final Set<Class<? extends AudioStream>> SUPPORTED_AUDIO_STREAMS = Set.of(ByteArrayAudioStream.class,
+            URLAudioStream.class);
 
     @Override
     public String getId() {

diff --git a/...es/org.openhab.core/src/main/java/org/openhab/core/cache/lru/InputStreamCacheWrapper.java b/...es/org.openhab.core/src/main/java/org/openhab/core/cache/lru/InputStreamCacheWrapper.java
@@ -37,6 +37,7 @@ public class InputStreamCacheWrapper extends InputStream {
 
     private LRUMediaCacheEntry<?> cacheEntry;
     private int offset = 0;
+    private int markedOffset = 0;
 
     /***
      * Construct a transparent InputStream wrapper around data from the cache.
@@ -113,4 +114,19 @@ public long length() {
     public InputStream getClonedStream() throws IOException {
         return cacheEntry.getInputStream();
     }
+
+    @Override
+    public synchronized void mark(int readlimit) {
+        markedOffset = offset;
+    }
+
+    @Override
+    public synchronized void reset() throws IOException {
+        offset = markedOffset;
+    }
+
+    @Override
+    public boolean markSupported() {
+        return true;
+    }
 }