From b932e06472ee5d6f28fef424826c54d399f4ce64 Mon Sep 17 00:00:00 2001 From: Lefucjusz Date: Thu, 16 Nov 2023 16:16:31 +0100 Subject: [PATCH] [BH-1823] Fix relaxation start delay for specific MP3 files Fix of the issue that when trying to play files with large ID3V2 metadata the playback would start with significant delay of several seconds. It was also possible to leave relaxation progress window without stopping the playback. --- harmony_changelog.md | 1 + module-audio/Audio/decoder/decoderMP3.cpp | 57 +++++++++++-- module-audio/Audio/decoder/decoderMP3.hpp | 13 +-- module-audio/Audio/decoder/xing_header.c | 84 ------------------- module-audio/Audio/decoder/xing_header.h | 26 ------ module-audio/CMakeLists.txt | 1 - .../services/audio/ServiceAudio.cpp | 38 +++++++-- pure_changelog.md | 1 + 8 files changed, 87 insertions(+), 134 deletions(-) delete mode 100644 module-audio/Audio/decoder/xing_header.c delete mode 100644 module-audio/Audio/decoder/xing_header.h diff --git a/harmony_changelog.md b/harmony_changelog.md index 486932cb0d1fb2a46d87f65a20062b6be97be7e5..ea554740a4565745f4dbbf84d44246426dd7fca2 100644 --- a/harmony_changelog.md +++ b/harmony_changelog.md @@ -5,6 +5,7 @@ ### Fixed * Fixed eink errors in logs * Fixed alarm when the onboarding is in progress +* Fixed relaxation start delay when trying to play MP3 files with large metadata ### Added * Added shortcuts instruction to settings diff --git a/module-audio/Audio/decoder/decoderMP3.cpp b/module-audio/Audio/decoder/decoderMP3.cpp index c9aa3b780ddcac9a20abe2b35409ff7087b89e23..450fbb202bbf3c61e433fec0e90b72e9cbc039a7 100644 --- a/module-audio/Audio/decoder/decoderMP3.cpp +++ b/module-audio/Audio/decoder/decoderMP3.cpp @@ -8,6 +8,45 @@ #include "decoderMP3.hpp" #include +namespace +{ + signed skipID3V2TagIfPresent(std::FILE *fd) + { + constexpr auto ID3V2FrameOffset = 0; + constexpr auto ID3V2FrameHeaderSize = 10; + constexpr auto ID3V2FrameMagicString = "ID3"; + constexpr auto ID3V2FrameMagicStringLength = 3; + std::uint8_t frameBuffer[ID3V2FrameHeaderSize]; + + /* Seek to the beginning of the frame and read frame's header */ + if (std::fseek(fd, ID3V2FrameOffset, SEEK_SET) != 0) { + return -EIO; + } + if (std::fread(frameBuffer, sizeof(*frameBuffer), ID3V2FrameHeaderSize, fd) != ID3V2FrameHeaderSize) { + return -EIO; + } + + /* Check magic */ + if (strncmp(reinterpret_cast(frameBuffer), ID3V2FrameMagicString, ID3V2FrameMagicStringLength) != + 0) { + return 0; + } + + /* The tag size (minus the 10-byte header) is encoded into four bytes, + * but the most significant bit needs to be masked in each byte. + * Those frame indices are just copied from the ID3V2 docs. */ + const auto ID3V2TagTotalSize = (((frameBuffer[6] & 0x7F) << 21) | ((frameBuffer[7] & 0x7F) << 14) | + ((frameBuffer[8] & 0x7F) << 7) | ((frameBuffer[9] & 0x7F) << 0)) + + ID3V2FrameHeaderSize; + + /* Skip the tag */ + if (std::fseek(fd, ID3V2FrameOffset + ID3V2TagTotalSize, SEEK_SET) != 0) { + return -EIO; + } + return ID3V2TagTotalSize; + } +} // namespace + namespace audio { decoderMP3::decoderMP3(const std::string &filePath) : Decoder(filePath) @@ -16,6 +55,14 @@ namespace audio return; } + const auto tagSkipStatus = skipID3V2TagIfPresent(fd); + if (tagSkipStatus < 0) { + LOG_ERROR("Failed to skip ID3V2 tag, error %d", tagSkipStatus); + } + else if (tagSkipStatus == 0) { + LOG_INFO("No ID3V2 tag to skip"); + } + mp3 = std::make_unique(); drmp3_init(mp3.get(), drmp3_read, drmp3_seek, this, nullptr); @@ -35,20 +82,21 @@ namespace audio { drmp3_uninit(mp3.get()); } + void decoderMP3::setPosition(float pos) { if (!isInitialized) { LOG_ERROR("MP3 decoder not initialized"); return; } - auto totalFramesCount = drmp3_get_pcm_frame_count(mp3.get()); + const auto totalFramesCount = drmp3_get_pcm_frame_count(mp3.get()); drmp3_seek_to_pcm_frame(mp3.get(), totalFramesCount * pos); position = static_cast(totalFramesCount) * pos / static_cast(sampleRate); } std::uint32_t decoderMP3::decode(std::uint32_t samplesToRead, std::int16_t *pcmData) { - std::uint32_t samplesRead = + const auto samplesRead = drmp3_read_pcm_frames_s16(mp3.get(), samplesToRead / chanNumber, reinterpret_cast(pcmData)); if (samplesRead > 0) { position += static_cast(samplesRead) / static_cast(sampleRate); @@ -57,7 +105,7 @@ namespace audio return samplesRead * chanNumber; } - size_t decoderMP3::drmp3_read(void *pUserData, void *pBufferOut, size_t bytesToRead) + std::size_t decoderMP3::drmp3_read(void *pUserData, void *pBufferOut, std::size_t bytesToRead) { const auto decoderContext = reinterpret_cast(pUserData); return !statFd(decoderContext->fd, "MP3 audio file deleted by user!") @@ -68,9 +116,8 @@ namespace audio drmp3_bool32 decoderMP3::drmp3_seek(void *pUserData, int offset, drmp3_seek_origin origin) { const auto decoderContext = reinterpret_cast(pUserData); - const int seekError = + const auto seekError = std::fseek(decoderContext->fd, offset, origin == drmp3_seek_origin_start ? SEEK_SET : SEEK_CUR); return (seekError == 0) ? DRMP3_TRUE : DRMP3_FALSE; } - } // namespace audio diff --git a/module-audio/Audio/decoder/decoderMP3.hpp b/module-audio/Audio/decoder/decoderMP3.hpp index 3c84abea882461925146c28097997e4de261452c..14b796476b68960a17168b7245d074c1fb3a9b50 100644 --- a/module-audio/Audio/decoder/decoderMP3.hpp +++ b/module-audio/Audio/decoder/decoderMP3.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #pragma once @@ -6,14 +6,8 @@ #include "Decoder.hpp" #include -extern "C" -{ -#include "xing_header.h" -}; - namespace audio { - class decoderMP3 : public Decoder { @@ -27,7 +21,7 @@ namespace audio void setPosition(float pos) override; private: - std::unique_ptr mp3 = nullptr; + std::unique_ptr mp3; // Callback for when data needs to be read from the client. // @@ -39,7 +33,7 @@ namespace audio // // A return value of less than bytesToRead indicates the end of the stream. Do _not_ return from this callback // until either the entire bytesToRead is filled or you have reached the end of the stream. - static size_t drmp3_read(void *pUserData, void *pBufferOut, size_t bytesToRead); + static std::size_t drmp3_read(void *pUserData, void *pBufferOut, std::size_t bytesToRead); // Callback for when data needs to be seeked. // @@ -54,5 +48,4 @@ namespace audio // drflac_seek_origin_current. static drmp3_bool32 drmp3_seek(void *pUserData, int offset, drmp3_seek_origin origin); }; - } // namespace audio diff --git a/module-audio/Audio/decoder/xing_header.c b/module-audio/Audio/decoder/xing_header.c deleted file mode 100644 index b6d190478dd9caef00b13ac700ab2cb99cfb5027..0000000000000000000000000000000000000000 --- a/module-audio/Audio/decoder/xing_header.c +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. -// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md - -#include "xing_header.h" -#include - -/* XING VBR-Header - - size description - 4 'Xing' or 'Info' - 4 flags (indicates which fields are used) - 4 frames (optional) - 4 bytes (optional) - 100 toc (optional) - 4 a VBR quality indicator: 0=best 100=worst (optional) - -*/ - -typedef struct -{ - char desc[4]; - uint32_t flags; -} xing_header_t; - -// for XING VBR Header flags -#define FRAMES_FLAG 0x0001 -#define BYTES_FLAG 0x0002 -#define TOC_FLAG 0x0004 -#define VBR_SCALE_FLAG 0x0008 - -//! Byte swap unsigned int -static inline uint32_t swap32(uint32_t ui) -{ - return (ui >> 24) | ((ui << 8) & 0x00FF0000) | ((ui >> 8) & 0x0000FF00) | (ui << 24); -} - -uint8_t parseXingHeader(uint8_t *data, size_t datasize, xing_info_t *info) -{ - char *p1 = memmem(data, datasize, "Xing", 4); - char *p2 = memmem(data, datasize, "Info", 4); - - if (p1 != NULL || p2 != NULL) { - xing_header_t *head = (xing_header_t *)(p1 != NULL ? p1 : p2); - uint8_t *raw_data = (uint8_t *)head + sizeof(xing_header_t); - - head->flags = swap32(head->flags); - - uint8_t offset = 0; - // get flags (mandatory in XING header) - - // extract total number of frames in file - if (head->flags & FRAMES_FLAG) { - uint32_t *tmp = (uint32_t *)&raw_data[offset]; - info->TotalFrames = swap32(*tmp); - offset += 4; - } - else { - return 0; - } - - // extract total number of bytes in file - if (head->flags & BYTES_FLAG) { - uint32_t *tmp = (uint32_t *)&raw_data[offset]; - info->TotalBytes = swap32(*tmp); - offset += 4; - } - - // extract TOC - if (head->flags & TOC_FLAG) { - memcpy(info->TOC, &raw_data[offset], sizeof(info->TOC)); - offset += sizeof(info->TOC); - } - - // extract quality status - if (head->flags & VBR_SCALE_FLAG) { - info->Quality = raw_data[offset + 3]; - } - - return 1; - } - else { - return 0; - } -} diff --git a/module-audio/Audio/decoder/xing_header.h b/module-audio/Audio/decoder/xing_header.h deleted file mode 100644 index f06c326711381b0ecf0ee9b738c9bdda39298d8b..0000000000000000000000000000000000000000 --- a/module-audio/Audio/decoder/xing_header.h +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. -// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md - -#pragma once - -#include -#include - -#define XING_HEADER_SIZE 120 - -typedef struct -{ - uint32_t TotalFrames; - uint32_t TotalBytes; - uint8_t TOC[100]; - uint32_t Quality; -} xing_info_t; - -/** - * @brief This function is used to parse Xing header in MP3 file - * @param data [in] Pointer to data buffer which contains data to be parsed - * @param datalen [in] Size of data buffer - * @param info [out] Pointer to structure describing xing header - * @return 1 if success, otherwise 0. - */ -uint8_t parseXingHeader(uint8_t *data, size_t datasize, xing_info_t *info); diff --git a/module-audio/CMakeLists.txt b/module-audio/CMakeLists.txt index 13c255a3d4cc97500619531bc536f4deb1c79377..feac765e4bc266c06c2cb09166447eb48aabcf6d 100755 --- a/module-audio/CMakeLists.txt +++ b/module-audio/CMakeLists.txt @@ -25,7 +25,6 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/Audio/decoder/decoderMP3.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Audio/decoder/decoderWAV.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Audio/decoder/DecoderWorker.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/Audio/decoder/xing_header.c ${CMAKE_CURRENT_SOURCE_DIR}/Audio/encoder/Encoder.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Audio/encoder/EncoderWAV.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Audio/Endpoint.cpp diff --git a/products/BellHybrid/services/audio/ServiceAudio.cpp b/products/BellHybrid/services/audio/ServiceAudio.cpp index 6c7412e7f189926eb6e874c7523dc6e11f0aacdb..d9049e58fb2c73c7f4023062fdb3936c351ef5d5 100644 --- a/products/BellHybrid/services/audio/ServiceAudio.cpp +++ b/products/BellHybrid/services/audio/ServiceAudio.cpp @@ -23,7 +23,7 @@ namespace { using namespace audio; // clang-format off - static constexpr std::initializer_list> values{ + constexpr std::initializer_list> values{ {DbPathElement{Setting::Volume, PlaybackType::Meditation, Profile::Type::PlaybackLoudspeaker},defaultVolume}, {DbPathElement{Setting::Volume, PlaybackType::Multimedia, Profile::Type::PlaybackLoudspeaker},defaultVolume}, {DbPathElement{Setting::Volume, PlaybackType::Alarm, Profile::Type::PlaybackLoudspeaker}, defaultVolume}, @@ -71,7 +71,7 @@ namespace service connect(typeid(AudioStartPlaybackRequest), [this](sys::Message *msg) -> sys::MessagePointer { auto *msgl = static_cast(msg); - return handleStart(audio::Operation::Type::Playback, msgl->fileName.c_str(), msgl->playbackType); + return handleStart(audio::Operation::Type::Playback, msgl->fileName, msgl->playbackType); }); connect(typeid(AudioInternalEOFNotificationMessage), [this](sys::Message *msg) -> sys::MessagePointer { @@ -95,18 +95,23 @@ namespace service return handleGetVolume(msgl->playbackType); }); - connect(typeid(AudioPauseRequest), [this](sys::Message *msg) -> sys::MessagePointer { return handlePause(); }); + connect(typeid(AudioPauseRequest), + [this]([[maybe_unused]] sys::Message *msg) -> sys::MessagePointer { return handlePause(); }); connect(typeid(AudioResumeRequest), - [this](sys::Message *msg) -> sys::MessagePointer { return handleResume(); }); + [this]([[maybe_unused]] sys::Message *msg) -> sys::MessagePointer { return handleResume(); }); } + Audio::~Audio() { } - sys::MessagePointer Audio::DataReceivedHandler(sys::DataMessage *msgl, sys::ResponseMessage *resp) + + sys::MessagePointer Audio::DataReceivedHandler([[maybe_unused]] sys::DataMessage *msgl, + [[maybe_unused]] sys::ResponseMessage *resp) { return sys::msgNotHandled(); } + sys::ReturnCodes Audio::InitHandler() { settingsProvider->init(service::ServiceProxy(weak_from_this())); @@ -114,11 +119,13 @@ namespace service LOG_INFO("Initialized"); return sys::ReturnCodes::Success; } + sys::ReturnCodes Audio::DeinitHandler() { LOG_INFO("Deinitialized"); return sys::ReturnCodes::Success; } + void Audio::ProcessCloseReason([[maybe_unused]] sys::CloseReason closeReason) { if (const auto &activeInputOpt = audioMux.GetActiveInput(); activeInputOpt.has_value()) { @@ -126,6 +133,7 @@ namespace service activeInput->audio->Stop(); } } + auto Audio::handleStart(const audio::Operation::Type opType, const std::string &fileName, const audio::PlaybackType &playbackType) -> std::unique_ptr @@ -141,7 +149,7 @@ namespace service retToken = audioMux.ResetInput(input); try { - retCode = (*input)->audio->Start(opType, retToken, fileName.c_str(), playbackType); + retCode = (*input)->audio->Start(opType, retToken, fileName, playbackType); } catch (const audio::AudioInitException &audioException) { retCode = audio::RetCode::FailedToAllocateMemory; @@ -154,8 +162,9 @@ namespace service return std::make_unique(retCode, retToken); } - auto Audio::handleStop(const std::vector &stopTypes, const audio::Token &token) - -> std::unique_ptr + + auto Audio::handleStop([[maybe_unused]] const std::vector &stopTypes, + const audio::Token &token) -> std::unique_ptr { std::vector> retCodes; @@ -178,6 +187,7 @@ namespace service return std::make_unique(audio::RetCode::Success, token); } + auto Audio::stopInput(audio::AudioMux::Input *input, Audio::StopReason stopReason) -> audio::RetCode { if (input->audio->GetCurrentState() == audio::Audio::State::Idle) { @@ -197,10 +207,12 @@ namespace service manageCpuSentinel(); return retCode; } + constexpr auto Audio::shouldLoop(const std::optional &type) const -> bool { return type == audio::PlaybackType::Alarm; } + auto Audio::isBusy() const -> bool { const auto &inputs = audioMux.GetAllInputs(); @@ -208,6 +220,7 @@ namespace service return input.audio->GetCurrentState() != audio::Audio::State::Idle; }); } + void Audio::handleEOF(const audio::Token &token) { if (const auto input = audioMux.GetInput(token); input) { @@ -222,6 +235,7 @@ namespace service } } } + auto Audio::AudioServicesCallback(const sys::Message *msg) -> std::optional { std::optional ret; @@ -243,6 +257,7 @@ namespace service return ret; } + auto Audio::handleSetVolume(const audio::PlaybackType &playbackType, const std::string &value) -> std::unique_ptr { @@ -261,6 +276,7 @@ namespace service } return std::make_unique(retCode); } + auto Audio::handleGetVolume(const audio::PlaybackType &playbackType) -> std::unique_ptr { constexpr auto setting = audio::Setting::Volume; @@ -272,10 +288,12 @@ namespace service return std::make_unique(audio::RetCode::Failed); } + sys::ReturnCodes Audio::SwitchPowerModeHandler([[maybe_unused]] const sys::ServicePowerMode mode) { return sys::ReturnCodes::Success; } + auto Audio::handlePause() -> std::unique_ptr { auto retCode = audio::RetCode::InvokedInIncorrectState; @@ -291,6 +309,7 @@ namespace service manageCpuSentinel(); return std::make_unique(retCode); } + auto Audio::handleResume() -> std::unique_ptr { auto retCode = audio::RetCode::InvokedInIncorrectState; @@ -301,15 +320,18 @@ namespace service manageCpuSentinel(); return std::make_unique(retCode); } + constexpr auto Audio::isResumable(audio::PlaybackType type) const -> bool { return type == audio::PlaybackType::Multimedia; } + void Audio::manageCpuSentinel() { isBusy() ? cpuSentinel->HoldMinimumFrequency(bsp::CpuFrequencyMHz::Level_6) : cpuSentinel->ReleaseMinimumFrequency(); } + void Audio::initializeDatabase() { for (const auto &entry : initializer::values) { diff --git a/pure_changelog.md b/pure_changelog.md index 14591bff54ffdb9b47e887019d06535b8b7bebaa..de9b022c8e4fa736bba86101d82f7b51182fed3c 100644 --- a/pure_changelog.md +++ b/pure_changelog.md @@ -19,6 +19,7 @@ * Fixed possible crash when entering phone number * Fixed incorrect calculation of requested CPU frequency * Fixed CPU frequency setting when dumping logs to a file +* Fixed playback start delay when trying to play MP3 files with large metadata ## [1.9.0 2023-10-19]