Address reviewer feedback.

This patch updates the PR with:

    - requested stylistic changes
    - addresses numerous comments

I still need to investigate the use of `AllocatedArray` in
class `SoundInfoCache`.

src/output/plugins/meson.build

@@ -163,14 +163,17 @@ endif
 libfftw3_dep = dependency('fftw3f', version: '>= 3.3.10', required: get_option('fftw3'))
 output_features.set('ENABLE_FFTW3', libfftw3_dep.found())
 output_features.set('ENABLE_VISUALIZATION_OUTPUT', get_option('visualization'))
-if get_option('visualization') and libfftw3_dep.found()
+if get_option('visualization')
+  if not libfftw3_dep.found()
+    error('libfftw3 not available, but is required for the visualization plugin')
+  endif
   output_plugins_sources += [
     'visualization/VisualizationOutputPlugin.cxx',
     'visualization/SoundAnalysis.cxx',
     'visualization/SoundInfoCache.cxx',
     'visualization/VisualizationServer.cxx',
     'visualization/VisualizationClient.cxx',
-    'visualization/protocol.cxx',
+    'visualization/Protocol.cxx',
   ]
   output_plugins_deps += [ event_dep, net_dep, libfftw3_dep ]
 endif

src/output/plugins/visualization/LowLevelProtocol.hxx

@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright The Music Player Daemon Project
+
+#ifndef LOW_LEVEL_PROTOCOL_HXX_INCLUDED
+#define LOW_LEVEL_PROTOCOL_HXX_INCLUDED
+
+#include "util/PackedBigEndian.hxx"
+
+#include <fftw3.h>
+
+#include <algorithm>
+#include <cstdint>
+#include <limits>
+
+namespace Visualization {
+
+/*  Write a uint16_t to an output iterator over byte in wire format; return the
+ *  iterator in its new position
+ */
+template <typename OutIter>
+OutIter
+SerializeU16(uint16_t n, OutIter pout) {
+	auto m = PackedBE16(n);
+	auto p = (std::byte*)(&m);
+	return std::copy(p, p + 2, pout);
+}
+
+static_assert(std::numeric_limits<float>::is_iec559);
+
+/* Convert an IEEE 754 single-precision floating-point number to wire format;
+ * write it to an output iterator & return the iterator in its new position
+ */
+template <typename OutIter>
+OutIter
+SerializeFloat(float f, OutIter pout) {
+	auto m = PackedBE32(*(uint32_t*)&f);
+	auto p = (std::byte*)(&m);
+	return std::copy(p, p + 4, pout);
+}
+
+/* Convert an fftwf_complex to wire format; write it to an output iterator &
+ * return the iterator in its new position
+ */
+template <typename OutIter>
+OutIter
+SerializeComplex(const fftwf_complex c, OutIter pout) {
+	auto r = PackedBE32(*(const uint32_t*)&(c[0]));
+	auto i = PackedBE32(*(const uint32_t*)&(c[1]));
+	auto pr = (std::byte*)(&r);
+	auto pi = (std::byte*)(&i);
+	pout = std::copy(pr, pr + 4, pout);
+	return std::copy(pi, pi + 4, pout);
+}
+
+} // namespace Visualization
+
+#endif // LOW_LEVEL_PROTOCOL_HXX_INCLUDED

src/output/plugins/visualization/Protocol.cxx

@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright The Music Player Daemon Project
+
+#include "Protocol.hxx"
+
+#include "Log.hxx"
+#include "util/ByteOrder.hxx"
+#include "util/Domain.hxx"
+
+Visualization::ParseResult
+Visualization::ParseClihlo(void *data,
+			   size_t length,
+			   ClientHello &clihlo) noexcept {
+	// CLIHLO payload is 6 bytes, header & footer are five more.
+	if (length < sizeof(ClientHello) + 5) {
+		return ParseResult::NEED_MORE_DATA;
+	}
+
+	uint8_t *buf = (uint8_t *)data;
+
+	uint16_t msg_type = FromBE16(*(uint16_t *)buf);
+	if (msg_type != 0) {
+		return ParseResult::ERROR;
+	}
+
+	buf += 2;
+	uint16_t payload_len = FromBE16(*(uint16_t *)buf);
+	if (payload_len != 6) {
+		return ParseResult::ERROR;
+	}
+
+	buf += 2;
+	clihlo.major_version = *buf++;
+	clihlo.minor_version = *buf++;
+
+	clihlo.requested_fps = FromBE16(*(uint16_t *)(buf));
+	buf += 2;
+	clihlo.tau = FromBE16(*(int16_t *)(buf));
+	buf += 2;
+
+	if (*buf != 0) {
+		return ParseResult::ERROR;
+	}
+
+	return ParseResult::OK;
+}

src/output/plugins/visualization/Protocol.hxx

@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright The Music Player Daemon Project
+
+#ifndef VISUALIZATION_PROTOCOL_HXX_INCLUDED
+#define VISUALIZATION_PROTOCOL_HXX_INCLUDED
+
+#include "LowLevelProtocol.hxx"
+#include "SoundAnalysis.hxx"
+
+#include <cstddef>
+#include <cstdint>
+
+namespace Visualization {
+
+/**
+ * \brief A parsed CLIHLO message
+ *
+ * \sa ParseCliHlo
+ *
+ *
+ * The visualization \ref vis_out_protocol "protocol" begins with the client
+ * connecting to the server & providing certain paramters of the sound analysis
+ * it would like to receive. That is done through the CLIHLO message (which see
+ * \a ref vis_out_protocol_proto_clihlo "here").
+ *
+ * See \a vis_out_protocol_timing "timing" for details on parameter tau.
+ *
+ *
+ */
+
+struct ClientHello {
+	/// Major protocol version the client would like to speak
+	uint8_t major_version;
+	/// Minor protocol version the client would like to speak
+	uint8_t minor_version;
+	/// The number of sound analyses per second the client would like to
+	/// receive (presumably the rate at which it is rendering frames, hence
+	/// the name "fps")
+	uint16_t requested_fps;
+	/// The desired offset (named "tau" in the documentation) between song
+	/// time and analysis time at each analysis performed
+	int16_t tau;
+};
+
+enum class ParseResult {
+	OK,
+	NEED_MORE_DATA,
+	ERROR,
+};
+
+/**
+ * \brief Attempt to parse a \ref vis_out_protocol_proto_clihlo "CLIHLO" message
+ * from the given buffer
+ *
+ * \param buf [in] An array of octets potentially containing the message
+ *
+ * \param length [in] The length of \a buf, in octets
+ *
+ * \param clihlo [out] A reference to a `client_hello_t` structure to be
+ * filled-in on successful execution
+ *
+ * \return ParseResult::OK if the message was successfully parsed,
+ * NEED_MORE_DATA if the message is incomplete, or ERROR if the message cannot
+ * be ready from \a buf
+ *
+ *
+ * CLIHLO is the first message in the protocol, sent by the client. See
+ * \ref vis_out_protocol_proto_clihlo "the protocol specification" for details,
+ * and \ref vis_out_protocol "Visualization Network Protocol" for discussion
+ * of the protocol altogether.
+ *
+ *
+ */
+
+ParseResult
+ParseClihlo(void *buf, size_t length, ClientHello &clihlo) noexcept;
+
+/// Serialize an SRVHLO message to wire format
+template <typename OutIter>
+void
+SerializeSrvhlo(std::byte major_ver, std::byte minor_ver, OutIter pout) {
+	using std::byte;
+
+	*pout++ = byte{0}; //
+	*pout++ = byte{1}; // message type
+	*pout++ = byte{0}; //
+	*pout++ = byte{2}; // payload length
+	*pout++ = major_ver;
+	*pout++ = minor_ver;
+	*pout++ = byte{0}; // check byte
+}
+
+/// Serialize a FRAME message header to wire format
+template <typename OutIter>
+OutIter
+SerializeSoundInfoFrameHeader(uint8_t num_chan,
+			      size_t num_samp,
+			      size_t num_freq,
+ OutIter pout) {
+
+	using std::byte;
+
+	// Start with the "magic number" allowing clients to "lock on" to the
+	// stream of sound info frames in the event of an error.
+	// See \ref vis_out_protocol_proto_msgs for details.
+	*pout++ = byte{0x63};
+	*pout++ = byte{0xac};
+	*pout++ = byte{0x84};
+	*pout++ = byte{0x03};
+
+	*pout++ = byte{16};
+	*pout++ = byte{0};
+
+	return SerializeU16(17 + 4 * num_chan * (num_samp + 3 * num_freq + 3),
+			    pout);
+}
+
+/// Serialize a FRAME message payload to wire format
+template <typename OutIter>
+void
+SerializeSoundInfoFrameFooter(OutIter pout) {
+	*pout = std::byte{0x00};
+}
+
+/// Serialize a FRAME message to wire format
+template <typename OutIter>
+void
+SerializeSoundInfoFrame(const Visualization::SoundAnalysis &a,
+			OutIter pout) {
+	pout = SerializeSoundInfoFrameHeader(a.NumChan(), a.NumSamp(),
+					     a.NumFreq(), pout);
+	pout = a.SerializeSoundInfoFramePayload(pout);
+	SerializeSoundInfoFrameFooter(pout);
+}
+
+} // namespace Visualization.
+
+#endif // VISUALIZATION_PROTOCOL_HXX_INCLUDED