Re: [PATCH] Add: tracktag interface and support for Opus

[Yoni Rabkin]

Grant Shoshin Shangreaux <grant@churls.world> writes:

> Here's a patch I've been using locally to tag Opus files. It could
> certainly use some more quality assurance, it works for me but it hasn't
> been used very extensively.

The emms-tracktag should be changed to emms-info-tracktag in order to be
consistent with the rest of the info methods.

> This wraps the `tracktag` executable from the `audiotools` package
> http://audiotools.sourceforge.net/index.html which should be available
> in GNU/Linux repositories. I am running PureOS backed by Debian and it
> is available to me through apt. It might be useful to find out if it
> can be easily installed on other platforms.

It is available in Trisquel 8.0:

    $ tracktag --version
    Python Audio Tools 3.1.1

> Unfortunately, it does not look to be available for homebrew on MacOS,
> and I'm really not sure about Windows. This is too bad, since an
> easily installed, multi-format tagger is part of the reason I went
> with this tool.

That is unfortunate, but not a show-stopper. We should still add it.

> I've also added a very naive setup for running ERT tests, any feedback
> there would be welcome. I will say, it was pretty useful for me while
> developing, but YMMV.

I think that adding ert testing is fine, but I would only add it after
the code has stabilized somewhat. Since the unit tests are very short,
they can be kept in the same file as the tracktag code for now.

> By the way, I used some real metadata from tracks I have, but afaik that
> is not copyrighted data. If we'd prefer to keep it out of the code base,
> I can replace it.

Indeed, we should not have real metadata in the codebase since this is
what an automated search 'bot would latch onto and potentially cause
trouble.

> If I add any audio files in for testing, I will ensure they are
> properly free. My plan was to just generate some very small files of
> silence in several formats. Whether or not this is useful for testing
> before a release is up for discussion.

That sounds like a fine idea.

> I hope this is useful!

Indeed it is! Thank you.

Based on the volume of code you are adding, I would recommend that you
open a Savannah account (if you don't have one already) and that I give
you write access to the repo. At that point you can do all of your work
in a remote branch.

In a remote branch you can break anything and everything with impunity,
and everyone else can easily grab a copy for testing. We recently did
this with Petteri's info-native branch and it worked well.


> -Grant
>
>>From c80315946e885a523f74d6a66ca7d7fa0682d6c2 Mon Sep 17 00:00:00 2001
> From: Grant Shangreaux <grant@unabridgedsoftware.com>
> Date: Sun, 10 Jan 2021 20:30:35 -0600
> Subject: [PATCH] Add: emms-tracktag wrapper for the audiotools program
>
> http://audiotools.sourceforge.net/ provides a tool `tracktag`
> which handles writing tags for several different audio formats,
> including Opus. this patch provides a very basic wrapper to interface
> between EMMS track info and tracktag metadata. It also configures
> emms-tag-editor.el to use it for writing tags to Opus files.
>
> Add: test.sh and test/test-all.el
>
> Basic setup to run ert tests from the terminal using Emacs in batch
> mode. This should load up only ERT, the tests in the test/ directory
> and their dependencies.
>
> Add: opus extension to emms-libtag-known-extensions
>
> libtag works as a reader for opus files, this just ads the opus
> extension to the regexp list.
> ---
>  emms-info-libtag.el        |   2 +-
>  emms-tag-editor.el         |   4 +-
>  emms-tracktag.el           |  71 ++++++++++++++++++++++++++
>  test.sh                    |   3 ++
>  test/emms-tracktag-test.el | 101 +++++++++++++++++++++++++++++++++++++
>  test/test-all.el           |   6 +++
>  6 files changed, 185 insertions(+), 2 deletions(-)
>  create mode 100644 emms-tracktag.el
>  create mode 100755 test.sh
>  create mode 100644 test/emms-tracktag-test.el
>  create mode 100644 test/test-all.el
>
> diff --git a/emms-info-libtag.el b/emms-info-libtag.el
> index fb9c5dd..9425a38 100644
> --- a/emms-info-libtag.el
> +++ b/emms-info-libtag.el
> @@ -74,7 +74,7 @@
>    :type '(string))
>  
>  (defcustom emms-info-libtag-known-extensions
> -  (regexp-opt '("mp3" "mp4" "m4a" "ogg" "flac" "spx" "wma"))
> +  (regexp-opt '("mp3" "mp4" "m4a" "ogg" "flac" "spx" "wma" "opus"))
>    "Regexp of known extensions compatible with 
> `emms-info-libtag-program-name'.
>  
>  Case is irrelevant."
> diff --git a/emms-tag-editor.el b/emms-tag-editor.el
> index 5b6e447..45c90f7 100644
> --- a/emms-tag-editor.el
> +++ b/emms-tag-editor.el
> @@ -37,6 +37,7 @@
>  (require 'emms-info-mp3info)
>  (require 'emms-playlist-mode)
>  (require 'emms-mark)
> +(require 'emms-tracktag)
>  (require 'format-spec)
>  (require 'subr-x)
>  
> @@ -138,7 +139,8 @@ See also `emms-tag-editor-default-parser'.")
>        (info-performer   . "--TOPE")
>        (info-date        . "--TDAT")))
>      ("ogg" . emms-tag-editor-tag-ogg)
> -    ("flac" . emms-tag-editor-tag-flac))
> +    ("flac" . emms-tag-editor-tag-flac)
> +    ("opus" . emms-tracktag-file))
>    "An alist used when committing changes to tags in files.
>  If the external program sets tags by command line options
>  one-by-one, then the list should like:
> diff --git a/emms-tracktag.el b/emms-tracktag.el
> new file mode 100644
> index 0000000..0b8b660
> --- /dev/null
> +++ b/emms-tracktag.el
> @@ -0,0 +1,71 @@
> +;;; emms-tracktag.el --- EMMS interface for audiotools tracktag  -*- 
> lexical-binding: t; -*-
> +
> +;; Copyright (C) 2021  Grant Shoshin Shangreaux
> +
> +;; Author: Grant Shoshin Shangreaux <grant@churls.world>
> +;; Keywords:
> +
> +;; This program is free software; you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.
> +
> +;; This program is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
> +
> +;;; Commentary:
> +
> +;; Provides a wrapper for audiotools tracktag executable
> +;; http://audiotools.sourceforge.net/tracktag.html
> +;; Given an EMMS TRACK structure, it will map the emms-info fields onto
> +;; arguments for tracktag. Then it calls the tracktag process to write the
> +;; info as metadata tags on the track's associated file.
> +
> +;;; Code:
> +
> +(require 'emms)
> +
> +(defvar emms-info-tracktag--info-fields
> +  '((info-album . album)
> +    (info-artist . artist)
> +    (info-composer . composer)
> +    (info-performer . performer)
> +    (info-year . year)
> +    (info-date . year)
> +    (info-tracknumber . number)
> +    (info-discnumber . album-number)
> +    (info-note . comment)
> +    (info-title . name))
> +  "An alist mapping info-* fields to tracktag fields.")
> +
> +(defun emms-tracktag--map-track-info (track)
> +  (seq-filter (lambda (cell) (cdr cell))
> +              (mapcar (lambda (pair)
> +                        (cons (cdr pair) (emms-track-get track (car pair))))
> +                      emms-info-tracktag--info-fields)))
> +
> +(defun emms-tracktag--build-args (track)
> +  (flatten-list
> +   (append (mapcar (lambda (pair)
> +                     (let ((tag (car pair)) (value (cdr pair)))
> +                       (when value
> +                         (if (string-equal value "") (concat "--remove-" 
> (format "%s" tag))
> +                           (concat "--" (format "%s" tag) "=" value)))))
> +                   (emms-tracktag--map-track-info track))
> +           (list (emms-track-name track)))))
> +
> +(defun emms-tracktag-file (track)
> +  (apply #'call-process
> +   "tracktag" nil
> +   (get-buffer-create emms-tag-editor-log-buffer)
> +   nil
> +   "-Vdebug"
> +   (emms-tracktag--build-args track)))
> +
> +(provide 'emms-tracktag)
> +;;; emms-tracktag.el ends here
> diff --git a/test.sh b/test.sh
> new file mode 100755
> index 0000000..50547b1
> --- /dev/null
> +++ b/test.sh
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +
> +emacs -Q --batch --directory="." -l ert -l test/test-all.el -f 
> ert-run-tests-batch-and-exit
> diff --git a/test/emms-tracktag-test.el b/test/emms-tracktag-test.el
> new file mode 100644
> index 0000000..9b66d5e
> --- /dev/null
> +++ b/test/emms-tracktag-test.el
> @@ -0,0 +1,101 @@
> +;; emms-tracktag-test.el --- Unit tests for emms-tracktag module  -*- 
> lexical-binding: t; -*-
> +
> +;; Copyright (C) 2021  Grant Shoshin Shangreaux
> +
> +;; Author: Grant Shoshin Shangreaux <grant@churls.world>
> +;; Keywords:
> +
> +;; This program is free software; you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.
> +
> +;; This program is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
> +
> +;;; Commentary:
> +
> +;; Tests for emms-tracktag.el
> +
> +;;; Code:
> +
> +(require 'emms)
> +(require 'emms-tracktag)
> +
> +(ert-deftest emms-tracktag--map-track-info-test ()
> +  "Ensure mapping of emms info to tracktag fields is correct."
> +  (let ((track '(*track*
> +                 (type . file) (name . "foo")
> +                 (info-album . "The Sounds of the Sounds of Science")
> +                 (info-artist . "Yo La Tengo")
> +                 (info-title . "Sea Urchins")
> +                 (info-date . "2020")
> +                 (info-tracknumber . "1")))
> +        (track2 '(*track*
> +                  (info-composer . "Ira Kaplan, Georgia Hubley, James Mcnew")
> +                  (info-performer . "Yo La Tengo")
> +                  (info-year . "2002")
> +                  (info-discnumber . "1")
> +                  (info-note . "new soundtrack to an old film"))))
> +    (should (seq-set-equal-p
> +             '((album . "The Sounds of the Sounds of Science")
> +               (artist . "Yo La Tengo")
> +               (name . "Sea Urchins")
> +               (year . "2020")
> +               (number . "1"))
> +             (emms-tracktag--map-track-info track)))
> +    (should (seq-set-equal-p
> +             '((composer . "Ira Kaplan, Georgia Hubley, James Mcnew")
> +               (performer . "Yo La Tengo")
> +               (year . "2002")
> +               (album-number . "1")
> +               (comment . "new soundtrack to an old film"))
> +             (emms-tracktag--map-track-info track2)))))
> +
> +(ert-deftest emms-tracktag--build-args-test ()
> +  "Ensure args for tracktag are properly formed."
> +  (let ((track '(*track*
> +                 (type . file) (name . "foo.flac")
> +                 (info-album . "The Sounds of the Sounds of Science")
> +                 (info-artist . "Yo La Tengo")
> +                 (info-title . "Sea Urchins")
> +                 (info-date . "2020")
> +                 (info-tracknumber . "1")
> +                 (info-composer . "Ira Kaplan, Georgia Hubley, James Mcnew")
> +                 (info-performer . "Yo La Tengo")
> +                 (info-discnumber . "1")
> +                 (info-note . "new soundtrack to an old film"))))
> +    (should (seq-set-equal-p
> +             '("--album=The Sounds of the Sounds of Science"
> +               "--artist=Yo La Tengo"
> +               "--name=Sea Urchins"
> +               "--year=2020"
> +               "--number=1"
> +               "--composer=Ira Kaplan, Georgia Hubley, James Mcnew"
> +               "--performer=Yo La Tengo"
> +               "--album-number=1"
> +               "--comment=new soundtrack to an old film"
> +               "foo.flac")
> +             (emms-tracktag--build-args track)))
> +    (let ((track-with-empty-strings (copy-alist track)))
> +      (setcdr (assq 'info-title track-with-empty-strings) "")
> +      (setcdr (assq 'info-note track-with-empty-strings) "")
> +      (should (seq-set-equal-p
> +               '("--album=The Sounds of the Sounds of Science"
> +                 "--artist=Yo La Tengo"
> +                 "--year=2020"
> +                 "--number=1"
> +                 "--composer=Ira Kaplan, Georgia Hubley, James Mcnew"
> +                 "--performer=Yo La Tengo"
> +                 "--album-number=1"
> +                 "--remove-comment"
> +                 "--remove-name"
> +                 "foo.flac")
> +               (emms-tracktag--build-args track-with-empty-strings))))))
> +
> +;;; emms-tracktag-test.el ends here
> diff --git a/test/test-all.el b/test/test-all.el
> new file mode 100644
> index 0000000..36f00c7
> --- /dev/null
> +++ b/test/test-all.el
> @@ -0,0 +1,6 @@
> +;; maybe not needed, but ensures tests run against the latest changes
> +(byte-recompile-directory ".")
> +
> +(mapc #'load
> +      (mapcar (lambda (f) (file-truename (concat "test/" f)))
> +              (seq-filter (lambda (f) (string-match "[^.#]+\\test.el$" f)) 
> (directory-files (file-truename "./test")))))

-- 
   "Cut your own wood and it will warm you twice"