Skip to content

Commit

Permalink
Issue #93 - handle negative millisecond length correctly in cue and s…
Browse files Browse the repository at this point in the history 
…kip 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
  • Loading branch information
@ddf
ddf committed Feb 27, 2019
1 parent c936a35 commit c061301
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 54 deletions.
64 changes: 33 additions & 31 deletions src/main/java/ddf/minim/AudioPlayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code>length()</code>, the amount will be clamped
* to zero or <code>length()</code>.
* that is greater than a non-negative <code>length()</code>,
* the amount will be clamped to zero or <code>length()</code>.
*
* @shortdesc Sets the position to <code>millis</code> milliseconds from
* the beginning.
Expand All @@ -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 <code>millis</code> milliseconds from the current position.
* <code>millis</code> 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
* <code>length()</code>, the new position will be clamped to zero or
* <code>length()</code>.
*
* @shortdesc Skips <code>millis</code> milliseconds from the current position.
*
* @param millis
* int: how many milliseconds to skip, sign indicates direction
*
* @example AudioPlayer/skip
*
* @related AudioPlayer
*/
/**
* Skips <code>millis</code> milliseconds from the current position.
* <code>millis</code> 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 <code>length()</code>, the new position will be clamped to zero or <code>length()</code>.
*
* @shortdesc Skips <code>millis</code> 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);
}

/**
Expand Down
24 changes: 18 additions & 6 deletions src/main/java/ddf/minim/AudioSnippet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
37 changes: 20 additions & 17 deletions src/main/java/ddf/minim/ugens/FilePlayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code>length()</code>, the amount will be clamped
* to zero or <code>length()</code>.
* that is greater than a non-negative <code>length()</code>,
* the amount will be clamped to zero or <code>length()</code>.
*
* @shortdesc Sets the position to <code>millis</code> milliseconds from
* the beginning.
Expand All @@ -241,24 +241,30 @@ 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();
}

/**
* Skips <code>millis</code> from the current position. <code>millis</code>
* 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
* <code>length()</code>, the new position will be clamped to zero or
* <code>length()</code>.
* 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 <code>length()</code>,
* the new position will be clamped to zero or <code>length()</code>.
*
* @shortdesc Skips <code>millis</code> from the current position.
*
Expand All @@ -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 );
}

Expand Down
51 changes: 51 additions & 0 deletions src/test/java/ddf/minim/tests/UnknownFileLength.java
View file
Original file line number Diff line number Diff line change
@@ -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 );
}
}
}

0 comments on commit c061301

Please sign in to comment.