0

I'm writing a rhythm game in Java; right now I've reached the point where I'm trying to implement a metronome object.
I've coded a data structure that stores 8 channels of music data into a single QuarterBeat object; these are in turn stored in groups of 64 to make 4-measure 'chunk' objects.
To keep things synchronized properly I wanted to use a pair of parallel threads: one runs through the various events that happen at every quarter-beat before stopping on a 'wait()' method, while the other waits an amount of time derived from the BPM before signaling the first.

This is the code for the thread doing the work.

public class InGame {
    public static boolean gameRunning = false;
    public static boolean holdChunk = false;
    public static boolean waiting = false;
    public static ArrayList<Player> players = new ArrayList<Player>();

    public void startUp() throws InterruptedException{
        Parser.loadSamples();
        for (int p = 0; p < Player.voicePool.size(); p++) {
            Player makePlay = new Player();
            makePlay.setChannel(p);
            players.add(makePlay);
        }
        LevelStructure.SongBuild();
        Metro timer = new Metro();
        gamePlay(timer);
        gameEnd();
    }

    synchronized public void cycle(Metro timer) throws InterruptedException{
        int endPoint = LevelStructure.getChunkTotal();
        for (int chunk = 0; chunk < endPoint; chunk++){
            LevelStructure.setActiveChunk(chunk);
            for (int quartBeat = 0; quartBeat < 64; quartBeat++){
                synchronized (this){
                    new Thread(timer.ticking(this));
                    Player.getNewNotes(LevelStructure.getQuartBeat(quartBeat));
                    players.get(0).playback(LevelStructure.getQuartBeat(quartBeat));
                    waiting = true;
                    while (waiting) {
                        wait();
                    }
                }   
            }
            if (holdChunk) chunk--;
        }
    }
}

And the code for the Metro object:

public class Metro {
    public static int BPM;

    synchronized public Runnable ticking(InGame parent) throws InterruptedException{
        synchronized (parent) {
            Thread.sleep(15000/BPM);
            InGame.waiting = false;
            parent.notifyAll();
        }
        return null;
    }
}

Right now it's throwing an Illegal Monitor State exception every time I try to run it; I've tried researching proper implementation of wait()/notify() on my own, but I'm still fairly new to Java and I can't find an explanation of handling parallel threading that I can understand. Do I need to invoke both the Cycle and Metro threads from a parent process?

Edit: Updated code: the issue now is that, instead of actually running in parallel, the Cycle object waits for the timer.ticking method to execute, then performs the actions it's supposed to do while Metro is sleeping, then gets stuck waiting for a notify that will never come. Which means that the threads aren't actually executing parallel to one-another.

Camkitsune
  • 23
  • 1
  • 4
  • 1
    Some existing threads that might be useful: http://stackoverflow.com/questions/886722/how-to-use-wait-and-notify-in-java?rq=1 (specifically why you're getting the exception), http://stackoverflow.com/questions/3278234/new-to-multithreading-how-to-use-wait-and-notify-in-java?rq=1 , http://stackoverflow.com/questions/8579934/wait-and-notify-concepts-java-multithreading?rq=1 (more general concepts about wait/notify and when to use them/not use them) – aruisdante Feb 26 '15 at 17:51
  • Where are the threads in this code? – RealSkeptic Feb 26 '15 at 17:54

2 Answers2

1

I guess it's because you are calling this.wait() without having the lock of the object this.

Try to get the lock first.

synchronized (this) {
    this.wait();
}
Kennedy Oliveira
  • 2,141
  • 2
  • 20
  • 25
1

Kennedy's answer gives the solution to the question you asked about the exception, but the bigger problem is that you are using wait()/notify() with only one thread. The thread that is supposed to call playback() is the one that executes ticking(), so it will pause for a quarter beat. I'd do the loop like to try to stay on schedule:

void cycle() 
  throws InterruptedException 
{
  int endPoint = LevelStructure.getChunkTotal();
  for (int chunk = 0; chunk < endPoint; chunk++) {
    LevelStructure.setActiveChunk(chunk);
    for (int quartBeat = 0; quartBeat < 64; quartBeat++) {
      long start = System.nanoTime();
      Player.playback(LevelStructure.getQuartBeat(quartBeat));
      long delay = 15_000_000_000L / BPM - (System.nanoTime() - start);
      if (delay < 0)
        throw new IllegalStateException("Allotted time exceeded");
      if (delay > 0)
        Thread.sleep(delay / 1_000_000, delay % 1_000_000);
    }
  }
}

If you feel like you need multiple threads, here's a more complicated approach. At its heart, it relies on a CyclicBarrier to coordinate between the worker and the timer. You could also schedule messages to the synthesizer using a ScheduledExecutorService.

import java.util.concurrent.BrokenBarrierException;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.TimeUnit;

class Test
{

  private final CyclicBarrier sync = new CyclicBarrier(2);

  /** Duration of quarter beat in milliseconds. */
  private final int qb;

  Test(int bpm)
  {
    qb = 15000 / bpm;
  }

  private void cycle()
    throws InterruptedException, BrokenBarrierException
  {
    Timer timer = new Timer();
    timer.start();
    sync.await();
    for (int chunk = 0; chunk < 4; ++chunk)
      chunk();
    timer.interrupt();
  }

  private void chunk()
    throws InterruptedException, BrokenBarrierException
  {
    long t1 = System.nanoTime();
    for (int count = 0; count < 64; ++count) {
      playback();
      long t2 = System.nanoTime();
      sync.await(); /* Now wait the remaining time, if any. */
      long t3 = System.nanoTime();
      float t = TimeUnit.NANOSECONDS.toMillis(t2 - t1) / 1000F;
      float c = TimeUnit.NANOSECONDS.toMillis(t3 - t1) / 1000F;
      System.out.printf("Task: %5.3f, Cycle: %5.3f seconds%n", t, c);
      t1 = t3;
    }
  }

  void playback()
  {
    /* Simulate performing some work sleeping a random time which is, 
     * on average, an eighth of a beat, but can "jank" occasionally by taking 
     * too long to do its job. */
    long delay = (long) (-qb / 2 * Math.log(1 - Math.random()));
    // long delay = qb / 2; /* Simulate a predictable workload. */
    try {
      Thread.sleep(delay);
    }
    catch (InterruptedException abort) {
      Thread.currentThread().interrupt();
    }
  }

  private final class Timer
    extends Thread
  {

    @Override
    public void run()
    {
      try {
        sync.await();
        while (true) {
          Thread.sleep(qb);
          sync.await();
        }
      }
      catch (InterruptedException abort) {
        return;
      }
      catch (BrokenBarrierException ex) {
        ex.printStackTrace();
      }
    }

  };

  public static void main(String... argv)
    throws Exception
  {
    Test player = new Test(120);
    long t0 = System.nanoTime();
    player.cycle();
    System.out.printf("Total: %5.3f seconds%n", TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - t0) / 1000F);
  }

}
Community
  • 1
  • 1
erickson
  • 265,237
  • 58
  • 395
  • 493
  • I tried something to that effect before looking into using a set of parallel threads; the issue is that there is still a fairly serious amount of desync that occurs. performing the check only once at the start and end of a 16-measure stretch at 120 BPM should result in a total elapsed time of 32 seconds. It wound up taking 324,671,649,360 nanoseconds to run through the whole loop with only one check at the start and the end of the 16 measures, and 325,341,672,160 when using a method similar to the one you've suggested. – Camkitsune Feb 26 '15 at 19:10
  • @Camkitsune What about the issue I pointed out, which is that you don't have multiple threads! You are already sleeping in the playback thread, but making no allowance for the playback time. And it sounds like your playback might be taking too long itself. – erickson Feb 26 '15 at 19:15
  • That is, in a way, part of the problem; instead of creating a second parallel thread, timer.ticking() gets executed as a continuation of whatever thread invokes it. How exactly do I call the 'ticking' thread in such a way as to cause it to run parallel to the 'cylce' thread? – Camkitsune Feb 26 '15 at 19:32
  • What is `Player`? Does `playback()` return immediately, while the sound plays asynchronously? Or does it block, returning when the sound is done? – erickson Feb 26 '15 at 22:31
  • Right now, Player is only associated with picking a particular system message based on the pitch data at a particular point in the data structure. In the future it will probably be necessary to further split playback into multiple threads based on channels (as of now there are 8), but before the more complicated elements of the program's design can be tackled I need to resolve the fairly basic one I've currently run up against. – Camkitsune Feb 27 '15 at 14:32
  • I wrote it; right now playback() doesn't do anything other than print out a line in the console, since I wanted to get the metronome functionality working properly before I started messing with the audio, since it's critical to linking together user-input (which doesn't even exist yet), moving through the data structure, scoring, and audio playback. – Camkitsune Feb 27 '15 at 14:39
  • I'm currently coding on a desktop running windows 7, so I'm fairly sure that the issue isn't with an internal clock. – Camkitsune Feb 27 '15 at 15:34
  • 1
    Many thanks, implementing a Cyclic Barrier did the trick. I've still got a bit of tinkering to do to determine whether or not everything is properly in time, but for the time being what I've written so far is functional. – Camkitsune Feb 27 '15 at 20:14