From 96d02a3e753b53533d2aaf50e2dd2d92738f0e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?= Date: Sat, 27 Nov 2021 17:00:19 +0100 Subject: LibAudio: New error propagation API in Loader and Buffer Previously, a libc-like out-of-line error information was used in the loader and its plugins. Now, all functions that may fail to do their job return some sort of Result. The universally-used error type ist the new LoaderError, which can contain information about the general error category (such as file format, I/O, unimplemented features), an error description, and location information, such as file index or sample index. Additionally, the loader plugins try to do as little work as possible in their constructors. Right after being constructed, a user should call initialize() and check the errors returned from there. (This is done transparently by Loader itself.) If a constructor caused an error, the call to initialize should check and return it immediately. This opportunity was used to rework a lot of the internal error propagation in both loader classes, especially FlacLoader. Therefore, a couple of other refactorings may have sneaked in as well. The adoption of LibAudio users is minimal. Piano's adoption is not important, as the code will receive major refactoring in the near future anyways. SoundPlayer's adoption is also less important, as changes to refactor it are in the works as well. aplay's adoption is the best and may serve as an example for other users. It also includes new buffering behavior. Buffer also gets some attention, making it OOM-safe and thereby also propagating its errors to the user. --- Userland/Utilities/aplay.cpp | 64 ++++++++++++++++++++++++++++++-------------- Userland/Utilities/asctl.cpp | 4 +++ 2 files changed, 48 insertions(+), 20 deletions(-) (limited to 'Userland/Utilities') diff --git a/Userland/Utilities/aplay.cpp b/Userland/Utilities/aplay.cpp index e7b4f2d1d0..47fdcbd8e8 100644 --- a/Userland/Utilities/aplay.cpp +++ b/Userland/Utilities/aplay.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -11,6 +12,10 @@ #include #include +// The Kernel has issues with very large anonymous buffers. +// FIXME: This appears to be fine for now, but it's really a hack. +constexpr size_t LOAD_CHUNK_SIZE = 128 * KiB; + ErrorOr serenity_main(Main::Arguments arguments) { const char* path = nullptr; @@ -24,11 +29,12 @@ ErrorOr serenity_main(Main::Arguments arguments) Core::EventLoop loop; auto audio_client = Audio::ClientConnection::construct(); - NonnullRefPtr loader = Audio::Loader::create(path); - if (loader->has_error()) { - warnln("Failed to load audio file: {}", loader->error_string()); + auto maybe_loader = Audio::Loader::create(path); + if (maybe_loader.is_error()) { + warnln("Failed to load audio file: {}", maybe_loader.error().description); return 1; } + auto loader = maybe_loader.release_value(); outln("\033[34;1m Playing\033[0m: {}", path); outln("\033[34;1m Format\033[0m: {} Hz, {}-bit, {}", @@ -39,25 +45,43 @@ ErrorOr serenity_main(Main::Arguments arguments) auto resampler = Audio::ResampleHelper(loader->sample_rate(), audio_client->get_sample_rate()); + // If we're downsampling, we need to appropriately load more samples at once. + size_t const load_size = static_cast(LOAD_CHUNK_SIZE * static_cast(loader->sample_rate()) / static_cast(audio_client->get_sample_rate())); + // We assume that the loader can load samples at at least 2x speed (testing confirms 9x-12x for FLAC, 14x for WAV). + // Therefore, when the server-side buffer can only play as long as the time it takes us to load a chunk, + // we give it new data. + int const min_buffer_size = load_size / 2; + for (;;) { - auto samples = loader->get_more_samples(); - if (samples) { - out("\033[u"); - out("{}/{}", loader->loaded_samples(), loader->total_samples()); - fflush(stdout); - resampler.reset(); - samples = Audio::resample_buffer(resampler, *samples); - audio_client->enqueue(*samples); - } else if (loader->has_error()) { - outln(); - outln("Error: {}", loader->error_string()); - break; - } else if (should_loop) { - loader->reset(); - } else if (audio_client->get_remaining_samples()) { - sleep(1); + auto samples = loader->get_more_samples(load_size); + if (!samples.is_error()) { + if (samples.value()->sample_count() > 0) { + // We can read and enqueue more samples + out("\033[u"); + out("{}/{}", loader->loaded_samples(), loader->total_samples()); + fflush(stdout); + resampler.reset(); + auto resampled_samples = TRY(Audio::resample_buffer(resampler, *samples.value())); + audio_client->async_enqueue(*resampled_samples); + } else if (should_loop) { + // We're done: now loop + auto result = loader->reset(); + if (result.is_error()) { + outln(); + outln("Error while resetting: {} (at {:x})", result.error().description, result.error().index); + } + } else if (samples.value()->sample_count() == 0 && audio_client->get_remaining_samples() == 0) { + // We're done and the server is done + break; + } + while (audio_client->get_remaining_samples() > min_buffer_size) { + // The server has enough data for now + sleep(1); + } } else { - break; + outln(); + outln("Error: {} (at {:x})", samples.error().description, samples.error().index); + return 1; } } outln(); diff --git a/Userland/Utilities/asctl.cpp b/Userland/Utilities/asctl.cpp index 19780c775c..4144f0e310 100644 --- a/Userland/Utilities/asctl.cpp +++ b/Userland/Utilities/asctl.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,9 @@ ErrorOr serenity_main(Main::Arguments arguments) args_parser.add_positional_argument(command_arguments, "Arguments for the command", "args", Core::ArgsParser::Required::No); args_parser.parse(arguments); + TRY(Core::System::unveil(nullptr, nullptr)); + TRY(Core::System::pledge("stdio rpath wpath recvfd", nullptr)); + if (command.equals_ignoring_case("get") || command == "g") { // Get variables Vector values_to_print; -- cgit v1.2.3