78rpm EQ Curve Generator

Yes, now I remember looking at that site, but I later mislaid the url.

I agree your re-arrangement looks a lot cleaner. It’s just that when I’m writing code, I tend to spell everything out step-by-step, and it can end up a bit clunky.

POL

I know what you mean - I tend to do the same myself, but I quite enjoy neatening it all up at the end, and I find it really helps if I ever need to come back to the code at a later date. Your code comments have been really helpful in showing what’s going on.

@POL, continuing from our off forum discussion, some thoughts about streamlining the code:


Once we’re all happy that the plug-in is “release ready” I think that we can remove much of the “change log”. This will help to bring the ;control data closer to the actual code, making it easier to follow.

As the license is GPL v2 we can add that in as a comment.

For the header data and comments I’d suggest trimming it down to something like this:

;nyquist plug-in
;version 3
;type generate
;name "78rpm EQ Curve V2.7"
;info "EQ Curve Generator for playback equalisation of 78rpm records. (V2.7)nBy Paraic O'Lochlainn. Released under GPL v2.nnGenerates an EQ curve and writes it to an xml file for import into Effect > Equalization.nSee  http://wiki.audacityteam.org/wiki/78rpm_playback_curves for a list of EQ curves."

;; EQ Curve Generator - version 3 plug-in (requires Audacity 1.3.13 or later)
;; by Paraic O'Lochlainn 
;; Released under terms of the GNU General Public License version 2:
;; http://www.gnu.org/licenses/old-licenses/gpl-2.0.html 
;;
;; Generates playback EQ curves, for 78rpm records, from "Bass Turnover" and "10kHz Gain Rolloff" parameters.
;; Writes the curve data to an xml file for use by Audacity's Equalization effect
;; Includes optional normalisation to 0dB at 1kHz.


;control mode "Select Function or Help" choice "Generate EQ Curve,Help: EQ Curves Part 1,Help: EQ Curves Part 2,Help: This Plug-in Part 1,Help: This Plug-in Part 2,Help: Cancelling RIAA EQ" 0
;control tofreq "Bass Turnover Frequency" real "Hz" 500.0 200.0 1000.0
;control tenkrolloff "10 kHz Gain Rolloff" real "dB" -13.656 -24.0 0.0
;control lfshelf "LF Shelving Frequencyn(50.0 safest - see Help)" real "Hz" 50.0 10.0 100.0
;control norm "Normalise to 0 dB at 1 kHz? " choice "Yes,No" 0
;control curname "Curve File Name" string "(max 30 char)" 78EQ
;control path "Valid File Output Foldern(default is Home directory)" string "" "Home directory"  

;; Note: the default input values generate the RIAA curve.

Is there any other info that needs to be included?


Again to help bring the ;control lines and the actual code closer together, I think it would be useful to wrap the help screens in functions and move the down near the bottom of the code (out of the way). At the very bottom of the code could be the “mode” conditional - something like this:

(defun help4 ()
  (format NIL
"Help text 4 ....."))

(defun help5 ()
  (format NIL
"Help text 5 ...."))


(case mode
  (0 (run))     ;Generate EQ
  (1 (help1))   ;Help: EQ Curves Part 1
  (2 (help2))   ;Help: EQ Curves Part 2
  (3 (help3))   ;Help: This Plug-in Part 1
  (4 (help4))   ;Help: This Plug-in Part 2
  (T (help5)))  ;Help: Cancelling RIAA EQ

The main body of the code would then be in the function (run) and all of the actual “doing” part of the code (other than this one simple conditional statement) would then be together.


Looking at the user value checking:

;; If Gain rolloff at 10kHz is empty, set it to zero 
;; Then if it is positive, make it negative 
(unless (numberp tenkrolloff)
(setq tenkrolloff 0.0)
)
(if (plusp tenkrolloff) 
(setq tenkrolloff (- tenkrolloff))
)

As “tenkrolloff” is supplied by the ;control

;control tenkrolloff "10 kHz Gain Rolloff" real "dB" -13.656 -24.0 0.0

it must be a float, so checking that it is a number is redundant.
(note in old versions of Audacity the returned value could be a float or integer, but this plug-in requires 1.3.13 or later so it will always be a float).
All that this bit of code is actually doing is ensuring that “tenkrolloff” is negative, so we can do that with the single line:

(setq tenkrolloff (- (abs tenkrolloff)))

The same for the other user entered parameters, so we can replace lines 211 to 238

;; Check for "sensible" parameter values

;; If Gain rolloff at 10kHz is empty, set it to zero 
;; Then if it is positive, make it negative 
(unless (numberp tenkrolloff)
(setq tenkrolloff 0.0)
)
(if (plusp tenkrolloff) 
(setq tenkrolloff (- tenkrolloff))
)

;; If Bass Turnover frequency is empty, set it to zero.
;; Then if it is negative, make it positive 
(unless (numberp tofreq)
(setq tofreq 0.0)
)
(if (minusp tofreq) 
(setq tofreq (- tofreq))
)

;; If LF Shelving frequency is empty, set it to zero.
;; Then if it is negative, make it positive
(unless (numberp lfshelf)
(setq lfshelf 0.0)
)
(if (minusp lfshelf) 
(setq lfshelf (- lfshelf))
)

with:

(setq tenkrolloff (- (abs tenkrolloff)))  ;10kHz must be negative
(setq tofreq (abs tofreq))                ;Bass Turnover must be positive
(setq lfshelf (abs lfshelf))              ;LF Shelving frequency must be positive.

Here’s quite a big one:
Rather than setting the values for each frequency as separate variables in the form of:

(setq gain20 (- (gaincalc 20.0) normfactor))
(setq gain25 (- (gaincalc 25.0) normfactor))
(setq gain31_5 (- (gaincalc 31.5) normfactor))
...
...

we could have an association list in the form:

((frequency1 . gain1)(frequency2 . gain2)(frequency3 . gain3)... )

As well as cutting down the number of lines of code considerably, the other advantage is that if ever anyone wanted to change the frequency values it would be much easier to do.

To do this, a list of frequency values could be created as:

;;; Create list of frequencies
(setf glist (list
  20 25 31 40 50 63 80 100 125 160 200 250 315
  400 500 630 800 1000 1250 1600 2000 2500 3150
  4000 5000 6300 8000 10000 12500 16000 20000
  25000 31500 48000))

and then the gain values associated with each frequency like this:

;; Calculate the gain at each frequency (34 in list)
(dotimes (i 34)
  (setf (nth i glist)
    (cons (nth i glist) (- (gaincalc (float (nth i glist))) normfactor))))

This could then be used to simplify the output strings by first creating the list of points as a single string

;; point list for XML
(setq pointlist "") ;initialise string
(dotimes (i 34)
  (setq pointlist (strcat pointlist (format nil
  "tt<point f="~a" d="~a"/>n"
  (float (car(nth i glist)))
  (cdr(nth i glist))))))

and then adding the and tags like this:

;; Output results to xml file
(format fp 
  "<equalizationeffect>nt<curve name=~
  "~a">n~at</curve>n</equalizationeffect>"
  curname pointlist)
(close fp)

and for the screen dump:

;; gain list for screen output
(setq screengain "") ;initialise string
(dotimes (i 17)
  (setq screengain (strcat screengain (format nil
  " ~a Hzt ~a dB   t ~a Hz       ~a dB~%"
  (car(nth i glist))
  (cdr(nth i glist))
  (car(nth (+ 17 i) glist))
  (cdr(nth (+ 17 i) glist))))))

The only slight drawback I can see with this method is that we need to round the 31.5 Hz to either 31 or 32 so that it does not mess up the formatting of the screen dump, though if that is considered to be a problem we could modify the screendump code so that it prints all values with a specified number of decimal places, for example, instead of

(car(nth i glist))

we could have something like:

(decimal (car(nth i glist)) 1)

where the function (decimal value places) is defined as:

(defun decimal (val places)
  (let* ((places (format nil "~a" places))
        (ff (strcat "%#1." places "f")))
    (setq *float-format*  ff)
    (format nil "~a" val)))

I’m not sure if we really need to support float values for frequency. What do you think?


The output path verification part of the code can be really useful for other plug-ins so I think that it would be good to separate this out as a function that can be reused in other plug-ins. I’ve not done this yet.


Thanks, Steve,

Quite a bit to look at there - I’ll get back to you when I’ve gone through it.

POL

Don’t waste too much time struggling to decipher my disjointed notes :wink: .
Here’s the suggestions in context.
78EQCurveGenV2-7_20120104.ny (12.7 KB)
(I’ve still not done anything about putting the output path verification part of the code into a reusable function).

Steve,

I’ve gone through your changes, and it all makes good sense. However, I noticed a couple of things when I did a bit of testing;

If the “10 kHz Rolloff” parameter is zero, or the input box is left blank, we get a “divide by zero” error, so the function “gencurve” has to be

(if (= 0.0 tenkrolloff)
    (setq hfto 10000000.0)
    (setq hfto 
      (sqrt 
        (/ (sqrd 10000.0) 
          (- (power 10.0 (/ (- tenkrolloff) 10.0)) 1.0)))))

The formatting of the line which writes the input parameters to the screen came out wrong on my machine, (it didn’t print a tab between the “10kHz Rolloff” value and the “LF Shelving” value) so I changed it from two lines

10kHz Rollofft LF Shelvingn ~a Hz    t ~a dB   ~
t ~a Hznn Normalisation Factor ~a dBnn Freqt ~

to one line

10kHz Rollofft LF Shelvingn ~a Hz    t ~a dB   t ~a Hznn Normalisation Factor ~a dBnn Freqt ~

which seems to work, though I’m not sure why.

One other question; in the mode selection code at the end, why 1, 2, 3, 4, T?

I’ll do a bit more later and see if anything else breaks.

POL

Good catch.

Ah yes, I see what’s happening there.
It was difficult for me to spot because the spacing looks a bit off anyway on my machine (different font and character spacing I presume).

The tilde (~) at the end of a line allows long lines of text to be split without creating a new line in the output. It also ignores any leading spaces on the next line, so:

    (format nil "Hello world, Hello world, Hello world, ~
       Hello world, Hello world, Hello world, "

will print the same as:

    (format nil "Hello world, Hello world, Hello world, ~
Hello world, Hello world, Hello world, "

and the same as:

    (format nil "Hello world, Hello world, Hello world, Hello world, Hello world, Hello world, "

What I was not aware of was that tabs (t) are treated in the same way as spaces. In fact all leading white-space characters on the next line are ignored.
This is documented here: XLISP format

~ > - continue the ‘format’ string on the next line. This signals a line break in the format. The ‘format’ function will ignore all white-space [blanks, tabs, newlines]. This is useful when the ‘format’ string is longer than a program line. Note that the ‘newline’ character must immediately follow the tilde character.

If it looks better to split the line, this should work:

10kHz Rollofft LF Shelvingn ~a Hz    t ~a dB   t ~a ~
Hznn Normalisation Factor ~a dBnn Freqt ~

or this:

10kHz Rollofft LF Shelvingn ~a Hz    t ~a ~
dB   t ~a Hznn Normalisation Factor ~a dBnn Freqt ~



You mean the “T” rather than “5”?
It will work just the same with “5”.
T represents ‘true’, as opposed to NIL , representing ‘false’.
It’s often convenient to end a “case” selection with “T” as a catch-all, especially while developing the code, as it ensures that you will never drop off the end of the list.
More about “case” here: XLISP case

Does this look right on Windows POL?

      ;; Output results to screen
      (format NIL "EQ Curve:  ~a, written ton~a~an~ann Bass ~
        Turnovert 10kHz Rollofft LF Shelvingn ~a Hz    t~
        ~a dB   t ~a Hznn Normalisation Factor ~a dBnn Freqt ~
        Gaintt Freqt     Gainn~a"
        curname path filen errmessg tofreq tenkrolloff
        lfshelf normfactor screengain))))

Yes, that looks fine.

There’s a question over the terminology of the LF Shelving Filter, but I’m in a rush now. I’ll post later.

POL

As I was saying …

I’ve come to realise that the term I used for the low frequency component, “LF Shelving Filter” is not strictly accurate, since it has a 6dB/octave rolloff, while a “Shelving” filter has a constant gain beyond the turnover frequency. The problen is that I’m not sure what to call it, since there doesn’t seem to be a standard term for it, at least in the literature Ive read. It could maybe be called “Low Bass Turnover” or “Rumble Filter Turnover” since its purpose is to overcome low-frequency rumble by boosting the bass at around 50 to 100Hz when recording. Any other suggestions? Or should we leave it as it is?

POL

I don’t think that it is a big problem as the effect of this part of the filter does tend to have a “shelf-like” response.

I think that the term “low cut” would be technically more accurate, but somewhat misleading for most users because although it is reducing the gain of low frequencies, the “Bass Turnover” part of the filter is causing the absolute gain level to continue rising as the frequency gets lower. Similarly with “rumble filter” one would probably expect a marked attenuation of low frequencies, which does not happen.

I think it’s probably as well to leave it as it is.

Just looking at this bit:

;; Test the file path (which may be the home directory or a user-defined path).
;; If there is no / (or ) at the end of the path, we need to add one so that the screen display looks right.
;; To do this, we add one anyway, then search for // (or \).
;; Depending on the result, we use the original path or the modified one.

(setq fwdslash (string-search "/" path))
(if (not fwdslash)
(setq pathplus (strcat path "\"))
(setq pathplus (strcat path "/"))
)
(if (not fwdslash)
(setq dblslash (string-search "\\" pathplus))
(setq dblslash (string-search "//" pathplus))
)
(when (not dblslash)
(setq path pathplus)
)

Could we just use this (does it work on Windows)?

;; Ensure one file separator at end of path
(setq path (strcat 
  (string-right-trim "\/" path)
  (format nil "~a" *file-separator*)))

Function to test if Windows OS:

;;; Check if Windows
(defun windowsp ()
  (char= #\ *file-separator*))

Suggestion for additional validation of the file name:

;;; Check file name. Return T if valid
(defun valid-file-name (fname)
  (if (windowsp)    ;if Windows OS
    (if 
      ;; invalid Windows names and characters
      (or (string-equal (string-trim ". " fname) "") ;not all spaces, all dots or empty
          (string-equal fname "CON")  (string-equal fname "PRN")
          (string-equal fname "AUX")  (string-equal fname "NUL")
          (string-equal fname "COM1") (string-equal fname "COM2")
          (string-equal fname "COM3") (string-equal fname "COM4")
          (string-equal fname "COM5") (string-equal fname "COM6")
          (string-equal fname "COM7") (string-equal fname "COM8")
          (string-equal fname "COM9") (string-equal fname "LPT1")
          (string-equal fname "LPT2") (string-equal fname "LPT3")
          (string-equal fname "LPT4") (string-equal fname "LPT5")
          (string-equal fname "LPT6") (string-equal fname "LPT7")
          (string-equal fname "LPT8") (string-equal fname "LPT9")
          (string-search "/" fname)   (string-search "\" fname)
          (string-search "?" fname)   (string-search "<" fname)
          (string-search ">" fname)   (string-search ":" fname)
          (string-search """ fname)  (string-search "|" fname)
          (string-search "*" fname)   (string-search ">" fname))
      nil T)
    ;; else linux / Mac
    (if (string-search "/" fname) nil t)))

We don’t need to test Linux / Mac for nul string as we will be adding a file extension.
“.txt” is a valid nix file name.

Yes it does. Much neater, I agree.
I also like the function to check if Windows is the OS.

Just looking at the filename validation code, does that cover all cases? Are there no reserved names on Mac or Linux?
I seem to remember we discussed this some time back, and the consensus was that making the check comprehensive might be too big a job, but if we can cover everything, I’d say include it.

POL

I think so (unless I’ve missed any)

On Linux the restrictions are: https://forum.audacityteam.org/t/unix-linux-file-names/22693/6

I thought that, being based on UNIX it was the same on Mac, but apparently there are a couple of other restrictions on Mac.
“:” should not be used as it is sometimes used to denote a file separator.
It is advisable to not start a file or folder name with a dot. Files / folders beginning with a dot are hidden on both Linux and Mac, but Mac makes more of a fuss about it.

I’ll look into this.

We probably don’t want the file to be hidden on Mac or Linux, so I’ve disallowed names starting with a dot.
We probably don’t want an empty, or “all spaces” file name either, even though these are legal before a file extension.
For Mac I’ve disallowed colons (:slight_smile:

I’ve only tested this on Linux.
I don’t have a Mac to test this on.

;;; Check if Windows
(defun windowsp ()
  (char= #\ *file-separator*))

;;; Check if Mac
(defun Macp ()
  (string-equal (subseq (get-env "HOME") 0 7) "/Users/"))

;;; Check file name. Return T if valid
(defun valid-file-name (fname)
  (if (windowsp)    ;if Windows OS
    (if 
      ;; invalid Windows names and characters
      (or (string-equal (string-trim ". " fname) "") ;not all spaces, all dots or empty
          (string-equal fname "CON")  (string-equal fname "PRN")
          (string-equal fname "AUX")  (string-equal fname "NUL")
          (string-equal fname "COM1") (string-equal fname "COM2")
          (string-equal fname "COM3") (string-equal fname "COM4")
          (string-equal fname "COM5") (string-equal fname "COM6")
          (string-equal fname "COM7") (string-equal fname "COM8")
          (string-equal fname "COM9") (string-equal fname "LPT1")
          (string-equal fname "LPT2") (string-equal fname "LPT3")
          (string-equal fname "LPT4") (string-equal fname "LPT5")
          (string-equal fname "LPT6") (string-equal fname "LPT7")
          (string-equal fname "LPT8") (string-equal fname "LPT9")
          (string-search "/" fname)   (string-search "\" fname)
          (string-search "?" fname)   (string-search "<" fname)
          (string-search ">" fname)   (string-search ":" fname)
          (string-search """ fname)  (string-search "|" fname)
          (string-search "*" fname)   (string-search ">" fname))
      nil T)
    ;; else linux / Mac
    (if 
      (or (string-search "/" fname)
          (string-equal (string-trim " " fname) "")  ;not all spaces or empty
          (string-equal (subseq fname 0 1) ".")     ;not a hidden file
          (and (macp)(string-search ":" fname)))    ;dot contain ':' on Mac        
      nil t)))

I’ve made the validator into an Analyze plug-in for easier testing.

OBSOLETE VERSION:
file-name-validator.ny (2.13 KB)
NEW VERSION:
file-name-validator.ny (2.1 KB)
(Extra “>” check removed)

Looks like we’re good to go with the validator. https://forum.audacityteam.org/t/could-someone-test-this-on-mac-and-windows-please-solved/22956/1
Just need to fit it into the “Save File” function now.

Neither do I, but although “/” is the file separator on Mac it is translated internally by the OS so should be accepted. You can save a filename including forward slash in Save or Export in Audacity (not export multiple).

Why do you have (string-search “>” fname) twice in the Windows exceptions?




Gale

Thanks Gale, I didn’t know that.
Has the current 78rpm EQ Curve Generator been tested with a “/” in the file name?
If it’s confirmed that Nyquist can save files that contain a “/” in the name then I’ll change the validator function.


That’s an error (though a harmless one). I’ll fix that now. [Update: fixed]