From c061301f74607595458bd76bb68bd337808a7f60 Mon Sep 17 00:00:00 2001 From: Damien Quartz Date: Wed, 27 Feb 2019 15:12:19 -0600 Subject: [PATCH] Issue #93 - handle negative millisecond length correctly in cue and skip implementations of Playable classes + just for completeness, I've fixed it in AudioSnippet even though loadSnippet is deprecated and I will be changing code so that a snippet will fail to load if the file length can't be determined --- src/main/java/ddf/minim/AudioPlayer.java | 64 ++++++++++--------- src/main/java/ddf/minim/AudioSnippet.java | 24 +++++-- src/main/java/ddf/minim/ugens/FilePlayer.java | 37 ++++++----- .../ddf/minim/tests/UnknownFileLength.java | 51 +++++++++++++++ 4 files changed, 122 insertions(+), 54 deletions(-) create mode 100644 src/test/java/ddf/minim/tests/UnknownFileLength.java diff --git a/src/main/java/ddf/minim/AudioPlayer.java b/src/main/java/ddf/minim/AudioPlayer.java index 57d95bd..238ec0b 100644 --- a/src/main/java/ddf/minim/AudioPlayer.java +++ b/src/main/java/ddf/minim/AudioPlayer.java @@ -237,8 +237,8 @@ public int position() * the beginning. This will not change the play state. If an error * occurs while trying to cue, the position will not change. * If you try to cue to a negative position or to a position - * that is greater than length(), the amount will be clamped - * to zero or length(). + * that is greater than a non-negative length(), + * the amount will be clamped to zero or length(). * * @shortdesc Sets the position to millis milliseconds from * the beginning. @@ -254,45 +254,47 @@ public int position() public void cue(int millis) { if (millis < 0) - { + { millis = 0; - } - else if (millis > length()) - { - millis = length(); - } + } + else + { + // only clamp millis to the length of the file if the length is known. + // otherwise we will try to skip what is asked and count on the underlying stream to handle it. + int len = recording.getMillisecondLength(); + if (len >= 0 && millis > len) + { + millis = len; + } + } recording.setMillisecondPosition(millis); } - /** - * Skips millis milliseconds from the current position. - * millis can be negative, which will make this skip backwards. - * If the skip amount would result in a negative position or a position that is greater than - * length(), the new position will be clamped to zero or - * length(). - * - * @shortdesc Skips millis milliseconds from the current position. - * - * @param millis - * int: how many milliseconds to skip, sign indicates direction - * - * @example AudioPlayer/skip - * - * @related AudioPlayer - */ + /** + * Skips millis milliseconds from the current position. + * millis can be negative, which will make this skip backwards. + * If the skip amount would result in a negative position or a position that is greater than + * a non-negative length(), the new position will be clamped to zero or length(). + * + * @shortdesc Skips millis milliseconds from the current position. + * + * @param millis + * int: how many milliseconds to skip, sign indicates direction + * + * @example AudioPlayer/skip + * + * @related AudioPlayer + */ public void skip(int millis) { int pos = position() + millis; - if (pos < 0) + if ( pos < 0 ) { pos = 0; } - else if (pos > length()) - { - pos = length(); - } - Minim.debug("AudioPlayer.skip: skipping " + millis + " milliseconds, new position is " + pos); - recording.setMillisecondPosition(pos); + + Minim.debug("AudioPlayer.skip: attempting to skip " + millis + " milliseconds, to position " + pos); + cue(pos); } /** diff --git a/src/main/java/ddf/minim/AudioSnippet.java b/src/main/java/ddf/minim/AudioSnippet.java index 5965f2c..cfd8248 100644 --- a/src/main/java/ddf/minim/AudioSnippet.java +++ b/src/main/java/ddf/minim/AudioSnippet.java @@ -98,20 +98,32 @@ public int position() public void cue(int millis) { if (millis < 0) + { millis = 0; - if (millis > length()) - millis = length(); + } + else + { + // only clamp millis to the length of the file if the length is known. + // otherwise we will try to skip what is asked and count on the underlying stream to handle it. + int len = recording.getMillisecondLength(); + if (len >= 0 && millis > len) + { + millis = len; + } + } recording.setMillisecondPosition(millis); } public void skip(int millis) { int pos = position() + millis; - if (pos < 0) + if ( pos < 0 ) + { pos = 0; - else if (pos > length()) - pos = length(); - recording.setMillisecondPosition(pos); + } + + Minim.debug("AudioSnippet.skip: attempting to skip " + millis + " milliseconds, to position " + pos); + cue(pos); } public boolean isLooping() diff --git a/src/main/java/ddf/minim/ugens/FilePlayer.java b/src/main/java/ddf/minim/ugens/FilePlayer.java index f97cb06..12b4ddc 100644 --- a/src/main/java/ddf/minim/ugens/FilePlayer.java +++ b/src/main/java/ddf/minim/ugens/FilePlayer.java @@ -228,8 +228,8 @@ public int position() * the beginning. This will not change the play state. If an error * occurs while trying to cue, the position will not change. * If you try to cue to a negative position or try to a position - * that is greater than length(), the amount will be clamped - * to zero or length(). + * that is greater than a non-negative length(), + * the amount will be clamped to zero or length(). * * @shortdesc Sets the position to millis milliseconds from * the beginning. @@ -241,13 +241,19 @@ public int position() public void cue(int millis) { if (millis < 0) - { + { millis = 0; - } - else if (millis > length()) - { - millis = length(); - } + } + else + { + // only clamp millis to the length of the file if the length is known. + // otherwise we will try to skip what is asked and count on the underlying stream to handle it. + int len = mFileStream.getMillisecondLength(); + if (len >= 0 && millis > len) + { + millis = len; + } + } mFileStream.setMillisecondPosition(millis); // change the position in the stream invalidates our buffer, so we read a new buffer fillBuffer(); @@ -255,10 +261,10 @@ else if (millis > length()) /** * Skips millis from the current position. millis - * can be negative, which will make this skip backwards. If the skip amount - * would result in a negative position or a position that is greater than - * length(), the new position will be clamped to zero or - * length(). + * can be negative, which will make this skip backwards. + * If the skip amount would result in a negative position + * or a position that is greater than a non-negative length(), + * the new position will be clamped to zero or length(). * * @shortdesc Skips millis from the current position. * @@ -274,11 +280,8 @@ public void skip(int millis) { pos = 0; } - else if (pos > length()) - { - pos = length(); - } - //Minim.debug("AudioPlayer.skip: skipping " + millis + " milliseconds, new position is " + pos); + + Minim.debug("FilePlayer.skip: attempting to skip " + millis + " milliseconds, to position " + pos); cue( pos ); } diff --git a/src/test/java/ddf/minim/tests/UnknownFileLength.java b/src/test/java/ddf/minim/tests/UnknownFileLength.java new file mode 100644 index 0000000..126e395 --- /dev/null +++ b/src/test/java/ddf/minim/tests/UnknownFileLength.java @@ -0,0 +1,51 @@ +package ddf.minim.tests; + +import ddf.minim.Minim; +import ddf.minim.Playable; +import ddf.minim.ugens.FilePlayer; + +/** + * This test was created to ensure that audio files whose length is unknown are treated correctly by all API calls that use the length + * + * @see https://github.com/ddf/Minim/issues/93 + * @author Damien Quartz + * + */ +public class UnknownFileLength extends TestBase +{ + + public static void main(String[] args) + { + Start(new UnknownFileLength(), args); + } + + protected void setup(String[] args) + { + // this is the test file noted in https://github.com/ddf/Minim/issues/73, which loads with an unknown length, + // allowing us to ensure that several methods behave correctly under this condition. + String fileName = "http://www.noiseaddicts.com/samples_1w72b820/2553.mp3"; + + testCueSkip( minim.loadFile(fileName) ); + + testCueSkip( new FilePlayer(minim.loadFileStream( fileName )) ); + } + + void testCueSkip(Playable player) + { + if (player != null) + { + // should report -1, if it doesn't we need to use a different file + Minim.debug( player.getMetaData().fileName() + " length: " + player.length() ); + + // should be ignored and report nothing + player.cue( -1000 ); + // debug should report skipping forward by 1000 milliseconds + player.cue( 1000 ); + + // debug should report skipping forward by 1000 milliseconds to position 2000 + player.skip( 1000 ); + // debug should report skipping by -3000 milliseconds to position 0 + player.skip( -3000 ); + } + } +}