alda-lang / alda-server-clj

A Clojure implementation of an Alda server
Other
2 stars 2 forks source link

Add an option to include the score data in `alda play` response #7

Closed daveyarwood closed 7 years ago

daveyarwood commented 7 years ago

Spin-off issue from https://github.com/alda-lang/alda-client-java/pull/9.

The new (Java) client-side Alda REPL operates by communicating with the Alda server. One of the things it needs to be able to do is to play a snippet of code and determine the current instruments of the score so that it can update the REPL prompt.

It would be nice to do this in a single request, so I think we should add an option to the play command to include the score data in the response.

daveyarwood commented 7 years ago

Closed via https://github.com/alda-lang/alda-server-clj/commit/5a6d16796608feaf2c91a835036bafddc7922650.

I ended up just always including the score map in the play-status response if it is available (i.e. after the score has been parsed).

jgkamat commented 7 years ago

I think there might be a race condition in the current implementation. If I print out the response json, I sometimes (but rarely) get:

> piano: c d e
{"success":true,"body":"Request received."}
{"success":true,"body":"playing","score":null,"pending":false}
p> piano: c d e
{"success":true,"body":"Request received."}
{"success":true,"body":"parsing","score":null,"pending":true}
{"success":true,"body":"playing","score":{"chord-mode":false,"current-instruments":["piano-O6OBg"],"events":[{"offset":2000.0,"instrument":"piano-O6OBg","volume":1.0,"track-volume":0.7874015748031497,"panning":0.5,"midi-note":62,"pitch":293.6647679174076,"duration":450.0,"voice":null},{"offset":0,"instrument":"piano-O6OBg","volume":1.0,"track-volume":0.7874015748031497,"panning":0.5,"midi-note":60,"pitch":261.6255653005986,"duration":450.0,"voice":null},{"offset":2500.0,"instrument":"piano-O6OBg","volume":1.0,"track-volume":0.7874015748031497,"panning":0.5,"midi-note":64,"pitch":329.6275569128699,"duration":450.0,"voice":null},{"offset":500.0,"instrument":"piano-O6OBg","volume":1.0,"track-volume":0.7874015748031497,"panning":0.5,"midi-note":62,"pitch":293.6647679174076,"duration":450.0,"voice":null},{"offset":1500.0,"instrument":"piano-O6OBg","volume":1.0,"track-volume":0.7874015748031497,"panning":0.5,"midi-note":60,"pitch":261.6255653005986,"duration":450.0,"voice":null},{"offset":1000.0,"instrument":"piano-O6OBg","volume":1.0,"track-volume":0.7874015748031497,"panning":0.5,"midi-note":64,"pitch":329.6275569128699,"duration":450.0,"voice":null}],"beats-tally":null,"instruments":{"piano-O6OBg":{"octave":4,"current-offset":{"offset":3000.0},"key-signature":{},"config":{"type":"midi","patch":1},"duration":{"beats":1,"ms":null},"min-duration":null,"volume":1.0,"last-offset":{"offset":2500.0},"id":"piano-O6OBg","quantization":0.9,"duration-inside-cram":null,"tempo":120,"panning":0.5,"current-marker":"start","time-scaling":1,"stock":"midi-acoustic-grand-piano","track-volume":0.7874015748031497}},"markers":{"start":0},"cram-level":0,"global-attributes":{},"nicknames":{},"beats-tally-default":null},"pending":false}

Adding any print information to the server seems to slow this down enough that I don't see it. I'm going to try getting some more information, but for now that's all I've got :cry:

daveyarwood commented 7 years ago

I think you're on the right track... it does look like a race condition in the worker code. I'll do some thinking about how to fix that.

daveyarwood commented 7 years ago

So the problem here seems to be that the response occasionally comes back with the status as "playing," but the score is null instead of a score like we expect.

I made one improvement in alda-server-clj 0.1.6 that should help keep this from happening. There are more details in the CHANGELOG, but basically, the improvement is that we make sure to update the status in sync with the score.

I'm not sure if this will fix it or not, but I thought it would be a good improvement to make, anyway. Would you mind trying it out?

jgkamat commented 7 years ago

Yup, I'll use that version to test out the repl instrument status! Thanks for the quick fix :O