From a719ca4b8101ee2eff9f8f43bb29b082bc52eb10 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Mon, 11 Mar 2024 19:22:45 +0000
Subject: [PATCH 01/12] xml: warn on use of gainUnit

closes #66
---
 ear/fileio/adm/test/test_xml.py | 12 ++++++++----
 ear/fileio/adm/xml.py           |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ear/fileio/adm/test/test_xml.py b/ear/fileio/adm/test/test_xml.py
index 717daeb..d416d21 100644
--- a/ear/fileio/adm/test/test_xml.py
+++ b/ear/fileio/adm/test/test_xml.py
@@ -28,6 +28,9 @@
 from ..generate_ids import generate_ids
 
 
+pytestmark = pytest.mark.filterwarnings("ignore:use of gainUnit .*")
+
+
 ns = "urn:ebu:metadata-schema:ebuCore_2015"
 nsmap = dict(adm=ns)
 E = ElementMaker(namespace=ns, nsmap=nsmap)
@@ -223,10 +226,11 @@ def test_gain(base):
     )
 
     # db
-    assert (
-        base.bf_after_mods(set_version(2), add_children(bf_path, E.gain("20", gainUnit="dB"))).gain
-        == 10.0
-    )
+    with pytest.warns(UserWarning, match="gainUnit"):
+        assert (
+            base.bf_after_mods(set_version(2), add_children(bf_path, E.gain("20", gainUnit="dB"))).gain
+            == 10.0
+        )
     expected = "gainUnit must be linear or dB, not 'DB'"
     with pytest.raises(ParseError, match=expected):
         base.bf_after_mods(set_version(2), add_children(bf_path, E.gain("20", gainUnit="DB")))
diff --git a/ear/fileio/adm/xml.py b/ear/fileio/adm/xml.py
index 293a41e..f93f215 100644
--- a/ear/fileio/adm/xml.py
+++ b/ear/fileio/adm/xml.py
@@ -538,6 +538,7 @@ def parse_gain(gain_str, gainUnit):
     if gainUnit == "linear":
         return gain_num
     elif gainUnit == "dB":
+        warnings.warn("use of gainUnit may not be compatible with older software")
         return math.pow(10, gain_num / 20.0)
     else:
         raise ValueError(f"gainUnit must be linear or dB, not {gainUnit!r}")

From 91fbdded3e2e1a006679f73f23b05088beaee336 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Tue, 12 Mar 2024 16:33:55 +0000
Subject: [PATCH 02/12] fileio: improve handling of wav files with
 missing/malformed ADM

- no AXML or CHNA: this used to throw an exception when accessing
  chna.audioIDs; now .adm is None

- AXML present, but missing CHNA chunk: same error; now throws a more
  meaningful error as this is not valid
---
 ear/fileio/bw64/test/test_wav.py | 26 ++++++++++++++++++++++++++
 ear/fileio/utils.py              | 25 +++++++++++++++++++++----
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/ear/fileio/bw64/test/test_wav.py b/ear/fileio/bw64/test/test_wav.py
index 4f06bf4..09673f5 100644
--- a/ear/fileio/bw64/test/test_wav.py
+++ b/ear/fileio/bw64/test/test_wav.py
@@ -211,3 +211,29 @@ def test_openBw64Adm():
         assert infile.adm is not None
         assert len(infile.adm.audioProgrammes) == 1  # loaded axml
         assert infile.adm.audioTrackUIDs[0].trackIndex == 1  # loaded chna
+
+
+def test_openBw64Adm_plain_wav(tmpdir):
+    """check that reading a plain wav results in .adm being None"""
+    fname = tmpdir / "plain_wav.wav"
+
+    with openBw64(fname, "w"):
+        pass
+
+    with openBw64Adm(fname) as f:
+        assert f.adm is None
+
+
+def test_openBw64Adm_no_chna(tmpdir):
+    """check that parsing a BW64 with AXML but no CHNA raises an error"""
+    fname = tmpdir / "no_chna.wav"
+
+    with openBw64(fname, "w", axml=b"axml"):
+        pass
+
+    with pytest.raises(
+        ValueError,
+        match="if 'axml' chunk is present, 'chna' must be too",
+    ):
+        with openBw64Adm(fname):
+            pass
diff --git a/ear/fileio/utils.py b/ear/fileio/utils.py
index d0f43f7..fac780c 100644
--- a/ear/fileio/utils.py
+++ b/ear/fileio/utils.py
@@ -63,7 +63,11 @@ class Bw64AdmReader(object):
     to create these.
 
     Attributes:
-        adm (ADM): ADM data
+        adm (Optional[ADM]): ADM data, or None if CHNA is not present
+
+    Note:
+        This throws if 'axml' is present but 'chna' is not, as this is not
+        valid according to BS.2088-1.
     """
 
     def __init__(self, bw64FileHandle, fix_block_format_durations=False):
@@ -124,11 +128,24 @@ def iter_sample_blocks(self, blockSize):
         return self._bw64.iter_sample_blocks(blockSize)
 
     def _parse_adm(self):
+        axml = self._bw64.axml
+        chna = self._bw64.chna
+
+        if axml is not None and chna is None:
+            raise ValueError("if 'axml' chunk is present, 'chna' must be too")
+
+        if chna is None:
+            return None
+
         adm = ADM()
         load_common_definitions(adm)
-        if self._bw64.axml is not None:
+        if axml is not None:
             self.logger.info("Parsing")
-            load_axml_string(adm, self._bw64.axml, fix_block_format_durations=self._fix_block_format_durations)
+            load_axml_string(
+                adm, axml, fix_block_format_durations=self._fix_block_format_durations
+            )
             self.logger.info("Parsing done!")
-        load_chna_chunk(adm, self._bw64.chna)
+
+        load_chna_chunk(adm, chna)
+
         return adm

From 5376da1139dd19df6a7d67465395e75e43725a94 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Tue, 12 Mar 2024 16:37:46 +0000
Subject: [PATCH 03/12] cmdline: raise an exception when there is no ADM data

---
 ear/cmdline/render_file.py |  5 +++++
 ear/test/test_integrate.py | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/ear/cmdline/render_file.py b/ear/cmdline/render_file.py
index e9fa2b0..9f46876 100644
--- a/ear/cmdline/render_file.py
+++ b/ear/cmdline/render_file.py
@@ -197,6 +197,11 @@ def run(self, input_file, output_file):
         output_monitor = PeakMonitor(n_channels)
 
         with openBw64Adm(input_file) as infile:
+            if infile.adm is None:
+                raise Exception(
+                    f"error: {input_file!r} does not have ADM metadata (missing 'chna' chunk)"
+                )
+
             if version_at_least(infile.adm.version, 2):
                 warnings.warn(
                     f"rendering of files with version {infile.adm.version} is not standardised"
diff --git a/ear/test/test_integrate.py b/ear/test/test_integrate.py
index ef1c4e2..d4ce6d0 100644
--- a/ear/test/test_integrate.py
+++ b/ear/test/test_integrate.py
@@ -141,6 +141,17 @@ def test_render_v2(tmpdir):
     npt.assert_allclose(samples, expected, atol=1e-6)
 
 
+def test_plain_wav(tmpdir):
+    rendered_file = str(tmpdir / "test_plain_wav_out.wav")
+    args = ["ear-render", "-s", "0+5+0", wav_file, rendered_file]
+    proc = subprocess.run(args, capture_output=True)
+
+    assert proc.returncode != 0
+
+    [err_no_adm] = [line for line in proc.stderr.split(b"\n") if line.strip()]
+    assert b"does not have ADM metadata" in err_no_adm
+
+
 @pytest.mark.parametrize("order", [1, 2])
 @pytest.mark.parametrize("chna_only", [False, True])
 def test_hoa(tmpdir, order, chna_only):

From e245bb9e949c3e0ddd97bff1a332eaf93fae562f Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Tue, 12 Mar 2024 18:45:35 +0000
Subject: [PATCH 04/12] doc: avoid html_static_path warning

---
 doc/conf.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/conf.py b/doc/conf.py
index 0631f64..e3c9891 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -59,7 +59,7 @@
 # Add any paths that contain custom static files (such as style sheets) here,
 # relative to this directory. They are copied after the builtin static files,
 # so a file named "default.css" will overwrite the builtin "default.css".
-html_static_path = ["_static"]
+# html_static_path = ["_static"]
 
 intersphinx_mapping = {
     "python": ("https://docs.python.org/3", None),

From 67e16718a157971afd3f1d237bb0229501cc59b9 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Tue, 12 Mar 2024 18:46:28 +0000
Subject: [PATCH 05/12] doc: use :ref: to make references look better

---
 doc/usage.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/usage.rst b/doc/usage.rst
index a2128f6..938c93a 100644
--- a/doc/usage.rst
+++ b/doc/usage.rst
@@ -37,7 +37,7 @@ the result in ``output_surround.wav``.
    :nodescription:
 
    -l, --layout
-       See speakers_file_.
+       See :ref:`speakers_file`.
 
 .. _ear-utils:
 
@@ -53,7 +53,7 @@ ADM files as sub-commands.
    :prog: ear-utils
 
    --screen
-       See speakers_file_.
+       See :ref:`speakers_file`.
 
 .. _speakers_file:
 

From 71d12955f869a5aa9e19eda530c3019c6435f751 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Tue, 12 Mar 2024 18:47:02 +0000
Subject: [PATCH 06/12] doc: add details of output format

closes #64
---
 doc/usage.rst | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/doc/usage.rst b/doc/usage.rst
index 938c93a..218b9b6 100644
--- a/doc/usage.rst
+++ b/doc/usage.rst
@@ -28,7 +28,8 @@ To render an ADM file, the following three parameters must be given:
 
 For example, ``ear-render -s 0+5+0 input.wav output_surround.wav`` will render
 the BW64/ADM file ``input.wav`` to a ``0+5+0`` target speaker layout and store
-the result in ``output_surround.wav``.
+the result in ``output_surround.wav``. See :ref:`output_format` for details of
+the output file format.
 
 .. argparse::
    :module: ear.cmdline.render_file
@@ -55,6 +56,30 @@ ADM files as sub-commands.
    --screen
        See :ref:`speakers_file`.
 
+.. _output_format:
+
+Output Format
+-------------
+
+The output of ``ear-render`` is a BW64 file containing one channel for each
+loudspeaker in the specified layout.
+
+The channel order is the same as in the "Loudspeaker configuration" tables in
+BS.2051-2 (e.g. table 4 for 0+5+0).
+
+The output may need further processing before being played back on loudspeakers.
+
+In particular, the renderer does not do any bass management -- LFE channels in
+the output must be treated according to section 3 or 4 of attachment 1 to annex
+7 of BS.775-4. This includes the addition of a 10dB gain, and routing to
+loudspeakers or a subwoofer.
+
+The renderer also does not apply any kind of loudspeaker distance compensation
+(beyond the gain which may be specified in the speakers file), or EQ.
+
+To illustrate this, if the input to the renderer exactly matches the output
+loudspeaker layout, then the output will be identical to the input.
+
 .. _speakers_file:
 
 Speakers File Format
@@ -100,7 +125,8 @@ The possible keys are as follows:
     A mapping containing the real loudspeaker position, with keys ``az``,
     ``el`` and ``r`` specifying the azimuth, elevation and distance of the
     loudspeaker in ADM angle format (anticlockwise azimuth, degrees) and
-    metres.
+    metres. Note that the radius specified is not used to apply distance
+    compensation.
 
 ``gain_linear`` (optional)
     A linear gain to apply to this output channel; this is useful for LFE

From eedd306a0c82a01d848ec7ac290483d77b2f07a2 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Tue, 12 Mar 2024 18:48:06 +0000
Subject: [PATCH 07/12] nix: add documentation requirements

---
 flake.nix | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/flake.nix b/flake.nix
index 25d210d..1dd30d6 100644
--- a/flake.nix
+++ b/flake.nix
@@ -52,6 +52,11 @@
               python3.pkgs.pip
               packages.darker
               python3.pkgs.venvShellHook
+
+              # for building docs
+              python3.pkgs.sphinx
+              python3.pkgs.sphinx-rtd-theme
+              pkgs.graphviz
             ];
             venvDir = "./venv";
             postShellHook = ''

From c4b272411160fbacdd3de16d54bbf852cdf32069 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Mon, 18 Mar 2024 17:15:32 +0000
Subject: [PATCH 08/12] fix wording in docs

---
 ear/fileio/adm/chna.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ear/fileio/adm/chna.py b/ear/fileio/adm/chna.py
index 6a5f120..6ea9255 100644
--- a/ear/fileio/adm/chna.py
+++ b/ear/fileio/adm/chna.py
@@ -131,7 +131,7 @@ def populate_chna_chunk(chna, adm):
     generated before calling this.
 
     Parameters:
-        adm (ADM): adm structure to get information to
+        adm (ADM): adm structure to get information from
         chna (ChnaChunk): chna chunk to populate
     """
     chna.audioIDs = list(_get_chna_entries(adm))

From b6561aafd9071ec571b83315b42912caea846f56 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Mon, 18 Mar 2024 17:15:54 +0000
Subject: [PATCH 09/12] fix zero trackIndex in validation test

---
 ear/core/select_items/test/test_validation.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ear/core/select_items/test/test_validation.py b/ear/core/select_items/test/test_validation.py
index a99a20f..ad8bb96 100644
--- a/ear/core/select_items/test/test_validation.py
+++ b/ear/core/select_items/test/test_validation.py
@@ -456,7 +456,7 @@ def test_hoa_pack_format_mismatch():
 
     builder.first_pack.absoluteDistance = 2.0
 
-    for i, track in enumerate(builder.first_tracks + builder.second_tracks):
+    for i, track in enumerate(builder.first_tracks + builder.second_tracks, 1):
         builder.create_track_uid(
             audioPackFormat=builder.second_pack, audioTrackFormat=track, trackIndex=i
         )

From d6cd2a51ad6849c889c3b52202f8a5334d066226 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Mon, 18 Mar 2024 17:16:38 +0000
Subject: [PATCH 10/12] validate that trackIndex is a non-zero int

---
 ear/fileio/adm/elements/main_elements.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ear/fileio/adm/elements/main_elements.py b/ear/fileio/adm/elements/main_elements.py
index 4b83e58..ae04a44 100644
--- a/ear/fileio/adm/elements/main_elements.py
+++ b/ear/fileio/adm/elements/main_elements.py
@@ -1,5 +1,5 @@
 from attr import attrs, attrib, Factory, validate
-from attr.validators import instance_of, optional
+from attr.validators import and_, gt, instance_of, optional
 from enum import Enum
 from fractions import Fraction
 from six import string_types
@@ -511,7 +511,7 @@ class AudioTrackUID(ADMElement):
         audioPackFormat (Optional[AudioPackFormat])
     """
 
-    trackIndex = attrib(default=None)
+    trackIndex = attrib(default=None, validator=optional(and_(instance_of(int), gt(0))))
     sampleRate = attrib(default=None)
     bitDepth = attrib(default=None)
     audioTrackFormat = attrib(default=None, repr=False,

From 21053a24f06496a807bfba8fd3a2965fde892f23 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Mon, 18 Mar 2024 17:17:32 +0000
Subject: [PATCH 11/12] add utility to validate trackIndex values

---
 ear/fileio/adm/chna.py           | 17 +++++++++++++++++
 ear/fileio/adm/test/test_chna.py | 21 ++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/ear/fileio/adm/chna.py b/ear/fileio/adm/chna.py
index 6ea9255..1a00652 100644
--- a/ear/fileio/adm/chna.py
+++ b/ear/fileio/adm/chna.py
@@ -157,3 +157,20 @@ def guess_track_indices(adm):
             raise Exception("Invalid track UID {}.".format(track_uid.id))
 
         track_uid.trackIndex = int(match.group(1), 16)
+
+
+def validate_trackIndex(adm, num_channels):
+    """Check that all audioTrackUIDs in adm have a trackIndex that is valid in
+    a file with num_channels.
+
+    Parameters:
+        adm (ADM): adm structure containing audioTrackUIDs to check
+        num_channels (int): number of channels in the BW64 file
+    """
+    for track_uid in adm.audioTrackUIDs:
+        if track_uid.trackIndex is not None and track_uid.trackIndex > num_channels:
+            tracks = "track" if num_channels == 1 else "tracks"
+            raise Exception(
+                f"audioTrackUID {track_uid.id} has track index {track_uid.trackIndex} "
+                f"(1-based) in a file with {num_channels} {tracks}"
+            )
diff --git a/ear/fileio/adm/test/test_chna.py b/ear/fileio/adm/test/test_chna.py
index 3e7e1a5..921064f 100644
--- a/ear/fileio/adm/test/test_chna.py
+++ b/ear/fileio/adm/test/test_chna.py
@@ -1,7 +1,7 @@
 import pytest
 from ...bw64.chunks import AudioID, ChnaChunk
 from ..adm import ADM
-from ..chna import load_chna_chunk, populate_chna_chunk
+from ..chna import load_chna_chunk, populate_chna_chunk, validate_trackIndex
 from ..common_definitions import load_common_definitions
 from ..elements import AudioTrackUID
 
@@ -245,3 +245,22 @@ def test_both_track_channel(self, adm, chna):
         expected = "Track UID ATU_00000001 has both track and channel formats."
         with pytest.raises(Exception, match=expected):
             populate_chna_chunk(chna, adm)
+
+
+def test_validate_trackIndex(adm):
+    atu = AudioTrackUID(id="ATU_00000001", trackIndex=1)
+    adm.addAudioTrackUID(atu)
+
+    # no error
+    validate_trackIndex(adm, 1)
+
+    # plural
+    expected = r"audioTrackUID ATU_00000001 has track index 1 \(1-based\) in a file with 0 tracks"
+    with pytest.raises(Exception, match=expected):
+        validate_trackIndex(adm, 0)
+
+    # singular
+    atu.trackIndex = 2
+    expected = r"audioTrackUID ATU_00000001 has track index 2 \(1-based\) in a file with 1 track"
+    with pytest.raises(Exception, match=expected):
+        validate_trackIndex(adm, 1)

From a04e287f247809653a031b7c99306066ec3d94c1 Mon Sep 17 00:00:00 2001
From: Thomas Nixon <tom@tomn.co.uk>
Date: Mon, 18 Mar 2024 17:18:11 +0000
Subject: [PATCH 12/12] fileio: run trackIndex validation in Bw64AdmReader

fixes #30
---
 ear/fileio/utils.py        |  3 ++-
 ear/test/test_integrate.py | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/ear/fileio/utils.py b/ear/fileio/utils.py
index fac780c..09c1f53 100644
--- a/ear/fileio/utils.py
+++ b/ear/fileio/utils.py
@@ -3,7 +3,7 @@
 from .adm.adm import ADM
 from .adm.xml import load_axml_string
 from .adm.common_definitions import load_common_definitions
-from .adm.chna import load_chna_chunk
+from .adm.chna import load_chna_chunk, validate_trackIndex
 
 
 def openBw64(filename, mode='r', **kwargs):
@@ -147,5 +147,6 @@ def _parse_adm(self):
             self.logger.info("Parsing done!")
 
         load_chna_chunk(adm, chna)
+        validate_trackIndex(adm, self.channels)
 
         return adm
diff --git a/ear/test/test_integrate.py b/ear/test/test_integrate.py
index d4ce6d0..90fdf51 100644
--- a/ear/test/test_integrate.py
+++ b/ear/test/test_integrate.py
@@ -152,6 +152,41 @@ def test_plain_wav(tmpdir):
     assert b"does not have ADM metadata" in err_no_adm
 
 
+def test_bad_track_index(tmpdir):
+    from ..fileio import openBw64
+    from ..fileio.bw64.chunks import AudioID, ChnaChunk, FormatInfoChunk
+
+    bad_track_index = str(tmpdir / "bad_track_index.wav")
+    rendered_file = str(tmpdir / "bad_track_index_out.wav")
+
+    chna = ChnaChunk()
+    chna.audioIDs.append(
+        AudioID(
+            trackIndex=2,
+            audioTrackUID="ATU_00000001",
+            audioTrackFormatIDRef="AT_00010003_01",
+            audioPackFormatIDRef="AP_00010001",
+        )
+    )
+
+    samples = generate_samples()[:, :1]
+    fmtInfo = FormatInfoChunk(
+        formatTag=1, channelCount=samples.shape[1], sampleRate=sr, bitsPerSample=24
+    )
+
+    with openBw64(bad_track_index, "w", chna=chna, formatInfo=fmtInfo) as outfile:
+        outfile.write(samples)
+
+    args = ["ear-render", "-s", "0+5+0", bad_track_index, rendered_file]
+    proc = subprocess.run(args, capture_output=True)
+
+    assert proc.returncode != 0
+
+    [err_bad_track] = [line for line in proc.stderr.split(b"\n") if line.strip()]
+    assert b"ATU_00000001 has track index 2" in err_bad_track
+    assert b"in a file with 1 track" in err_bad_track
+
+
 @pytest.mark.parametrize("order", [1, 2])
 @pytest.mark.parametrize("chna_only", [False, True])
 def test_hoa(tmpdir, order, chna_only):