Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[sonos] Removed dependency on 'org.apache.commons.io.IOUtils' #7730

Closed
wants to merge 2 commits into from
Closed

[sonos] Removed dependency on 'org.apache.commons.io.IOUtils' #7730

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db695bc
[sonos] Removed dependency on 'org.apache.commons.io.IOUtils'
lolodomo May 22, 2020
f9d86da
Log added in case of error while closing the audio stream
lolodomo May 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ...penhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/SonosAudioSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.apache.commons.io.IOUtils;
import org.eclipse.smarthome.core.audio.AudioFormat;
import org.eclipse.smarthome.core.audio.AudioHTTPServer;
import org.eclipse.smarthome.core.audio.AudioSink;
Expand Down Expand Up @@ -115,7 +114,11 @@ public void process(AudioStream audioStream)
logger.warn("We do not have any callback url, so Sonos cannot play the audio stream!");
}
} else {
IOUtils.closeQuietly(audioStream);
try {
audioStream.close();
Copy link
Contributor

@cweitkamp cweitkamp May 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I once again have to jump into the discussion if we really have to close the passed AudioStream in AudioSink implementations. IIRC Chromecast and Kodi do not close it and I am not aware of any resource leak. Maybe we can find a final decision on it in openhab/openhab-core#532.

At the end we should make it clear in the JavaDoc for the Audiosink#process(AudioStream) method.

} catch (IOException e) {
logger.debug("Error while closing the audio stream: {}", e.getMessage());
}
throw new UnsupportedAudioStreamException(
"Sonos can only handle FixedLengthAudioStreams and URLAudioStreams.", audioStream.getClass());
// Instead of throwing an exception, we could ourselves try to wrap it into a
Expand Down