diff --git a/doc/changelog.dox b/doc/changelog.dox index cf14aae83..979cba044 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1991,6 +1991,17 @@ See also: @relativeref{Corrade,Containers::Array} / @relativeref{Corrade,Containers::ArrayView} of them, instead of a @ref std::string and a @ref std::vector. + - @ref Audio::Context::deviceSpecifierStrings(), + @relativeref{Audio::Context,hrtfSpecifierString()}, + @relativeref{Audio::Context,deviceSpecifierString()}, + @relativeref{Audio::Context,vendorString()}, + @relativeref{Audio::Context,rendererString()}, + @relativeref{Audio::Context,versionString()}, + @relativeref{Audio::Context,extensionStrings()} now return + @relativeref{Corrade,Containers::StringView} or a + @relativeref{Corrade,Containers::Array} / + @relativeref{Corrade,Containers::ArrayView} of them, instead of a + @ref std::string and a @ref std::vector. - All @ref GL::Buffer::label() "label()" and @ref GL::Buffer::setLabel() "setLabel()" APIs now work with a @relativeref{Corrade,Containers::StringView} / diff --git a/src/Magnum/Audio/Context.cpp b/src/Magnum/Audio/Context.cpp index 742467078..c9c174fbe 100644 --- a/src/Magnum/Audio/Context.cpp +++ b/src/Magnum/Audio/Context.cpp @@ -29,11 +29,10 @@ #include "Context.h" #include /* std::lower_bound() */ +#include #include #include #include -#include -#include #include "Magnum/Audio/Extensions.h" @@ -61,6 +60,16 @@ constexpr Extension ExtensionList[]{ #undef _entension }; +const Extension* findExtension(const Containers::StringView extension) { + const auto found = std::lower_bound(std::begin(ExtensionList), std::end(ExtensionList), extension, [](const Extension& a, const Containers::StringView& b) { + return a.string() < b; + }); + if(found != std::end(ExtensionList) && found->string() == extension) + return found; + + return {}; +} + } Containers::ArrayView Extension::extensions() { @@ -108,11 +117,11 @@ const char* alcErrorString(const ALenum error) { } -std::vector Context::deviceSpecifierStrings() { - std::vector list; +Containers::Array Context::deviceSpecifierStrings() { + Containers::Array list; const char* device = alcGetString(nullptr, ALC_DEVICE_SPECIFIER); while(*device) { - list.emplace_back(device); + arrayAppend(list, {device, Containers::StringViewFlag::Global}); device += list.back().size() + 1; } @@ -197,9 +206,17 @@ Context::Context(NoCreateT, const Int argc, const char* const* const argv) noexc /* Decide how to display initialization log */ _displayInitializationLog = !(args.value("log") == "quiet" || args.value("log") == "QUIET"); - /* Disable extensions */ - for(auto&& extension: Utility::String::splitWithoutEmptyParts(args.value("disable-extensions"))) - _disabledExtensionStrings.push_back(extension); + /* Disable extensions. Here we search for them among the known extensions + and store the Extension objects instead, which avoids the string copying + and another binary search in tryCreate(). */ + const Containers::StringView disabledExtensions = args.value("disable-extensions"); + if(!disabledExtensions.isEmpty()) { + const Containers::Array split = disabledExtensions.splitOnWhitespaceWithoutEmptyParts(); + arrayReserve(_disabledExtensions, split.size()); + for(const Containers::StringView extension: split) + if(const Extension* found = findExtension(extension)) + arrayAppend(_disabledExtensions, *found); + } } void Context::create(const Configuration& configuration) { @@ -211,7 +228,7 @@ bool Context::tryCreate(const Configuration& configuration) { CORRADE_ASSERT(!currentContext, "Audio::Context: context already created", false); /* Open the device */ - const ALCchar* const deviceSpecifier = configuration.deviceSpecifier().empty() ? alcGetString(nullptr, ALC_DEFAULT_DEVICE_SPECIFIER) : configuration.deviceSpecifier().data(); + const ALCchar* const deviceSpecifier = configuration.deviceSpecifier().isEmpty() ? alcGetString(nullptr, ALC_DEFAULT_DEVICE_SPECIFIER) : configuration.deviceSpecifier().data(); if(!(_device = alcOpenDevice(deviceSpecifier))) { Error() << "Audio::Context: cannot open sound device" << deviceSpecifier; return false; @@ -270,11 +287,10 @@ bool Context::tryCreate(const Configuration& configuration) { currentContext = this; /* Check for presence of extensions */ - const std::vector extensions = extensionStrings(); - for(const std::string& extension: extensions) { - const auto found = std::lower_bound(std::begin(ExtensionList), std::end(ExtensionList), extension, [](const Extension& a, const std::string& b) { return a.string() < b; }); - if(found != std::end(ExtensionList) && found->string() == extension) { - _supportedExtensions.push_back(*found); + const Containers::Array extensions = extensionStrings(); + for(const Containers::StringView extension: extensions) { + if(const Extension* found = findExtension(extension)) { + arrayAppend(_supportedExtensions, *found); _extensionStatus.set(found->index(), true); } } @@ -286,33 +302,15 @@ bool Context::tryCreate(const Configuration& configuration) { Debug{output} << "OpenAL version:" << versionString(); /* Disable extensions as requested by the user */ - if(!_disabledExtensionStrings.empty()) { - bool headerPrinted = false; + if(!_disabledExtensions.isEmpty()) { + Debug{output} << "Disabling extensions:"; /* Disable extensions that are known and supported and print a message for each */ - for(auto&& extension: _disabledExtensionStrings) { - const auto found = std::lower_bound(std::begin(ExtensionList), std::end(ExtensionList), extension, [](const Extension& a, const std::string& b) { return a.string() < b; }); - /* No error message here because some of the extensions could be - from Vulkan or OpenGL. That also means we print the header only - when we actually have something to say */ - if(found == std::end(ExtensionList) || found->string() != extension) - continue; - - /* If the extension isn't supported in the first place, don't do - anything. If it is, set its status as unsupported but flip the - corresponding bit in the disabled bitmap so we know it is - supported and only got disabled */ - if(!_extensionStatus[found->index()]) - continue; - _extensionStatus.set(found->index(), false); - _disabledExtensions.set(found->index(), true); - - if(!headerPrinted) { - Debug{output} << "Disabling extensions:"; - headerPrinted = true; - } - Debug{output} << " " << extension; + for(const Extension& extension: _disabledExtensions) { + _extensionStatus.set(extension.index(), false); + _extensionDisabledStatus.set(extension.index(), true); + Debug{output} << " " << extension.string(); } } @@ -335,15 +333,12 @@ Context::~Context() { currentContext = nullptr; } -std::vector Context::extensionStrings() const { - std::vector extensions; - - /* Don't crash when alGetString() returns nullptr */ - extensions = Utility::String::splitWithoutEmptyParts(Utility::String::fromArray(reinterpret_cast(alGetString(AL_EXTENSIONS))), ' '); +Containers::Array Context::extensionStrings() const { + Containers::Array extensions = Containers::StringView{reinterpret_cast(alGetString(AL_EXTENSIONS)), Containers::StringViewFlag::Global}.splitWithoutEmptyParts(' '); - /* Add ALC extensions as well */ - auto splitAlcExts = Utility::String::splitWithoutEmptyParts(Utility::String::fromArray(reinterpret_cast(alcGetString(_device, ALC_EXTENSIONS))), ' '); - extensions.insert(extensions.end(), splitAlcExts.begin(), splitAlcExts.end()); + /* Add ALC extensions as well. This reallocates the above array but that's + fine, it's not a hot code path. */ + arrayAppend(extensions, Containers::StringView{reinterpret_cast(alcGetString(_device, ALC_EXTENSIONS)), Containers::StringViewFlag::Global}.splitWithoutEmptyParts(' ')); return extensions; } @@ -369,9 +364,9 @@ Context::HrtfStatus Context::hrtfStatus() const { return Context::HrtfStatus(status); } -std::string Context::hrtfSpecifierString() const { +Containers::StringView Context::hrtfSpecifierString() const { /* Returns a string on ALC_SOFT_HRTF, nullptr on ALC_SOFTX_HRTF */ - return Utility::String::fromArray(alcGetString(_device, ALC_HRTF_SPECIFIER_SOFT)); + return {alcGetString(_device, ALC_HRTF_SPECIFIER_SOFT), Containers::StringViewFlag::Global}; } Int Context::monoSourceCount() const { @@ -392,33 +387,20 @@ Int Context::refreshRate() const { return count; } -std::string Context::deviceSpecifierString() const { - return alcGetString(_device, ALC_DEVICE_SPECIFIER); +Containers::StringView Context::deviceSpecifierString() const { + return {alcGetString(_device, ALC_DEVICE_SPECIFIER), Containers::StringViewFlag::Global}; } -std::string Context::vendorString() const { - return alGetString(AL_VENDOR); +Containers::StringView Context::vendorString() const { + return {alGetString(AL_VENDOR), Containers::StringViewFlag::Global}; } -std::string Context::rendererString() const { - return alGetString(AL_RENDERER); -} - -std::string Context::versionString() const { - return alGetString(AL_VERSION); -} - -Context::Configuration::Configuration() = default; -Context::Configuration::~Configuration() = default; - -Context::Configuration& Context::Configuration::setDeviceSpecifier(const std::string& specifier) { - _deviceSpecifier = specifier; - return *this; +Containers::StringView Context::rendererString() const { + return {alGetString(AL_RENDERER), Containers::StringViewFlag::Global}; } -Context::Configuration& Context::Configuration::setDeviceSpecifier(std::string&& specifier) { - _deviceSpecifier = Utility::move(specifier); - return *this; +Containers::StringView Context::versionString() const { + return {alGetString(AL_VERSION), Containers::StringViewFlag::Global}; } }} diff --git a/src/Magnum/Audio/Context.h b/src/Magnum/Audio/Context.h index d7f41720d..21b62d462 100644 --- a/src/Magnum/Audio/Context.h +++ b/src/Magnum/Audio/Context.h @@ -33,9 +33,9 @@ */ #include /* std::abort() in MAGNUM_ASSERT_AUDIO_EXTENSION_SUPPORTED() */ -#include -#include +#include #include +#include /** @todo PIMPL Configuration instead? */ #include #include "Magnum/Magnum.h" @@ -50,6 +50,12 @@ #include #include "MagnumExternal/OpenAL/extensions.h" +#ifdef MAGNUM_BUILD_DEPRECATED +/* For return types of Context::versionString() etc., which used to be a + std::string. Not ideal for the return types, but at least something. */ +#include +#endif + namespace Magnum { namespace Audio { namespace Implementation { @@ -180,7 +186,7 @@ class MAGNUM_AUDIO_EXPORT Context { * @see @ref deviceSpecifierString(), @ref Configuration::setDeviceSpecifier() * @fn_alc{GetString} with @def_alc_keyword{DEVICE_SPECIFIER} */ - static std::vector deviceSpecifierStrings(); + static Containers::Array deviceSpecifierStrings(); /** * @brief Whether there is any current context @@ -306,11 +312,14 @@ class MAGNUM_AUDIO_EXPORT Context { * @brief HRTF specifier * * Name of the HRTF being used. The result is *not* cached, repeated - * queries will result in repeated OpenAL calls. + * queries will result in repeated OpenAL calls. The returned view is + * always @relativeref{Corrade::Containers::StringViewFlag,Global}, and + * if non-empty it's also + * @relativeref{Corrade,Containers::StringViewFlag::NullTerminated}. * @see @fn_al{GetString} with @def_alc_keyword{HRTF_SPECIFIER_SOFT} * @requires_al_extension @alc_extension{SOFT,HRTF} */ - std::string hrtfSpecifierString() const; + Containers::StringView hrtfSpecifierString() const; /** * @brief Count of supported mono sources @@ -349,40 +358,48 @@ class MAGNUM_AUDIO_EXPORT Context { * @brief Device specifier string * * The result is *not* cached, repeated queries will result in repeated - * OpenAL calls. + * OpenAL calls. The returned view is always + * @relativeref{Corrade,Containers::StringViewFlag::NullTerminated} and + * @relativeref{Corrade::Containers::StringViewFlag,Global}. * @see @ref deviceSpecifierStrings(), @ref vendorString(), @ref rendererString(), * @fn_al{GetString} with @def_alc_keyword{DEVICE_SPECIFIER} */ - std::string deviceSpecifierString() const; + Containers::StringView deviceSpecifierString() const; /** * @brief Vendor string * * The result is *not* cached, repeated queries will result in repeated - * OpenAL calls. + * OpenAL calls. The returned view is always + * @relativeref{Corrade,Containers::StringViewFlag::NullTerminated} and + * @relativeref{Corrade::Containers::StringViewFlag,Global}. * @see @ref deviceSpecifierString(), @ref rendererString(), * @fn_al{GetString} with @def_al_keyword{VENDOR} */ - std::string vendorString() const; + Containers::StringView vendorString() const; /** * @brief Renderer string * * The result is *not* cached, repeated queries will result in repeated - * OpenAL calls. + * OpenAL calls. The returned view is always + * @relativeref{Corrade,Containers::StringViewFlag::NullTerminated} and + * @relativeref{Corrade::Containers::StringViewFlag,Global}. * @see @ref deviceSpecifierString(), @ref vendorString(), * @fn_al{GetString} with @def_al_keyword{RENDERER} */ - std::string rendererString() const; + Containers::StringView rendererString() const; /** * @brief Version string * * The result is *not* cached, repeated queries will result in repeated - * OpenAL calls. + * OpenAL calls. The returned view is always + * @relativeref{Corrade,Containers::StringViewFlag::NullTerminated} and + * @relativeref{Corrade::Containers::StringViewFlag,Global}. * @see @fn_al{GetString} with @def_al_keyword{VERSION} */ - std::string versionString() const; + Containers::StringView versionString() const; /** * @brief Extension strings @@ -391,12 +408,15 @@ class MAGNUM_AUDIO_EXPORT Context { * OpenAL calls. Note that this function returns list of all extensions * reported by the driver (even those not supported by Magnum), see * @ref supportedExtensions(), @ref Extension::extensions() or - * @ref isExtensionSupported() for alternatives. + * @ref isExtensionSupported() for alternatives. The returned views are + * always @relativeref{Corrade::Containers::StringViewFlag,Global}, but + * as the extensions are split from a space-separated list only the + * last one is guaranteed to be null-terminated. * @see @fn_al{Get} with @def_al_keyword{NUM_EXTENSIONS}, * @fn_al{GetString} with @def_al_keyword{EXTENSIONS}, * @fn_alc{GetString} with @def_alc_keyword{EXTENSIONS} */ - std::vector extensionStrings() const; + Containers::Array extensionStrings() const; /** * @brief Supported extensions @@ -405,7 +425,7 @@ class MAGNUM_AUDIO_EXPORT Context { * the current. * @see @ref isExtensionSupported(), @ref Extension::extensions() */ - const std::vector& supportedExtensions() const { + Containers::ArrayView supportedExtensions() const { return _supportedExtensions; } @@ -446,7 +466,7 @@ class MAGNUM_AUDIO_EXPORT Context { * if they are advertised as being supported by the driver. */ template bool isExtensionDisabled() const { - return _disabledExtensions[T::Index]; + return _extensionDisabledStatus[T::Index]; } /** @@ -457,7 +477,7 @@ class MAGNUM_AUDIO_EXPORT Context { * as it does most operations in compile time. */ bool isExtensionDisabled(const Extension& extension) const { - return _disabledExtensions[extension.index()]; + return _extensionDisabledStatus[extension.index()]; } private: @@ -467,9 +487,9 @@ class MAGNUM_AUDIO_EXPORT Context { ALCcontext* _context; Math::BitVector _extensionStatus; - Math::BitVector _disabledExtensions; - std::vector _supportedExtensions; - std::vector _disabledExtensionStrings; + Math::BitVector _extensionDisabledStatus; + Containers::Array _supportedExtensions; + Containers::Array _disabledExtensions; }; /** @@ -477,7 +497,7 @@ class MAGNUM_AUDIO_EXPORT Context { @see @ref Context() */ -class MAGNUM_AUDIO_EXPORT Context::Configuration { +class Context::Configuration { public: /** * @brief HRTF configuration @@ -491,22 +511,33 @@ class MAGNUM_AUDIO_EXPORT Context::Configuration { Disabled = 2 /**< Disabled */ }; - explicit Configuration(); - ~Configuration(); - - /** @brief Device specifier */ - const std::string& deviceSpecifier() const { return _deviceSpecifier; } + /** + * @brief Device specifier + * + * The returned string view is + * @relativeref{Corrade,Containers::StringViewFlag::NullTerminated} and + * is valid until the next call to @ref setDeviceSpecifier(). + */ + Containers::StringView deviceSpecifier() const { return _deviceSpecifier; } /** * @brief Set device specifier * @return Reference to self (for method chaining) * - * If set to empty string (the default), default device specifier is + * If set to an empty string (the default), default device specifier is * used. + * + * @note The function makes a copy of the view if it's not not global + * or null-terminated, use the + * @link Corrade::Containers::Literals::StringLiterals::operator""_s() Containers::Literals::operator""_s() @endlink + * literal to prevent that where possible. + * * @see @ref Context::deviceSpecifierStrings() */ - Configuration& setDeviceSpecifier(const std::string& specifier); - Configuration& setDeviceSpecifier(std::string&& specifier); /**< @overload */ + Configuration& setDeviceSpecifier(Containers::StringView specifier) { + _deviceSpecifier = Containers::String::nullTerminatedGlobalView(specifier); + return *this; + } /** @brief Sampling rate in Hz */ Int frequency() const { return _frequency; } @@ -592,7 +623,7 @@ class MAGNUM_AUDIO_EXPORT Context::Configuration { } private: - std::string _deviceSpecifier; + Containers::String _deviceSpecifier; Int _frequency{-1}; Hrtf _hrtf{}; diff --git a/src/Magnum/Audio/Test/ContextALTest.cpp b/src/Magnum/Audio/Test/ContextALTest.cpp index b6afcddad..b1733ac3c 100644 --- a/src/Magnum/Audio/Test/ContextALTest.cpp +++ b/src/Magnum/Audio/Test/ContextALTest.cpp @@ -54,6 +54,8 @@ struct ContextALTest: TestSuite::Tester { void isExtensionSupported(); void isExtensionUnsupported(); void isExtensionDisabled(); + + void stringFlags(); }; const struct { @@ -82,13 +84,15 @@ ContextALTest::ContextALTest(): &ContextALTest::extensionsString, &ContextALTest::isExtensionSupported, &ContextALTest::isExtensionUnsupported, - &ContextALTest::isExtensionDisabled}); + &ContextALTest::isExtensionDisabled, + + &ContextALTest::stringFlags}); } void ContextALTest::deviceSpecifierStrings() { /* Just verify that it produces something reasonable without crashing or entering an infinite loop */ - CORRADE_VERIFY(!Context::deviceSpecifierStrings().empty()); + CORRADE_VERIFY(!Context::deviceSpecifierStrings().isEmpty()); } void ContextALTest::constructDefault() { @@ -155,10 +159,10 @@ void ContextALTest::constructConfiguration() { /* HRTF gets enabled only if the extension is supported */ if(context.isExtensionSupported()) { CORRADE_COMPARE(context.hrtfStatus(), Context::HrtfStatus::Enabled); - CORRADE_VERIFY(!context.hrtfSpecifierString().empty()); + CORRADE_VERIFY(!context.hrtfSpecifierString().isEmpty()); } else if(context.isExtensionSupported()) { CORRADE_COMPARE(context.hrtfStatus(), Context::HrtfStatus::Enabled); - CORRADE_VERIFY(context.hrtfSpecifierString().empty()); + CORRADE_VERIFY(context.hrtfSpecifierString().isEmpty()); } else { CORRADE_COMPARE(context.hrtfStatus(), Context::HrtfStatus::Disabled); } @@ -232,9 +236,7 @@ void ContextALTest::ignoreUnrelatedOptions() { void ContextALTest::extensionsString() { Context context; - std::vector extensions = context.extensionStrings(); - - CORRADE_VERIFY(!extensions.empty()); + CORRADE_VERIFY(!context.extensionStrings().isEmpty()); } void ContextALTest::isExtensionSupported() { @@ -278,6 +280,43 @@ void ContextALTest::isExtensionDisabled() { CORRADE_VERIFY(context.isExtensionDisabled(e)); } +void ContextALTest::stringFlags() { + Containers::Array deviceSpecifierStrings = Context::deviceSpecifierStrings(); + CORRADE_VERIFY(!deviceSpecifierStrings.isEmpty()); + for(Containers::StringView specifier: deviceSpecifierStrings) { + CORRADE_VERIFY(!specifier.isEmpty()); + CORRADE_COMPARE(specifier.flags(), Containers::StringViewFlag::Global|Containers::StringViewFlag::NullTerminated); + } + + Context context; + + /* HRTF specifier string might be null, in which case it's *not* + null-terminated obviously */ + CORRADE_COMPARE(context.hrtfSpecifierString().flags(), Containers::StringViewFlag::Global|(context.hrtfSpecifierString().data() ? Containers::StringViewFlag::NullTerminated : Containers::StringViewFlags{})); + + CORRADE_VERIFY(!context.deviceSpecifierString().isEmpty()); + CORRADE_COMPARE(context.deviceSpecifierString().flags(), Containers::StringViewFlag::Global|Containers::StringViewFlag::NullTerminated); + + CORRADE_VERIFY(!context.vendorString().isEmpty()); + CORRADE_COMPARE(context.vendorString().flags(), Containers::StringViewFlag::Global|Containers::StringViewFlag::NullTerminated); + + CORRADE_VERIFY(!context.rendererString().isEmpty()); + CORRADE_COMPARE(context.rendererString().flags(), Containers::StringViewFlag::Global|Containers::StringViewFlag::NullTerminated); + + CORRADE_VERIFY(!context.versionString().isEmpty()); + CORRADE_COMPARE(context.versionString().flags(), Containers::StringViewFlag::Global|Containers::StringViewFlag::NullTerminated); + + for(Containers::StringView extension: context.extensionStrings()) { + CORRADE_ITERATION(extension); + CORRADE_VERIFY(!extension.isEmpty()); + + /* The extensions are split from a long string and thus aren't all + null-terminated, only the last one */ + CORRADE_COMPARE_AS(extension.flags(), Containers::StringViewFlag::Global, + TestSuite::Compare::GreaterOrEqual); + } +} + }}}} CORRADE_TEST_MAIN(Magnum::Audio::Test::ContextALTest)