Prefer std::optional return types over output parameters

This commit is contained in:
Chris Thrasher 2023-11-26 17:16:59 -07:00
parent fe0785769e
commit c6919e28fc
10 changed files with 72 additions and 74 deletions

View File

@ -29,6 +29,8 @@
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
#include <SFML/Audio/Export.hpp> #include <SFML/Audio/Export.hpp>
#include <optional>
#include <cstdint> #include <cstdint>
@ -68,12 +70,11 @@ public:
/// during the whole lifetime of the reader. /// during the whole lifetime of the reader.
/// ///
/// \param stream Source stream to read from /// \param stream Source stream to read from
/// \param info Structure to fill with the properties of the loaded sound
/// ///
/// \return True if the file was successfully opened /// \return Properties of the loaded sound if the file was successfully opened
/// ///
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
[[nodiscard]] virtual bool open(InputStream& stream, Info& info) = 0; [[nodiscard]] virtual std::optional<Info> open(InputStream& stream) = 0;
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
/// \brief Change the current read position to the given sample offset /// \brief Change the current read position to the given sample offset
@ -132,7 +133,7 @@ public:
/// // return true if the reader can handle the format /// // return true if the reader can handle the format
/// } /// }
/// ///
/// [[nodiscard]] bool open(sf::InputStream& stream, Info& info) override /// [[nodiscard]] std::optional<sf::SoundFileReader::Info> open(sf::InputStream& stream) override
/// { /// {
/// // read the sound file header and fill the sound attributes /// // read the sound file header and fill the sound attributes
/// // (channel count, sample count and sample rate) /// // (channel count, sample count and sample rate)

View File

@ -81,8 +81,8 @@ bool InputSoundFile::openFromFile(const std::filesystem::path& filename)
return false; return false;
// Pass the stream to the reader // Pass the stream to the reader
SoundFileReader::Info info; const auto info = reader->open(*file);
if (!reader->open(*file, info)) if (!info)
return false; return false;
// Take ownership of successfully opened reader and stream // Take ownership of successfully opened reader and stream
@ -90,9 +90,9 @@ bool InputSoundFile::openFromFile(const std::filesystem::path& filename)
m_stream = std::move(file); m_stream = std::move(file);
// Retrieve the attributes of the open sound file // Retrieve the attributes of the open sound file
m_sampleCount = info.sampleCount; m_sampleCount = info->sampleCount;
m_channelCount = info.channelCount; m_channelCount = info->channelCount;
m_sampleRate = info.sampleRate; m_sampleRate = info->sampleRate;
return true; return true;
} }
@ -116,8 +116,8 @@ bool InputSoundFile::openFromMemory(const void* data, std::size_t sizeInBytes)
memory->open(data, sizeInBytes); memory->open(data, sizeInBytes);
// Pass the stream to the reader // Pass the stream to the reader
SoundFileReader::Info info; const auto info = reader->open(*memory);
if (!reader->open(*memory, info)) if (!info)
return false; return false;
// Take ownership of successfully opened reader and stream // Take ownership of successfully opened reader and stream
@ -125,9 +125,9 @@ bool InputSoundFile::openFromMemory(const void* data, std::size_t sizeInBytes)
m_stream = std::move(memory); m_stream = std::move(memory);
// Retrieve the attributes of the open sound file // Retrieve the attributes of the open sound file
m_sampleCount = info.sampleCount; m_sampleCount = info->sampleCount;
m_channelCount = info.channelCount; m_channelCount = info->channelCount;
m_sampleRate = info.sampleRate; m_sampleRate = info->sampleRate;
return true; return true;
} }
@ -152,8 +152,8 @@ bool InputSoundFile::openFromStream(InputStream& stream)
} }
// Pass the stream to the reader // Pass the stream to the reader
SoundFileReader::Info info; const auto info = reader->open(stream);
if (!reader->open(stream, info)) if (!info)
return false; return false;
// Take ownership of reader and store a reference to the stream without taking ownership // Take ownership of reader and store a reference to the stream without taking ownership
@ -161,9 +161,9 @@ bool InputSoundFile::openFromStream(InputStream& stream)
m_stream = {&stream, false}; m_stream = {&stream, false};
// Retrieve the attributes of the open sound file // Retrieve the attributes of the open sound file
m_sampleCount = info.sampleCount; m_sampleCount = info->sampleCount;
m_channelCount = info.channelCount; m_channelCount = info->channelCount;
m_sampleRate = info.sampleRate; m_sampleRate = info->sampleRate;
return true; return true;
} }

View File

@ -227,14 +227,14 @@ bool SoundFileReaderFlac::check(InputStream& stream)
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
bool SoundFileReaderFlac::open(InputStream& stream, Info& info) std::optional<SoundFileReader::Info> SoundFileReaderFlac::open(InputStream& stream)
{ {
// Create the decoder // Create the decoder
m_decoder.reset(FLAC__stream_decoder_new()); m_decoder.reset(FLAC__stream_decoder_new());
if (!m_decoder) if (!m_decoder)
{ {
err() << "Failed to open FLAC file (failed to allocate the decoder)" << std::endl; err() << "Failed to open FLAC file (failed to allocate the decoder)" << std::endl;
return false; return std::nullopt;
} }
// Initialize the decoder with our callbacks // Initialize the decoder with our callbacks
@ -255,13 +255,11 @@ bool SoundFileReaderFlac::open(InputStream& stream, Info& info)
{ {
m_decoder.reset(); m_decoder.reset();
err() << "Failed to open FLAC file (failed to read metadata)" << std::endl; err() << "Failed to open FLAC file (failed to read metadata)" << std::endl;
return false; return std::nullopt;
} }
// Retrieve the sound properties // Retrieve the sound properties
info = m_clientData.info; // was filled in the "metadata" callback return m_clientData.info; // was filled in the "metadata" callback
return true;
} }

View File

@ -57,10 +57,11 @@ public:
/// \brief Open a sound file for reading /// \brief Open a sound file for reading
/// ///
/// \param stream Stream to open /// \param stream Stream to open
/// \param info Structure to fill with the attributes of the loaded sound ///
/// \return Properties of the loaded sound if the file was successfully opened
/// ///
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
[[nodiscard]] bool open(sf::InputStream& stream, Info& info) override; [[nodiscard]] std::optional<Info> open(sf::InputStream& stream) override;
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
/// \brief Change the current read position to the given sample offset /// \brief Change the current read position to the given sample offset

View File

@ -118,7 +118,7 @@ SoundFileReaderMp3::~SoundFileReaderMp3()
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
bool SoundFileReaderMp3::open(InputStream& stream, Info& info) std::optional<SoundFileReader::Info> SoundFileReaderMp3::open(InputStream& stream)
{ {
// Init IO callbacks // Init IO callbacks
m_io.read_data = &stream; m_io.read_data = &stream;
@ -127,15 +127,16 @@ bool SoundFileReaderMp3::open(InputStream& stream, Info& info)
// Init mp3 decoder // Init mp3 decoder
mp3dec_ex_open_cb(&m_decoder, &m_io, MP3D_SEEK_TO_SAMPLE); mp3dec_ex_open_cb(&m_decoder, &m_io, MP3D_SEEK_TO_SAMPLE);
if (!m_decoder.samples) if (!m_decoder.samples)
return false; return std::nullopt;
// Retrieve the music attributes // Retrieve the music attributes
Info info;
info.channelCount = static_cast<unsigned int>(m_decoder.info.channels); info.channelCount = static_cast<unsigned int>(m_decoder.info.channels);
info.sampleRate = static_cast<unsigned int>(m_decoder.info.hz); info.sampleRate = static_cast<unsigned int>(m_decoder.info.hz);
info.sampleCount = m_decoder.samples; info.sampleCount = m_decoder.samples;
m_numSamples = info.sampleCount; m_numSamples = info.sampleCount;
return true; return info;
} }

View File

@ -86,12 +86,11 @@ public:
/// \brief Open a sound file for reading /// \brief Open a sound file for reading
/// ///
/// \param stream Source stream to read from /// \param stream Source stream to read from
/// \param info Structure to fill with the properties of the loaded sound
/// ///
/// \return True if the file was successfully opened /// \return Properties of the loaded sound if the file was successfully opened
/// ///
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
[[nodiscard]] bool open(InputStream& stream, Info& info) override; [[nodiscard]] std::optional<Info> open(InputStream& stream) override;
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
/// \brief Change the current read position to the given sample offset /// \brief Change the current read position to the given sample offset

View File

@ -95,18 +95,19 @@ SoundFileReaderOgg::~SoundFileReaderOgg()
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
bool SoundFileReaderOgg::open(InputStream& stream, Info& info) std::optional<SoundFileReader::Info> SoundFileReaderOgg::open(InputStream& stream)
{ {
// Open the Vorbis stream // Open the Vorbis stream
const int status = ov_open_callbacks(&stream, &m_vorbis, nullptr, 0, callbacks); const int status = ov_open_callbacks(&stream, &m_vorbis, nullptr, 0, callbacks);
if (status < 0) if (status < 0)
{ {
err() << "Failed to open Vorbis file for reading" << std::endl; err() << "Failed to open Vorbis file for reading" << std::endl;
return false; return std::nullopt;
} }
// Retrieve the music attributes // Retrieve the music attributes
vorbis_info* vorbisInfo = ov_info(&m_vorbis, -1); vorbis_info* vorbisInfo = ov_info(&m_vorbis, -1);
Info info;
info.channelCount = static_cast<unsigned int>(vorbisInfo->channels); info.channelCount = static_cast<unsigned int>(vorbisInfo->channels);
info.sampleRate = static_cast<unsigned int>(vorbisInfo->rate); info.sampleRate = static_cast<unsigned int>(vorbisInfo->rate);
info.sampleCount = static_cast<std::size_t>(ov_pcm_total(&m_vorbis, -1) * vorbisInfo->channels); info.sampleCount = static_cast<std::size_t>(ov_pcm_total(&m_vorbis, -1) * vorbisInfo->channels);
@ -114,7 +115,7 @@ bool SoundFileReaderOgg::open(InputStream& stream, Info& info)
// We must keep the channel count for the seek function // We must keep the channel count for the seek function
m_channelCount = info.channelCount; m_channelCount = info.channelCount;
return true; return info;
} }

View File

@ -61,12 +61,11 @@ public:
/// \brief Open a sound file for reading /// \brief Open a sound file for reading
/// ///
/// \param stream Source stream to read from /// \param stream Source stream to read from
/// \param info Structure to fill with the properties of the loaded sound
/// ///
/// \return True if the file was successfully opened /// \return Properties of the loaded sound if the file was successfully opened
/// ///
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
[[nodiscard]] bool open(InputStream& stream, Info& info) override; [[nodiscard]] std::optional<Info> open(InputStream& stream) override;
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
/// \brief Change the current read position to the given sample offset /// \brief Change the current read position to the given sample offset

View File

@ -120,17 +120,15 @@ bool SoundFileReaderWav::check(InputStream& stream)
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
bool SoundFileReaderWav::open(InputStream& stream, Info& info) std::optional<SoundFileReader::Info> SoundFileReaderWav::open(InputStream& stream)
{ {
m_stream = &stream; m_stream = &stream;
if (!parseHeader(info)) const auto info = parseHeader();
{ if (!info)
err() << "Failed to open WAV sound file (invalid or unsupported file)" << std::endl; err() << "Failed to open WAV sound file (invalid or unsupported file)" << std::endl;
return false;
}
return true; return info;
} }
@ -213,7 +211,7 @@ std::uint64_t SoundFileReaderWav::read(std::int16_t* samples, std::uint64_t maxC
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
bool SoundFileReaderWav::parseHeader(Info& info) std::optional<SoundFileReader::Info> SoundFileReaderWav::parseHeader()
{ {
assert(m_stream && "Input stream cannot be null. Call SoundFileReaderWav::open() to initialize it."); assert(m_stream && "Input stream cannot be null. Call SoundFileReaderWav::open() to initialize it.");
@ -222,9 +220,10 @@ bool SoundFileReaderWav::parseHeader(Info& info)
char mainChunk[mainChunkSize]; char mainChunk[mainChunkSize];
if (static_cast<std::size_t>(m_stream->read(mainChunk, static_cast<std::int64_t>(sizeof(mainChunk)))) != if (static_cast<std::size_t>(m_stream->read(mainChunk, static_cast<std::int64_t>(sizeof(mainChunk)))) !=
sizeof(mainChunk)) sizeof(mainChunk))
return false; return std::nullopt;
// Parse all the sub-chunks // Parse all the sub-chunks
Info info;
bool dataChunkFound = false; bool dataChunkFound = false;
while (!dataChunkFound) while (!dataChunkFound)
{ {
@ -232,13 +231,13 @@ bool SoundFileReaderWav::parseHeader(Info& info)
char subChunkId[4]; char subChunkId[4];
if (static_cast<std::size_t>(m_stream->read(subChunkId, static_cast<std::int64_t>(sizeof(subChunkId)))) != if (static_cast<std::size_t>(m_stream->read(subChunkId, static_cast<std::int64_t>(sizeof(subChunkId)))) !=
sizeof(subChunkId)) sizeof(subChunkId))
return false; return std::nullopt;
std::uint32_t subChunkSize = 0; std::uint32_t subChunkSize = 0;
if (!decode(*m_stream, subChunkSize)) if (!decode(*m_stream, subChunkSize))
return false; return std::nullopt;
const std::int64_t subChunkStart = m_stream->tell(); const std::int64_t subChunkStart = m_stream->tell();
if (subChunkStart == -1) if (subChunkStart == -1)
return false; return std::nullopt;
// Check which chunk it is // Check which chunk it is
if ((subChunkId[0] == 'f') && (subChunkId[1] == 'm') && (subChunkId[2] == 't') && (subChunkId[3] == ' ')) if ((subChunkId[0] == 'f') && (subChunkId[1] == 'm') && (subChunkId[2] == 't') && (subChunkId[3] == ' '))
@ -248,41 +247,41 @@ bool SoundFileReaderWav::parseHeader(Info& info)
// Audio format // Audio format
std::uint16_t format = 0; std::uint16_t format = 0;
if (!decode(*m_stream, format)) if (!decode(*m_stream, format))
return false; return std::nullopt;
if ((format != waveFormatPcm) && (format != waveFormatExtensible)) if ((format != waveFormatPcm) && (format != waveFormatExtensible))
return false; return std::nullopt;
// Channel count // Channel count
std::uint16_t channelCount = 0; std::uint16_t channelCount = 0;
if (!decode(*m_stream, channelCount)) if (!decode(*m_stream, channelCount))
return false; return std::nullopt;
info.channelCount = channelCount; info.channelCount = channelCount;
// Sample rate // Sample rate
std::uint32_t sampleRate = 0; std::uint32_t sampleRate = 0;
if (!decode(*m_stream, sampleRate)) if (!decode(*m_stream, sampleRate))
return false; return std::nullopt;
info.sampleRate = sampleRate; info.sampleRate = sampleRate;
// Byte rate // Byte rate
std::uint32_t byteRate = 0; std::uint32_t byteRate = 0;
if (!decode(*m_stream, byteRate)) if (!decode(*m_stream, byteRate))
return false; return std::nullopt;
// Block align // Block align
std::uint16_t blockAlign = 0; std::uint16_t blockAlign = 0;
if (!decode(*m_stream, blockAlign)) if (!decode(*m_stream, blockAlign))
return false; return std::nullopt;
// Bits per sample // Bits per sample
std::uint16_t bitsPerSample = 0; std::uint16_t bitsPerSample = 0;
if (!decode(*m_stream, bitsPerSample)) if (!decode(*m_stream, bitsPerSample))
return false; return std::nullopt;
if (bitsPerSample != 8 && bitsPerSample != 16 && bitsPerSample != 24 && bitsPerSample != 32) if (bitsPerSample != 8 && bitsPerSample != 16 && bitsPerSample != 24 && bitsPerSample != 32)
{ {
err() << "Unsupported sample size: " << bitsPerSample err() << "Unsupported sample size: " << bitsPerSample
<< " bit (Supported sample sizes are 8/16/24/32 bit)" << std::endl; << " bit (Supported sample sizes are 8/16/24/32 bit)" << std::endl;
return false; return std::nullopt;
} }
m_bytesPerSample = bitsPerSample / 8; m_bytesPerSample = bitsPerSample / 8;
@ -291,28 +290,28 @@ bool SoundFileReaderWav::parseHeader(Info& info)
// Extension size // Extension size
std::uint16_t extensionSize = 0; std::uint16_t extensionSize = 0;
if (!decode(*m_stream, extensionSize)) if (!decode(*m_stream, extensionSize))
return false; return std::nullopt;
// Valid bits per sample // Valid bits per sample
std::uint16_t validBitsPerSample = 0; std::uint16_t validBitsPerSample = 0;
if (!decode(*m_stream, validBitsPerSample)) if (!decode(*m_stream, validBitsPerSample))
return false; return std::nullopt;
// Channel mask // Channel mask
std::uint32_t channelMask = 0; std::uint32_t channelMask = 0;
if (!decode(*m_stream, channelMask)) if (!decode(*m_stream, channelMask))
return false; return std::nullopt;
// Subformat // Subformat
char subformat[16]; char subformat[16];
if (static_cast<std::size_t>(m_stream->read(subformat, static_cast<std::int64_t>(sizeof(subformat)))) != if (static_cast<std::size_t>(m_stream->read(subformat, static_cast<std::int64_t>(sizeof(subformat)))) !=
sizeof(subformat)) sizeof(subformat))
return false; return std::nullopt;
if (std::memcmp(subformat, waveSubformatPcm, sizeof(subformat)) != 0) if (std::memcmp(subformat, waveSubformatPcm, sizeof(subformat)) != 0)
{ {
err() << "Unsupported format: extensible format with non-PCM subformat" << std::endl; err() << "Unsupported format: extensible format with non-PCM subformat" << std::endl;
return false; return std::nullopt;
} }
if (validBitsPerSample != bitsPerSample) if (validBitsPerSample != bitsPerSample)
@ -321,13 +320,13 @@ bool SoundFileReaderWav::parseHeader(Info& info)
<< " bits) and " << " bits) and "
"sample container size (" "sample container size ("
<< bitsPerSample << " bits) differ" << std::endl; << bitsPerSample << " bits) differ" << std::endl;
return false; return std::nullopt;
} }
} }
// Skip potential extra information // Skip potential extra information
if (m_stream->seek(subChunkStart + subChunkSize) == -1) if (m_stream->seek(subChunkStart + subChunkSize) == -1)
return false; return std::nullopt;
} }
else if ((subChunkId[0] == 'd') && (subChunkId[1] == 'a') && (subChunkId[2] == 't') && (subChunkId[3] == 'a')) else if ((subChunkId[0] == 'd') && (subChunkId[1] == 'a') && (subChunkId[2] == 't') && (subChunkId[3] == 'a'))
{ {
@ -346,11 +345,11 @@ bool SoundFileReaderWav::parseHeader(Info& info)
{ {
// unknown chunk, skip it // unknown chunk, skip it
if (m_stream->seek(m_stream->tell() + subChunkSize) == -1) if (m_stream->seek(m_stream->tell() + subChunkSize) == -1)
return false; return std::nullopt;
} }
} }
return true; return info;
} }
} // namespace sf::priv } // namespace sf::priv

View File

@ -53,12 +53,11 @@ public:
/// \brief Open a sound file for reading /// \brief Open a sound file for reading
/// ///
/// \param stream Stream to open /// \param stream Stream to open
/// \param info Structure to fill with the attributes of the loaded sound
/// ///
/// \return True if the file was successfully opened /// \return Properties of the loaded sound if the file was successfully opened
/// ///
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
[[nodiscard]] bool open(sf::InputStream& stream, Info& info) override; [[nodiscard]] std::optional<Info> open(sf::InputStream& stream) override;
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
/// \brief Change the current read position to the given sample offset /// \brief Change the current read position to the given sample offset
@ -95,7 +94,7 @@ private:
/// \return True on success, false on error /// \return True on success, false on error
/// ///
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
bool parseHeader(Info& info); std::optional<Info> parseHeader();
//////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////
// Member data // Member data