TarsosDSP and SurfaceView Multiple Threading Issue

Question

I am using TarsosDSP to calculate pitch frequencies in real time. It uses an AudioDispatcher which implements Runnable and post the results via handlePitch method to make use of in the main thread.

I am using SurfaceView to draw this value as it updates. SurfaceView also requires another thread to draw on canvas. So I have 2 runnable objects. I couldnt manage how to update surface view via one thread while getting the pitch values from another thread (audiodispatcher).

I just want to use cent value which I get in the handlePitch() method to update my drawing over surfaceview. But my app freezes. Any idea?

In MainAcitivity.java (onCreate(...))

   myView = (MySurfaceView) findViewById(R.id.myview);

    int sr = 44100;//The sample rate
    int bs = 2048;
    AudioDispatcher d = AudioDispatcherFactory.fromDefaultMicrophone(sr,bs,0);
    PitchDetectionHandler printPitch = new PitchDetectionHandler() {
        @Override
        public void handlePitch(final PitchDetectionResult pitchDetectionResult, AudioEvent audioEvent) {
            final float p = pitchDetectionResult.getPitch();

            runOnUiThread(new Runnable() {
                @Override
                public void run() {

                    if (p != -1){
                        float cent = (float) (1200*Math.log(p/8.176)/Math.log(2)) % 12;
                        System.out.println(cent);
                        myView.setCent(cent);
                    }
                }
            });
        }
    };

    PitchProcessor.PitchEstimationAlgorithm algo = PitchProcessor.PitchEstimationAlgorithm.YIN; //use YIN
    AudioProcessor pitchEstimator = new PitchProcessor(algo, sr,bs,printPitch);
    d.addAudioProcessor(pitchEstimator);
    d.run();//starts the dispatching process
    AudioProcessor p = new PitchProcessor(algo, sr, bs, printPitch);
    d.addAudioProcessor(p);
    new Thread(d,"Audio Dispatcher").start();

In SurfaceView.java (below code is triggered from the constructor)

    myThread = new MyThread(this);
    surfaceHolder = getHolder();
    bmpIcon = BitmapFactory.decodeResource(getResources(),
            R.mipmap.ic_launcher);

    iconWidth = bmpIcon.getWidth();
    iconHeight = bmpIcon.getHeight();
    density = getResources().getDisplayMetrics().scaledDensity;
    setLabelTextSize(Math.round(DEFAULT_LABEL_TEXT_SIZE_DP * density));

    surfaceHolder.addCallback(new SurfaceHolder.Callback(){

        @Override
        public void surfaceCreated(SurfaceHolder holder) {
            myThread.setRunning(true);
            myThread.start();
        }

        @Override
        public void surfaceChanged(SurfaceHolder holder,
                                   int format, int width, int height) {
            // TODO Auto-generated method stub

        }

        @Override
        public void surfaceDestroyed(SurfaceHolder holder) {
            boolean retry = true;
            myThread.setRunning(false);
            while (retry) {
                try {
                    myThread.join();
                    retry = false;
                } catch (InterruptedException e) {
                }
            }
        }});


      protected void drawSomething(Canvas canvas) {

         updateCanvas(canvas, this.cent); //draws some lines depending on the cent value
      }


    public void setCent(double cent) {

        if (this.cent > maxCent)
            this.cent = maxCent;
        this.cent = cent;
    }

UPDATE:

MyThread.java

public class MyThread extends Thread {

    MySurfaceView myView;
    private boolean running = false;

    public MyThread(MySurfaceView view) {
        myView = view;
    }

    public void setRunning(boolean run) {
        running = run;
    }

    @Override
    public void run() {
        while(running){

            Canvas canvas = myView.getHolder().lockCanvas();

            if(canvas != null){
                synchronized (myView.getHolder()) {
                    myView.drawSomething(canvas);
                }
                myView.getHolder().unlockCanvasAndPost(canvas);
            }

            try {
                sleep(1000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

        }
    }

Answer 1

If I understand your problem correctly, you have an independent source of events working on its own thread (PitchDetectionHandler) and a SurfaceView that you want to re-paint on its own thread when the event from the source comes. If this is the case, then I think the whole idea with sleep(1000) is wrong. You should track actual events and react to them rather than sleep waiting for them. And it seems that on Android the easiest solution is to use HandlerThread/Looper/Handler infrastructure like so:

Beware of the bugs in the following code; not only haven't I tried it but I even haven't compiled it.

import android.graphics.Canvas;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.Message;
import android.view.SurfaceHolder;


public class SurfacePitchDrawingHelper implements Handler.Callback, SurfaceHolder.Callback2 {

    private static final int MSG_DRAW = 100;
    private static final int MSG_FORCE_REDRAW = 101;

    private final Object _lock = new Object();
    private SurfaceHolder _surfaceHolder;
    private HandlerThread _drawingThread;
    private Handler _handler;

    private float _lastDrawnCent;
    private volatile float _lastCent;

    private final boolean _processOnlyLast = true;

    @Override
    public void surfaceCreated(SurfaceHolder holder) {
        synchronized (_lock) {
            _surfaceHolder = holder;

            _drawingThread = new HandlerThread("SurfaceDrawingThread") {
                @Override
                protected void onLooperPrepared() {
                    super.onLooperPrepared();
                }
            };

            _drawingThread.start();
            _handler = new Handler(_drawingThread.getLooper(), this); // <-- this is where bug was
            _lastDrawnCent = Float.NaN;
            //postForceRedraw(); // if needed
        }
    }


    @Override
    public void surfaceDestroyed(SurfaceHolder holder) {
        synchronized (_lock) {
            // clean queue and kill looper
            _handler.removeCallbacksAndMessages(null);
            _drawingThread.getLooper().quit();

            while (true) {
                try {
                    _drawingThread.join();
                    break;
                } catch (InterruptedException e) {
                }
            }

            _handler = null;
            _drawingThread = null;
            _surfaceHolder = null;
        }
    }

    @Override
    public void surfaceRedrawNeeded(SurfaceHolder holder) {
        postForceRedraw();
    }


    @Override
    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
        synchronized (_lock) {
            _surfaceHolder = holder;
        }
        postForceRedraw();
    }

    private void postForceRedraw() {
        _handler.sendEmptyMessage(MSG_FORCE_REDRAW);
    }

    public void postRedraw(float cent) {
        if (_processOnlyLast) {
            _lastCent = cent;
            _handler.sendEmptyMessage(MSG_DRAW);
        } else {
            Message message = _handler.obtainMessage(MSG_DRAW);
            message.obj = Float.valueOf(cent);
            _handler.sendMessage(message);
        }
    }


    private void doRedraw(Canvas canvas, float cent) {
        // put actual painting logic here
    }

    @Override
    public boolean handleMessage(Message msg) {
        float lastCent = _processOnlyLast ? _lastCent : ((Float) msg.obj).floatValue();
        boolean shouldRedraw = (MSG_FORCE_REDRAW == msg.what)
                || ((MSG_DRAW == msg.what) && (_lastDrawnCent != lastCent));

        if (shouldRedraw) {
            Canvas canvas = null;
            synchronized (_lock) {
                if (_surfaceHolder != null)
                    canvas =_surfaceHolder.lockCanvas();
            }
            if (canvas != null) {
                doRedraw(canvas, lastCent);
                _surfaceHolder.unlockCanvasAndPost(canvas);
                _lastDrawnCent = lastCent;
            }

            return true;
        }

        return false;
    }
}

And then in your activity class you do something like

private SurfaceView surfaceView;
private SurfacePitchDrawingHelper surfacePitchDrawingHelper = new SurfacePitchDrawingHelper();

...

@Override
protected void onCreate(Bundle savedInstanceState) {

    ...

    surfaceView = (SurfaceView) findViewById(R.id.surfaceView);
    surfaceView.getHolder().addCallback(surfacePitchDrawingHelper);

    int sr = 44100;//The sample rate
    int bs = 2048;
    AudioDispatcher d = AudioDispatcherFactory.fromDefaultMicrophone(sr, bs, 0);
    PitchDetectionHandler printPitch = new PitchDetectionHandler() {
        @Override
        public void handlePitch(final PitchDetectionResult pitchDetectionResult, AudioEvent audioEvent) {
            final float p = pitchDetectionResult.getPitch();
            float cent = (float) (1200 * Math.log(p / 8.176) / Math.log(2)) % 12;
            System.out.println(cent);
            surfacePitchDrawingHelper.postRedraw(cent);
        }
    };

    PitchProcessor.PitchEstimationAlgorithm algo = PitchProcessor.PitchEstimationAlgorithm.YIN; //use YIN
    AudioProcessor pitchEstimator = new PitchProcessor(algo, sr, bs, printPitch);
    d.addAudioProcessor(pitchEstimator);
    // d.run();//starts the dispatching process <-- this was another bug in the original code (see update)!
    AudioProcessor p = new PitchProcessor(algo, sr, bs, printPitch);
    d.addAudioProcessor(p);
    new Thread(d, "Audio Dispatcher").start();

    ...

}

Note that SurfacePitchDrawingHelper encapsulates most of the logic related to drawing and there is no need in your subclass MySurfaceView (which I think is a bad idea anyway).

The main idea is that SurfacePitchDrawingHelper creates are dedicated HandlerThread when new Surface is created. HandlerThread + Looper + Handler provide a useful infrastructure of running (in an efficient way) an infinite loop on a separate thread that waits for incoming messages and handles them one by one. So its effective public API besides SurfaceHolder.Callback2 consists of single postRedraw method that might be used to ask the drawing thread to do another redraw and this is exactly what is used by custom PitchDetectionHandler. "Asking" is done by putting a message in the queue to be processed by the drawing thread (more specifically our custom Handler on that thread). I didn't bother with reducing real public API to "effective" one because it makes code a bit more complicated and I'm too lazy. But of course both "implements" can be moved to inner classes.

There is one important decision to be made by you: whether drawing thread should produce each inbound message (all cent values) in order the came or just the latest at the moment drawing occurs. This might become especially important in case PitchDetectionHandler produces events much faster then the "drawing thread" can update Surface. I believe that for most of the cases it is OK to handle just the last value from the PitchDetectionHandler but I left both version in the code for illustration. This distinction is currently implemented in the code by _processOnlyLast field. Most probably you should make this decision ones and just get rid of this almost-constant-field and the code in the irrelevant branches.

And of course don't forget to put your actual drawing logic inside doRedraw

---

Update (why Back button doesn't work)

TLDR version

The offending line is

 d.run();//starts the dispatching process

Just comment it out!

Longer version

Looking at your example we can see that d is AudioDispatcher which implements Runnable and thus run method is a method to be called on a new thread. You may notice that this is important because inside this method does some IO and blocks the thread it runs on. So in your case it blocked the main UI thread. A few lines down you do

new Thread(d, "Audio Dispatcher").start();

and this seems to be a correct way to use AudioDispatcher

This can easily be seen from the stack traces that I asked in the comments for.