From 05f0213a9fb6fd374058c776e4547ff7ea9afa28 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 17 Apr 2021 15:00:14 +0200 Subject: [PATCH 1/5] Do not crash when transcoding a non existing file (regression from eea27b8), fixes #141 --- src/libs/av/impl/TranscodeResourceHandler.cpp | 1 - src/libs/av/impl/Transcoder.cpp | 25 +++++-------- src/libs/av/impl/Transcoder.hpp | 4 +-- src/libs/subsonic/impl/Stream.cpp | 35 +++++++++++-------- .../ui/resource/AudioTranscodeResource.cpp | 35 +++++++++++-------- 5 files changed, 53 insertions(+), 47 deletions(-) diff --git a/src/libs/av/impl/TranscodeResourceHandler.cpp b/src/libs/av/impl/TranscodeResourceHandler.cpp index 7cbd99c6..6554999c 100644 --- a/src/libs/av/impl/TranscodeResourceHandler.cpp +++ b/src/libs/av/impl/TranscodeResourceHandler.cpp @@ -33,7 +33,6 @@ namespace Av TranscodeResourceHandler::TranscodeResourceHandler(const std::filesystem::path& trackPath, const TranscodeParameters& parameters) : _transcoder {trackPath, parameters} { - _transcoder.start(); } Wt::Http::ResponseContinuation* diff --git a/src/libs/av/impl/Transcoder.cpp b/src/libs/av/impl/Transcoder.cpp index caf719ba..7efa0442 100644 --- a/src/libs/av/impl/Transcoder.cpp +++ b/src/libs/av/impl/Transcoder.cpp @@ -48,11 +48,12 @@ Transcoder::Transcoder(const std::filesystem::path& filePath, const TranscodePar , _filePath {filePath} , _parameters {parameters} { + start(); } Transcoder::~Transcoder() = default; -bool +void Transcoder::start() { if (ffmpegPath.empty()) @@ -61,20 +62,13 @@ Transcoder::start() try { if (!std::filesystem::exists(_filePath)) - { - LOG(ERROR) << "File '" << _filePath << "' does not exist!"; - return false; - } + throw Exception {"File '" + _filePath.string() + "' does not exist!"}; else if (!std::filesystem::is_regular_file( _filePath) ) - { - LOG(ERROR) << "File '" << _filePath << "' is not regular!"; - return false; - } + throw Exception {"File '" + _filePath.string() + "' is not regular!"}; } catch (const std::filesystem::filesystem_error& e) { - LOG(ERROR) << "File error on '" << _filePath.string() << "': " << e.what(); - return false; + throw Exception {"File error '" + _filePath.string() + "': " + e.what()}; } LOG(INFO) << "Transcoding file '" << _filePath.string() << "'"; @@ -163,7 +157,7 @@ Transcoder::start() break; default: - return false; + throw Exception {"Unhandled format (" + std::to_string(static_cast(_parameters.format)) + ")"}; } _outputMimeType = formatToMimetype(_parameters.format); @@ -181,11 +175,8 @@ Transcoder::start() } catch (ChildProcessException& exception) { - LOG(ERROR) << "Cannot execute '" << ffmpegPath << "': " << exception.what(); - return false; + throw Exception {"Cannot execute '" + ffmpegPath.string() + "': " + exception.what()}; } - - return true; } void @@ -223,6 +214,8 @@ Transcoder::readSome(std::byte* buffer, std::size_t bufferSize) bool Transcoder::finished() const { + assert(_childProcess); + return _childProcess->finished(); } diff --git a/src/libs/av/impl/Transcoder.hpp b/src/libs/av/impl/Transcoder.hpp index aa9ace31..0ebb9179 100644 --- a/src/libs/av/impl/Transcoder.hpp +++ b/src/libs/av/impl/Transcoder.hpp @@ -40,8 +40,6 @@ namespace Av Transcoder(Transcoder&&) = delete; Transcoder& operator=(Transcoder&&) = delete; - bool start(); - using WaitCallback = std::function; void asyncWaitForData(WaitCallback cb); @@ -58,6 +56,8 @@ namespace Av private: static void init(); + void start(); + const std::size_t _id {}; const std::filesystem::path _filePath; const TranscodeParameters _parameters; diff --git a/src/libs/subsonic/impl/Stream.cpp b/src/libs/subsonic/impl/Stream.cpp index 29085c83..911b1514 100644 --- a/src/libs/subsonic/impl/Stream.cpp +++ b/src/libs/subsonic/impl/Stream.cpp @@ -26,6 +26,7 @@ #include "database/Track.hpp" #include "database/User.hpp" #include "utils/IResourceHandler.hpp" +#include "utils/Logger.hpp" #include "utils/FileResourceHandlerCreator.hpp" #include "utils/Utils.hpp" #include "ParameterParsing.hpp" @@ -147,24 +148,30 @@ handleStream(RequestContext& context, const Wt::Http::Request& request, Wt::Http { std::shared_ptr resourceHandler; - Wt::Http::ResponseContinuation* continuation = request.continuation(); - if (!continuation) + try { - StreamParameters streamParameters {getStreamParameters(context)}; - if (streamParameters.transcodeParameters) - resourceHandler = Av::createTranscodeResourceHandler(streamParameters.trackPath, *streamParameters.transcodeParameters); + Wt::Http::ResponseContinuation* continuation = request.continuation(); + if (!continuation) + { + StreamParameters streamParameters {getStreamParameters(context)}; + if (streamParameters.transcodeParameters) + resourceHandler = Av::createTranscodeResourceHandler(streamParameters.trackPath, *streamParameters.transcodeParameters); + else + resourceHandler = createFileResourceHandler(streamParameters.trackPath); + } else - resourceHandler = createFileResourceHandler(streamParameters.trackPath); + { + resourceHandler = Wt::cpp17::any_cast>(continuation->data()); + } + + continuation = resourceHandler->processRequest(request, response); + if (continuation) + continuation->setData(resourceHandler); } - else + catch (const Av::Exception& e) { - resourceHandler = Wt::cpp17::any_cast>(continuation->data()); + LMS_LOG(API_SUBSONIC, ERROR) << "Caught Av exception: " << e.what(); } - - continuation = resourceHandler->processRequest(request, response); - if (continuation) - continuation->setData(resourceHandler); -} - } +} // namespace API::Subsonic::Stream diff --git a/src/lms/ui/resource/AudioTranscodeResource.cpp b/src/lms/ui/resource/AudioTranscodeResource.cpp index 818eb67a..f20a159c 100644 --- a/src/lms/ui/resource/AudioTranscodeResource.cpp +++ b/src/lms/ui/resource/AudioTranscodeResource.cpp @@ -180,23 +180,30 @@ AudioTranscodeResource::handleRequest(const Wt::Http::Request& request, { std::shared_ptr resourceHandler; - Wt::Http::ResponseContinuation* continuation {request.continuation()}; - if (!continuation) + try { - const std::optional& parameters {readTranscodeParameters(request)}; - if (parameters) - resourceHandler = Av::createTranscodeResourceHandler(parameters->file, parameters->transcodeParameters); - } - else - { - resourceHandler = Wt::cpp17::any_cast>(continuation->data()); - } + Wt::Http::ResponseContinuation* continuation {request.continuation()}; + if (!continuation) + { + const std::optional& parameters {readTranscodeParameters(request)}; + if (parameters) + resourceHandler = Av::createTranscodeResourceHandler(parameters->file, parameters->transcodeParameters); + } + else + { + resourceHandler = Wt::cpp17::any_cast>(continuation->data()); + } - if (resourceHandler) + if (resourceHandler) + { + continuation = resourceHandler->processRequest(request, response); + if (continuation) + continuation->setData(resourceHandler); + } + } + catch (const Av::Exception& e) { - continuation = resourceHandler->processRequest(request, response); - if (continuation) - continuation->setData(resourceHandler); + LOG(ERROR) << "Caught Av exception: " << e.what(); } } From 17f086ad58f0d40909855f194c7b423e38c42fd8 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 17 Apr 2021 15:07:05 +0200 Subject: [PATCH 2/5] Fixed bad formatted zh translation file --- approot/messages_zh.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/approot/messages_zh.xml b/approot/messages_zh.xml index 23e0e3a0..16a9eed2 100644 --- a/approot/messages_zh.xml +++ b/approot/messages_zh.xml @@ -218,5 +218,3 @@ 此字段不能为空 这个值应该在 {1} 和 {2} - - From b45b30ded44bb49865bbce7b43612f36043126ea Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 25 Apr 2021 14:05:30 +0200 Subject: [PATCH 3/5] Fixed includes. ref #143 --- src/libs/scrobbling/impl/IScrobbler.hpp | 3 ++- .../scrobbling/impl/listenbrainz/ListenBrainzScrobbler.hpp | 2 -- src/libs/scrobbling/include/scrobbling/IScrobbling.hpp | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libs/scrobbling/impl/IScrobbler.hpp b/src/libs/scrobbling/impl/IScrobbler.hpp index 7587ce0e..840d24ef 100644 --- a/src/libs/scrobbling/impl/IScrobbler.hpp +++ b/src/libs/scrobbling/impl/IScrobbler.hpp @@ -19,8 +19,9 @@ #pragma once +#include #include -#include +#include #include diff --git a/src/libs/scrobbling/impl/listenbrainz/ListenBrainzScrobbler.hpp b/src/libs/scrobbling/impl/listenbrainz/ListenBrainzScrobbler.hpp index 80c50fb2..690a7afc 100644 --- a/src/libs/scrobbling/impl/listenbrainz/ListenBrainzScrobbler.hpp +++ b/src/libs/scrobbling/impl/listenbrainz/ListenBrainzScrobbler.hpp @@ -19,9 +19,7 @@ #pragma once -#include #include -#include #include #include diff --git a/src/libs/scrobbling/include/scrobbling/IScrobbling.hpp b/src/libs/scrobbling/include/scrobbling/IScrobbling.hpp index 7b46436b..376ad085 100644 --- a/src/libs/scrobbling/include/scrobbling/IScrobbling.hpp +++ b/src/libs/scrobbling/include/scrobbling/IScrobbling.hpp @@ -21,9 +21,9 @@ #include #include -#include +#include #include -#include +#include #include From 807958e9b3fdc2ae49cf31831e6517ce8eee06c6 Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 26 Apr 2021 12:55:38 +0200 Subject: [PATCH 4/5] Fixed fcntl call to make it compile again on non linux. ref #144 --- src/libs/utils/impl/ChildProcess.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/utils/impl/ChildProcess.cpp b/src/libs/utils/impl/ChildProcess.cpp index 82f319fc..e45c0210 100644 --- a/src/libs/utils/impl/ChildProcess.cpp +++ b/src/libs/utils/impl/ChildProcess.cpp @@ -71,11 +71,12 @@ ChildProcess::ChildProcess(boost::asio::io_context& ioContext, const std::filesy { const std::size_t pipeSize {65536*8}; +#if defined(__linux__) && defined(F_SETPIPE_SZ) if (fcntl(pipe[0], F_SETPIPE_SZ, pipeSize) == -1) throw SystemException {errno, "fcntl failed!"}; - if (fcntl(pipe[1], F_SETPIPE_SZ, pipeSize) == -1) throw SystemException {errno, "fcntl failed!"}; +#endif } res = fork(); From 7a6ee6fa119a79c7b3d199c9a3c2014c43930bdf Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 27 Apr 2021 08:52:41 +0200 Subject: [PATCH 5/5] Supress warning. ref #144 --- src/libs/utils/impl/ChildProcess.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/utils/impl/ChildProcess.cpp b/src/libs/utils/impl/ChildProcess.cpp index e45c0210..ae0ae91b 100644 --- a/src/libs/utils/impl/ChildProcess.cpp +++ b/src/libs/utils/impl/ChildProcess.cpp @@ -69,9 +69,10 @@ ChildProcess::ChildProcess(boost::asio::io_context& ioContext, const std::filesy throw SystemException {errno, "pipe2 failed!"}; { - const std::size_t pipeSize {65536*8}; - #if defined(__linux__) && defined(F_SETPIPE_SZ) + // Just a hint here to prevent the writer from writing too many bytes ahead of the reader + constexpr std::size_t pipeSize {65536*4}; + if (fcntl(pipe[0], F_SETPIPE_SZ, pipeSize) == -1) throw SystemException {errno, "fcntl failed!"}; if (fcntl(pipe[1], F_SETPIPE_SZ, pipeSize) == -1)