From 9c2f12a786eed8e107c33f1abeeaca2fc3ba774b Mon Sep 17 00:00:00 2001 From: Niels Martin Hansen <nielsm@indvikleren.dk> Date: Sat, 3 May 2014 01:05:09 +0200 Subject: [PATCH] Clearer error messages when audio open fails The existing chain of errors from all providers can be really confusing, so instead try to interpret the exception type to add some meaning. --- src/audio_provider.cpp | 15 ++++++++------ src/command/audio.cpp | 45 +++++++++++++++++++++++++++--------------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/audio_provider.cpp b/src/audio_provider.cpp index e74c5adf9..1ea0a6516 100644 --- a/src/audio_provider.cpp +++ b/src/audio_provider.cpp @@ -141,7 +141,8 @@ std::unique_ptr<AudioProvider> AudioProviderFactory::GetProvider(agi::fs::path c std::unique_ptr<AudioProvider> provider; bool found_file = false; bool found_audio = false; - std::string msg; + std::string msg_all; // error messages from all attempted providers + std::string msg_partial; // error messages from providers that could partially load the file (knows container, missing codec) for (auto const& factory : sorted) { try { @@ -152,26 +153,28 @@ std::unique_ptr<AudioProvider> AudioProviderFactory::GetProvider(agi::fs::path c } catch (agi::fs::FileNotFound const& err) { LOG_D("audio_provider") << err.GetChainedMessage(); - msg += std::string(factory->name) + ": " + err.GetMessage() + " not found.\n"; + msg_all += std::string(factory->name) + ": " + err.GetMessage() + " not found.\n"; } catch (agi::AudioDataNotFoundError const& err) { LOG_D("audio_provider") << err.GetChainedMessage(); found_file = true; - msg += std::string(factory->name) + ": " + err.GetChainedMessage() + "\n"; + msg_all += std::string(factory->name) + ": " + err.GetChainedMessage() + "\n"; } catch (agi::AudioOpenError const& err) { LOG_D("audio_provider") << err.GetChainedMessage(); found_audio = true; found_file = true; - msg += std::string(factory->name) + ": " + err.GetChainedMessage() + "\n"; + std::string thismsg = std::string(factory->name) + ": " + err.GetChainedMessage() + "\n"; + msg_all += thismsg; + msg_partial += thismsg; } } if (!provider) { if (found_audio) - throw agi::AudioProviderOpenError(msg, nullptr); + throw agi::AudioProviderOpenError(msg_partial, nullptr); if (found_file) - throw agi::AudioDataNotFoundError(msg, nullptr); + throw agi::AudioDataNotFoundError(msg_all, nullptr); throw agi::fs::FileNotFound(filename); } diff --git a/src/command/audio.cpp b/src/command/audio.cpp index 5943f2f76..84b346fb7 100644 --- a/src/command/audio.cpp +++ b/src/command/audio.cpp @@ -45,6 +45,7 @@ #include "../video_context.h" #include <libaegisub/make_unique.h> +#include <libaegisub/fs.h> #include <wx/msgdlg.h> @@ -70,7 +71,31 @@ struct audio_close final : public validate_audio_open { } }; -struct audio_open final : public Command { +namespace { + struct audio_open_from_file : public Command { + protected: + void do_open(agi::Context *c, agi::fs::path const& filename) { + try { + c->audioController->OpenAudio(filename); + } + catch (agi::UserCancelException const&) {} + catch (agi::fs::FileNotFound const& e) { + wxMessageBox(_("The audio file was not found: ") + to_wx(e.GetChainedMessage()), "Error loading file", wxOK | wxICON_ERROR | wxCENTER, c->parent); + } + catch (agi::AudioDataNotFoundError const& e) { + wxMessageBox(_("None of the available audio providers recognised the selected file as containing audio data.\n\nThe following providers were tried:\n") + to_wx(e.GetChainedMessage()), "Error loading file", wxOK | wxICON_ERROR | wxCENTER, c->parent); + } + catch (agi::AudioProviderOpenError const& e) { + wxMessageBox(_("None of the available audio providers have a codec available to handle the selected file.\n\nThe following providers were tried:\n") + to_wx(e.GetChainedMessage()), "Error loading file", wxOK | wxICON_ERROR | wxCENTER, c->parent); + } + catch (agi::Exception const& e) { + wxMessageBox(to_wx(e.GetChainedMessage()), "Error loading file", wxOK | wxICON_ERROR | wxCENTER, c->parent); + } + } + }; +}; + +struct audio_open final : public audio_open_from_file { CMD_NAME("audio/open") CMD_ICON(open_audio_menu) STR_MENU("&Open Audio File...") @@ -84,13 +109,7 @@ struct audio_open final : public Command { auto filename = OpenFileSelector(_("Open Audio File"), "Path/Last/Audio", "", "", str, c->parent); if (filename.empty()) return; - try { - c->audioController->OpenAudio(filename); - } - catch (agi::UserCancelException const&) { } - catch (agi::Exception const& e) { - wxMessageBox(to_wx(e.GetChainedMessage()), "Error loading file", wxOK | wxICON_ERROR | wxCENTER, c->parent); - } + do_open(c, filename); } }; @@ -126,7 +145,7 @@ struct audio_open_noise final : public Command { } }; -struct audio_open_video final : public Command { +struct audio_open_video final : public audio_open_from_file { CMD_NAME("audio/open/video") CMD_ICON(open_audio_from_video_menu) STR_MENU("Open Audio from &Video") @@ -139,13 +158,7 @@ struct audio_open_video final : public Command { } void operator()(agi::Context *c) override { - try { - c->audioController->OpenAudio(c->videoController->GetVideoName()); - } - catch (agi::UserCancelException const&) { } - catch (agi::Exception const& e) { - wxMessageBox(to_wx(e.GetChainedMessage()), "Error loading file", wxOK | wxICON_ERROR | wxCENTER, c->parent); - } + do_open(c, c->videoController->GetVideoName()); } };