Merge pull request #72 from UniversityRadioYork/mw-more-cleanup

Various clang-format-guided cleanups.

src/audio/audio.cpp

@@ -21,15 +21,15 @@
 #include "audio_sink.hpp"
 #include "audio_source.hpp"
 
-PipeAudio::PipeAudio(AudioSource *source, AudioSink *sink) : source(source), sink(sink)
+PipeAudio::PipeAudio(AudioSource *src, AudioSink *sink) : src(src), sink(sink)
 {
 	this->ClearFrame();
 }
 
 void PipeAudio::Emit(const ResponseSink &sink) const
 {
-	assert(this->source != nullptr);
-	sink.Respond(ResponseCode::FILE, this->source->Path());
+	assert(this->src != nullptr);
+	sink.Respond(ResponseCode::FILE, this->src->Path());
 }
 
 void PipeAudio::Start()
@@ -47,18 +47,17 @@ void PipeAudio::Stop()
 std::uint64_t PipeAudio::Position() const
 {
 	assert(this->sink != nullptr);
-	assert(this->source != nullptr);
+	assert(this->src != nullptr);
 
-	return this->source->MicrosecondPositionFromSamples(
-	                this->sink->Position());
+	return this->src->MicrosFromSamples(this->sink->Position());
 }
 
 void PipeAudio::Seek(std::uint64_t position)
 {
 	assert(this->sink != nullptr);
-	assert(this->source != nullptr);
+	assert(this->src != nullptr);
 
-	auto samples = this->source->Seek(position);
+	auto samples = this->src->Seek(position);
 	this->sink->SetPosition(samples);
 
 	// We might still have decoded samples from the old position in
@@ -76,7 +75,7 @@ void PipeAudio::ClearFrame()
 Audio::State PipeAudio::Update()
 {
 	assert(this->sink != nullptr);
-	assert(this->source != nullptr);
+	assert(this->src != nullptr);
 
 	bool more_frames_available = this->DecodeIfFrameEmpty();
 
@@ -94,7 +93,7 @@ void PipeAudio::TransferFrame()
 {
 	assert(!this->frame.empty());
 	assert(this->sink != nullptr);
-	assert(this->source != nullptr);
+	assert(this->src != nullptr);
 
 	this->sink->Transfer(this->frame_iterator, this->frame.end());
 
@@ -122,8 +121,8 @@ bool PipeAudio::DecodeIfFrameEmpty()
 	// If we still have a frame, don't bother decoding yet.
 	if (!this->FrameFinished()) return true;
 
-	assert(this->source != nullptr);
-	AudioSource::DecodeResult result = this->source->Decode();
+	assert(this->src != nullptr);
+	AudioSource::DecodeResult result = this->src->Decode();
 
 	this->frame = result.second;
 	this->frame_iterator = this->frame.begin();

src/audio/audio.hpp

@@ -50,7 +50,7 @@ class Audio
 		NONE,    ///< There is no Audio.
 		STOPPED, ///< The Audio has been stopped, or not yet played.
 		PLAYING, ///< The Audio is currently playing.
-		AT_END,  ///< The Audio is at its end and cannot play without seeking.
+		AT_END,  ///< The Audio has ended and can't play without a seek.
 	};
 
 	//
@@ -98,8 +98,7 @@ class Audio
 	 * This is usually literally the absolute file-path, but depends
 	 * on the implementation.
 	 *
-	 * @param sink The ResponseSink to which a FILE response shall be
-	 *   sent.
+	 * @param sink The ResponseSink to which the response shall be sent.
 	 */
 	virtual void Emit(const ResponseSink &sink) const = 0;
 
@@ -131,23 +130,23 @@ class PipeAudio : public Audio
 public:
 	/**
 	 * Constructs a PipeAudio from a source and a sink.
-	 * @param source The source of decoded audio frames.
+	 * @param src The source of decoded audio frames.
 	 * @param sink The target of decoded audio frames.
 	 * @see AudioSystem::Load
 	 */
-	PipeAudio(AudioSource *source, AudioSink *sink);
+	PipeAudio(AudioSource *src, AudioSink *sink);
 
 	void Start() override;
 	void Stop() override;
 	void Seek(std::uint64_t position) override;
 	Audio::State Update() override;
-	
+
 	void Emit(const ResponseSink &sink) const override;
 	std::uint64_t Position() const override;
 
 private:
 	/// The source of audio data.
-	std::unique_ptr<AudioSource> source;
+	std::unique_ptr<AudioSource> src;
 
 	/// The sink to which audio data is sent.
 	std::unique_ptr<AudioSink> sink;

src/audio/audio_sink.cpp

@@ -43,9 +43,7 @@ void AudioSink::Start()
 
 void AudioSink::Stop()
 {
-	if (!this->stream->isStopped()) {
-		this->stream->abort();
-	}
+	if (!this->stream->isStopped()) this->stream->abort();
 }
 
 bool AudioSink::IsStopped()

src/audio/audio_source.cpp

@@ -65,18 +65,18 @@ double AudioSource::SampleRate() const
 	return this->context->signal.rate;
 }
 
-std::uint64_t AudioSource::SamplePositionFromMicroseconds(std::uint64_t position) const
+std::uint64_t AudioSource::SamplesFromMicros(std::uint64_t micros) const
 {
 	// The sample rate is expressed in terms of samples per second, so we
 	// need to convert the position to seconds then multiply by the rate.
 	// We do things in a slightly peculiar order to minimise rounding.
 
-	return (position * this->SampleRate()) / 1000000;
+	return (micros * this->SampleRate()) / 1000000;
 }
 
-std::uint64_t AudioSource::MicrosecondPositionFromSamples(std::uint64_t samples) const
+std::uint64_t AudioSource::MicrosFromSamples(std::uint64_t samples) const
 {
-	// This is basically SamplePositionFromMicroseconds but backwards.
+	// This is basically SamplesFromMicros but backwards.
 
 	return (samples * 1000000) / this->SampleRate();
 }
@@ -99,7 +99,7 @@ std::uint64_t AudioSource::Seek(std::uint64_t position)
 {
 	assert(this->context != nullptr);
 
-	auto samples = this->SamplePositionFromMicroseconds(position);
+	auto samples = this->SamplesFromMicros(position);
 
 	// See BytesPerSample() for an explanation of this ChannelCount().
 	auto sox_samples = samples * this->ChannelCount();
@@ -180,8 +180,6 @@ void AudioSource::Open(const std::string &path)
 
 void AudioSource::Close()
 {
-	if (this->context != nullptr) {
-		sox_close(this->context);
-		this->context = nullptr;
-	}
+	if (this->context != nullptr) sox_close(this->context);
+	this->context = nullptr;
 }

src/audio/audio_source.hpp

@@ -119,16 +119,14 @@ class AudioSource
 	 * @param position The song position, in microseconds.
 	 * @return The corresponding number of elapsed samples.
 	 */
-	std::uint64_t SamplePositionFromMicroseconds(
-	                std::uint64_t position) const;
+	std::uint64_t SamplesFromMicros(std::uint64_t micros) const;
 
 	/**
 	 * Converts an elapsed sample count to a position in microseconds.
 	 * @param samples The number of elapsed samples.
 	 * @return The corresponding song position, in microseconds.
 	 */
-	std::uint64_t MicrosecondPositionFromSamples(
-	                std::uint64_t samples) const;
+	std::uint64_t MicrosFromSamples(std::uint64_t samples) const;
 
 private:
 	/// The size of the internal decoding buffer, in bytes.

src/audio/audio_system.cpp

@@ -88,17 +88,17 @@ Audio *PaSoxAudioSystem::Load(const std::string &path) const
 }
 
 portaudio::Stream *PaSoxAudioSystem::Configure(const AudioSource &source,
-                                          portaudio::CallbackInterface &cb) const
+                                               portaudio::CallbackInterface &cb) const
 {
 	std::uint8_t channel_count = source.ChannelCount();
 	SampleFormat sample_format = source.OutputSampleFormat();
 	double sample_rate = source.SampleRate();
 	size_t buffer_size = source.BufferSampleCapacity();
-	const portaudio::Device &device = PaDeviceFrom(this->device_id);
+	const portaudio::Device &device = PaDevice(this->device_id);
 
 	portaudio::DirectionSpecificStreamParameters out_pars(
-	                device, channel_count, PaSampleFormatFrom(sample_format),
-	                true, device.defaultLowOutputLatency(), nullptr);
+	                device, channel_count, PaFormat(sample_format), true,
+	                device.defaultLowOutputLatency(), nullptr);
 
 	portaudio::StreamParameters pars(
 	                portaudio::DirectionSpecificStreamParameters::null(),
@@ -107,17 +107,13 @@ portaudio::Stream *PaSoxAudioSystem::Configure(const AudioSource &source,
 	return new portaudio::InterfaceCallbackStream(pars, cb);
 }
 
-/* static */ const portaudio::Device &PaSoxAudioSystem::PaDeviceFrom(const std::string &id_string)
+/* static */ const portaudio::Device &PaSoxAudioSystem::PaDevice(
+                const std::string &id)
 {
 	auto &pa = portaudio::System::instance();
 
-	PaDeviceIndex id_pa = 0;
-
-	std::istringstream is(id_string);
-	is >> id_pa;
-
-	if (id_pa >= pa.deviceCount()) throw ConfigError(MSG_DEV_BADID);
-
+	PaDeviceIndex id_pa = std::stoi(id);
+	if (pa.deviceCount() <= id_pa) throw ConfigError(MSG_DEV_BADID);
 	return pa.deviceByIndex(id_pa);
 }
 
@@ -129,7 +125,8 @@ static const std::map<SampleFormat, portaudio::SampleDataFormat> pa_from_sf = {
 	{ SampleFormat::PACKED_FLOAT_32, portaudio::FLOAT32 }
 };
 
-/* static */ portaudio::SampleDataFormat PaSoxAudioSystem::PaSampleFormatFrom(SampleFormat fmt)
+/* static */ portaudio::SampleDataFormat PaSoxAudioSystem::PaFormat(
+                SampleFormat fmt)
 {
 	try {
 		return pa_from_sf.at(fmt);

src/audio/audio_system.hpp

@@ -113,14 +113,14 @@ class PaSoxAudioSystem : public AudioSystem, public AudioSinkConfigurator
 	 * @param id_string The device ID, as a string.
 	 * @return The device.
 	 */
-	static const portaudio::Device &PaDeviceFrom(const std::string &id_string);
+	static const portaudio::Device &PaDevice(const std::string &id_string);
 
 	/**
 	 * Converts a sample format identifier from playd to PortAudio.
 	 * @param fmt The playd sample format identifier.
 	 * @return The PortAudio equivalent of the given SampleFormat.
 	 */
-	static portaudio::SampleDataFormat PaSampleFormatFrom(SampleFormat fmt);
+	static portaudio::SampleDataFormat PaFormat(SampleFormat fmt);
 };
 
 #endif // PLAYD_AUDIO_SYSTEM_HPP

src/cmd_result.hpp

@@ -84,7 +84,8 @@ class CommandResult
 	 * @param sink The ResponseSink to which the response will be sent.
 	 * @param cmd The original command that created this CommandResult.
 	 */
-	void Emit(const ResponseSink &sink, const std::vector<std::string> &cmd) const;
+	void Emit(const ResponseSink &sink,
+	          const std::vector<std::string> &cmd) const;
 
 private:
 	Type type;       ///< The command result's type.

src/io/io_core.cpp

@@ -17,6 +17,7 @@
 #include <algorithm>
 #include <csignal>
 #include <cstring>
+#include <sstream>
 #include <string>
 
 extern "C" {
@@ -53,7 +54,8 @@ const std::uint16_t IoCore::PLAYER_UPDATE_PERIOD = 5; // ms
  *
  * [b]: https://nikhilm.github.io/uvbook/filesystem.html#buffers-and-streams
  */
-struct WriteReq {
+struct WriteReq
+{
 	uv_write_t req; ///< The main libuv write handle.
 	uv_buf_t buf;   ///< The associated write buffer.
 };
@@ -254,26 +256,30 @@ std::string Connection::Name()
 	// Using this instead of struct sockaddr is advised by the libuv docs,
 	// for IPv6 compatibility.
 	struct sockaddr_storage s;
-	auto sp = (struct sockaddr *) &s;
+	auto sp = (struct sockaddr *)&s;
 
 	// Turns out if you don't do this, Windows (and only Windows?) is upset.
 	socklen_t namelen = sizeof(s);
 
 	int pe = uv_tcp_getpeername(this->tcp, sp, (int *)&namelen);
-	if (pe) return std::string("(error getting peer info: ") + uv_strerror(pe) + ")";
+	// These std::string()s are needed as, otherwise, the compiler would
+	// think we're trying to add const char*s together.  We need AT LEAST
+	// ONE of the sides of the first + to be a std::string.
+	if (pe) return "<error@peer: " + std::string(uv_strerror(pe)) + ">";
 
 	// Now, split the sockaddr into host and service.
 	char host[NI_MAXHOST];
 	char serv[NI_MAXSERV];
 
 	// We use NI_NUMERICSERV to ensure a port number comes out.
 	// Otherwise, we could get a (likely erroneous) string description of
-	// what
-	// the network stack *thinks* the port is used for.
-	int ne = getnameinfo(sp, namelen, host, sizeof(host), serv, sizeof(serv), NI_NUMERICSERV);
-	if (ne) return std::string("(error getting name: ") + gai_strerror(ne) + ")";
+	// what the network stack *thinks* the port is used for.
+	int ne = getnameinfo(sp, namelen, host, sizeof(host), serv,
+	                     sizeof(serv), NI_NUMERICSERV);
+	// See comment for above error.
+	if (ne) return "<error@name: " + std::string(gai_strerror(ne)) + ">";
 
-	return std::string(host) + ":" + std::string(serv);
+	return host + std::string(":") + serv;
 }
 
 void Connection::Read(ssize_t nread, const uv_buf_t *buf)

src/main.hpp

@@ -57,13 +57,13 @@ class Playd : public ResponseSink
 	std::vector<std::string> argv; ///< The argument vector.
 	PaSoxAudioSystem audio;        ///< The audio subsystem.
 
-	PlayerFile pfile;              ///< The player-file subsystem.
-	PlayerPosition pposition;      ///< The player-position subsystem.
-	PlayerState pstate;            ///< The player-state subsystem.
-	Player player;                 ///< The player subsystem.
+	PlayerFile pfile;         ///< The player-file subsystem.
+	PlayerPosition pposition; ///< The player-position subsystem.
+	PlayerState pstate;       ///< The player-state subsystem.
+	Player player;            ///< The player subsystem.
 
-	CommandHandler handler;        ///< The command handler.
-	std::unique_ptr<IoCore> io;    ///< The I/O handler.
+	CommandHandler handler;     ///< The command handler.
+	std::unique_ptr<IoCore> io; ///< The I/O handler.
 
 	void RespondRaw(const std::string &string) const override;
 

src/player/player.cpp

@@ -28,22 +28,21 @@
 const std::vector<std::string> Player::FEATURES{ "End", "FileLoad", "PlayStop",
 	                                         "Seek", "TimeReport" };
 
-Player::Player(const ResponseSink *end_sink,
-               PlayerFile &file,
-               PlayerPosition &position,
-               PlayerState &state)
-    : file(file),
-      position(position),
-      state(state),
-      end_sink(end_sink)
+Player::Player(const ResponseSink *end_sink, PlayerFile &file,
+               PlayerPosition &position, PlayerState &state)
+    : file(file), position(position), state(state), end_sink(end_sink)
 {
 }
 
 bool Player::Update()
 {
 	auto as = this->file.Update();
 	if (as == Audio::State::AT_END) this->End();
-	if (as == Audio::State::PLAYING) this->position.Update(this->file.Position());
+	if (as == Audio::State::PLAYING) {
+		// Since the audio is currently playing, the position may have
+		// advanced since last update.  So we need to update it.
+		this->position.Update(this->file.Position());
+	}
 
 	return this->state.IsRunning();
 }
@@ -150,7 +149,8 @@ CommandResult Player::Seek(const std::string &time_str)
 	// cpos will point to the first character in pos that wasn't a number.
 	// We don't want any characters here, so bail if the position isn't at
 	// the end of the string.
-	if (cpos != time_str.length()) return CommandResult::Invalid(MSG_SEEK_INVALID_VALUE);
+	auto sl = time_str.length();
+	if (cpos != sl) return CommandResult::Invalid(MSG_SEEK_INVALID_VALUE);
 
 	try {
 		this->SeekRaw(pos);

src/player/player.hpp

@@ -54,7 +54,8 @@ class Player
 	 * @param position The player's position component.
 	 * @param state The player's state component.
 	 */
-	Player(const ResponseSink *end_sink, PlayerFile &file, PlayerPosition &position, PlayerState &state);
+	Player(const ResponseSink *end_sink, PlayerFile &file,
+	       PlayerPosition &position, PlayerState &state);
 
 	/// Deleted copy constructor.
 	Player(const Player &) = delete;