Modify defscins to place bins in the asdf-cache

[rubenphilipp]

As of now, defscins places the resulting files from the compilation to the bin directory (see https://github.com/mdedwards/slippery-chicken/issues/81). Since we are now moving to asdf, let's change this so that instruments's binaries are also located at asdf's cache (cf. compile-clm-ins in the .asd file for reference).

[rubenphilipp]

By the way: isn't utilities.lsp a better location for defscins?

[rubenphilipp]

Okay, that was really easy (after some erroneous tries). We can now use definstrument in sc for clm instruments, as clm instruments are, via asdf, "automatically" compiled (as of my .asd "hack") to asdf's cache directory. Cf. e3e3d6a22a61985c1b0061ea3b5f732aa0b7675a. Tests run smoothly on my machine.

[Leon-Focker]

Works for me as well!

[mdedwards]

We don't need defscins anymore, now we're using asdf. I've removed it, along with autoc.ins. Tests are passing without either. Checked in.

[rubenphilipp]

Unfortunately, removing defscins as well as the instruments originally taken from CLM does have some side-effects. The actual fasl files are correctly placed in ASDF's cache, but not the .c, .o, and .so files, which now pollute my home directory. As you might now, it is not possible to change this behavior when using compile-file, as the :c-file has to be defined with the definstrument. I shortly had the hope to be able to sensibly change this via the *definstrument-hook*, but this does not work since the hook is expanded/evaluated after the respective c-file has already been opened. Thus, we might need to re-introduce the defscins macro and get back some crucial CLM-instruments unless we are happy with having the files generated by CLM's compilation in our home-dir.

[rubenphilipp]

Alright. I have reintroduced defscins with some ASDF-specific modifications (please check). This is the related commit: 8a6d332

[rubenphilipp]

P.S.: I think, this also aligns better aligns with the "ASDF-agnostic" policy we discussed in several threads here. P.P.S.: Maybe, samp5.lsp isn't the best location for this macro as it is also used by more than one file. For my taste, we should either add a comment to the .asd (and all.lsp) that samp5 needs to be the first instrument loaded/compiled or load the macro from a different file.

[rubenphilipp]

Update: It also seems as if we could now (after this workaround) remove the compilation function from the .asd file (cf. 9504d82). It works on my systems, but please test on yours.

Cheers Ruben

[mdedwards]

Ah you broke my lisp! samp5 would no longer compile :/

I think we might both be a little too quick on the draw here. I've made defscins work again by writing .c and .o files to the tmp dir. I'm not so happy at having asdf magic in that call anyway. I've also asked Bill if he knows of a global solution.

if we retain defscins then it's fine in samp5.ins but I can see why you might prefer it in clm.lsp

also if we retain decscins then I guess we have to bring back autoc.ins :/

[rubenphilipp]

Oh, I am sorry for that.

I think we might both be a little too quick on the draw here. I've made defscins work again by writing .c and .o files to the tmp dir. I'm not so happy at having asdf magic in that call anyway. I've also asked Bill if he knows of a global solution.

That's true. Let's keep the CLM-compilation in this state until we've heard something from Bill (thanks for contacting him).

also if we retain decscins then I guess we have to bring back autoc.ins :/

Maybe we should, unless we expect the user to handle properly compiling CLM's default instruments, which would be worth mentioning in the installation instructions in the readme.

[mdedwards]

actually, if we get the .c/.o problem sorted out with Bill (i.e. he accepts my modifications to defins.lisp--see email) then we can compile clm instruments from its src dir by using clm-source-directory --- assuming that looks right on your machine: it does on mine.

[rubenphilipp]

Yes, that looks promising. Thanks a lot for this suggestion to Bill. The only problem we've got then is to discern the *clm-c-directory* as set by sc. If we hard-code it to /tmp, we might get problems on some OSs. On the other hand, getting the value from sc's globals does not make much more sense, as the user (without modifying globals.lsp) could not change the respective value during the load/compilation of the system. We could also use a conditional for our setf in samp5.lsp which just changes the *clm-c-directory* when its value is NIL:

(unless *clm-c-directory*
  (setf *clm-c-directory* "/tmp/"))

Or:

(unless *clm-c-directory*
  (setf *clm-c-directory* (slippery-chicken::get-sc-config 'default-dir)))

This should then also be mentioned in the installation instructions.

[rubenphilipp]

I was curious and tested the new approach by applying your patch to my local defins.lisp, added the aforementioned code (using (slippery-chicken::get-sc-config 'default-dir)) to samp5.lsp and changed all defscins back to definstrument (all done locally without intending to do a commit). Strangely, this works for all sc instruments except samp5 which still pollutes my home dir. I assume that this relates to ASDF's loading/compiling mechanism that does not actually alter a global variable during compilation, but just when loading the compiled fasl, while we expect the var to be changed during compilation. I've placed then the setf *clm-c-directory*... form to globals.lsp and everything works fine.

[mdedwards]

Famous last words and all that but I don't think it'll be complicated and I don't think we'll need any instructions for the user. We'll just #+clm (setf *clm-c-directory* (sc::get-sc-config 'tmp-dir)) in globals or clm.lsp then load autoc.ins from *clm-source-directory* and then samp5 etc.

[rubenphilipp]

That sounds very charming, after this tour de force.

[mdedwards]

Yes, the whole asdf thing has been/is almost as bad as I feared. But we're not done yet. There's still time for everything to go to hell (says the eternal optimist) ;-)

[rubenphilipp]

I hope, that we'll be able to write a proper requiem using sc before doom. But Bill has just come to rescue…

[mdedwards]

Hi Ruben, I've checked in a new version that anticipates the CLM changes and handles definstrument write directories (using 'tmp-dir not default-dir as we wouldn't want to 'pollute' that either). defscins is out. autoc.ins and scentroid.ins only compile and load their .ins files (from the clm src dir) if and when they're called :-) I've left samp5 in your asd but we could consider only loading that if the user calls clm-play---that would speed up system loading.

vidfile.lsp has also been added but is not yet fully functional or tested

other than that the full tests pass (thought I'd have to hammer it given such a fundamental change)

[mdedwards]

give it a look over and a good test yourself and then close this issue if you're happy

[rubenphilipp]

Brilliant. Thank you, Michael! I have pulled your commit and successfully run the full tests. Though, I will wait for Bill's clm-update and then redo the tests prior to closing.

I've left samp5 in your asd but we could consider only loading that if the user calls clm-play---that would speed up system loading

This is not a bad idea. What is your preference?

I did not check vidfile yet but will do so later.

[mdedwards]

I've left samp5 in your asd but we could consider only loading that if the user calls clm-play---that would speed up system loading

This is not a bad idea. What is your preference?

To be honest I think it would be better to load as and when necessary, like the others. But we’re not loading from the clm src so would need a little different logic.

[rubenphilipp]

That's true. Will you figure this out?

I've just noticed that the /tmp dir might not be an ideal location to keep the .so files as they get removed after system restart unlike the fasls in the cache. This leads to an error as the loading system assumes the .so files to be present (as they are called from the fasls). Do you have a clever idea for this? One option would be to set the *clm-c-directory* to the asdf-cache folder when ASDF is available, but I understand that you are reluctant to such an approach.

[mdedwards]

This is what I was thinking: we don't generally rely on .fasl for clm instruments rather we compile and load each time. Thus we can take sine.lsp and samp5.lsp out of the asd definition (I've commented them out for now) and have them auto-loaded the first time clm-play is called each session--using the now extended get-clm-ins fun (clm.lsp). Thus using the /tmp folder shouldn't be an issue.

git updated and tests pass. The only problem I see is that the samp5 and sine.fasl are now polluting the src dir. Some asdf magic perhaps?

Of course maybe my logic is flawed there. I've updated git and the standard tests pass. Let me know.

[mdedwards]

PS This is what happens if we just load the .fasl in advance:

SC> (load "/Users/michael/sc/src/samp5.fasl")
T
SC> (load-from-test-suite-dir "sc-test-suite-aux.lsp")

*******  Loading sc-test-suite-aux.lsp
T
SC> (test-clm-play)
******* section (1)
Getting notes for VN
Getting notes for VC
******* section (2)
Getting notes for VN
******* section (3)
Getting notes for VN
Getting notes for VC
Shortening short, fast leaps...
Shortened 0 large fast leaps
Testing: (TEST-CLM-PLAY)...
slippery-chicken::midi-play: updating amplitudes to reflect dynamic marks. 
Set :update-amplitudes to NIL if you don't want this 
(e.g. if you've set amplitudes directly).
Respelling notes...
Inserting automatic clefs....
Generating VN...
Generating VC...
Inserting line breaks...
Creating systems...
Calling CMN...
**************************** 
"/tmp/clm_SINE.c" 
"/tmp/clm_SINE.c" ; Writing "/tmp/clm_SINE.c"
; Compiling "/tmp/clm_SINE.c"
; Creating shared object file "/tmp/clm_SINE.so"

So you see that we don't gain much with the .fasl....but I don't like it in my src folder :/

[rubenphilipp]

That sounds reasonable. I've also run the tests now, successfully.

The only problem I see is that the samp5 and sine.fasl are now polluting the src dir. Some asdf magic perhaps?

Isn't this CL's/SBCL's default behavior? I've changed the location for the binaries by adding an :output-file to the comile-file in defun get-clm-ins and it works.

(defun get-clm-ins (function-symbol ins-src
                    &optional (dir clm::*clm-source-directory*))
  (unless (fboundp function-symbol)
    (let ((*package* (find-package :clm))) ; compile within the clm package
      (load (compile-file (format nil "~a~a" dir ins-src)
                          :output-file (format nil "~a/~a"
                                               (get-sc-config 'tmp-dir)
                                               (pathname-name ins-src)))))
    (unless (fboundp function-symbol)
      (error "clm::get-clm-ins: the CLM instrument ~
              ~a (defined in ~a) needs to be loaded in order for a dependent ~
              function to work. We tried to auto-load this and it failed."
             function-symbol ins-src ))))

Shall I check this in?

[mdedwards]

If the tests pass like that and there's no pollution we might just be able to close this issue

[mdedwards]

check it in yes

[rubenphilipp]

I've just done the commit. If this patch works for you, as it does for me, feel free to close this issue.

P.S.: Puuhh, the tests with ffprobe are really, really slow.

[mdedwards]

I've just done the commit. If this patch works for you, as it does for me, feel free to close this issue.

Thanks. That's great.

P.S.: Puuhh, the tests with ffprobe are really, really slow.

yes, I warned you. I already reduced the number of times that update was being called in the sndfile class. That helped but not much. I did some further investigative work this morning and found lots of places where parent classes were copying/cloning data and this was triggering update when working on reaper items. I also made calling update when init'ing a sndfile optional and turned it off where I thought it wasn't necessary. Also calling make-reaper-item in the reaper-play main loop was essentially creating a sndfile object (and thus calling update) when all we need really was to copy over slots. So I created a new function called make-reaper-item-fast and all in all (not one single thing did the job) speed was massively improved.