From 3b381617561839035657c670774af79a3751d2d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Mei=C3=9Fner?= Date: Sun, 2 Nov 2014 00:26:57 +0100 Subject: [PATCH] Address things pointed out during review --- CMakeLists.txt | 6 +-- Sources/Plasma/Apps/plClient/CMakeLists.txt | 8 +-- .../FeatureLib/pfMoviePlayer/CMakeLists.txt | 4 +- .../pfMoviePlayer/plMoviePlayer.cpp | 54 ++++++++++--------- .../FeatureLib/pfMoviePlayer/plMoviePlayer.h | 2 - 5 files changed, 37 insertions(+), 37 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 19fcc063..acac2ce5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -51,9 +51,9 @@ if(WIN32) find_package(DirectX REQUIRED) endif(WIN32) -if (VPX_FOUND AND Opus_FOUND) - set(VIDEO_AVAILABLE TRUE) - add_definitions(-DVIDEO_AVAILABLE) +if(VPX_FOUND AND Opus_FOUND) + set(MOVIE_AVAILABLE TRUE) + add_definitions(-DMOVIE_AVAILABLE) endif() include(PrecompiledHeader) #Precompiled Header helper macros diff --git a/Sources/Plasma/Apps/plClient/CMakeLists.txt b/Sources/Plasma/Apps/plClient/CMakeLists.txt index 68baa7be..ce07cbbd 100644 --- a/Sources/Plasma/Apps/plClient/CMakeLists.txt +++ b/Sources/Plasma/Apps/plClient/CMakeLists.txt @@ -12,10 +12,10 @@ include_directories(${OPENSSL_INCLUDE_DIR}) include_directories(${PYTHON_INCLUDE_DIR}) include_directories(${CURL_INCLUDE_DIR}) -if (VIDEO_AVAILABLE) +if(MOVIE_AVAILABLE) include_directories(${VPX_INCLUDE_DIR}) include_directories(${Opus_INCLUDE_DIR}) -endif (VIDEO_AVAILABLE) +endif() # Test for Python Interpreter, which will be used for extra build scripts if available find_package(PythonInterp) @@ -171,10 +171,10 @@ if(USE_VLD) target_link_libraries(plClient ${VLD_LIBRARY}) endif() -if (VIDEO_AVAILABLE) +if(MOVIE_AVAILABLE) target_link_libraries(plClient ${VPX_LIBRARY}) target_link_libraries(plClient ${Opus_LIBRARIES}) -endif (VIDEO_AVAILABLE) +endif() if (WIN32) target_link_libraries(plClient rpcrt4) diff --git a/Sources/Plasma/FeatureLib/pfMoviePlayer/CMakeLists.txt b/Sources/Plasma/FeatureLib/pfMoviePlayer/CMakeLists.txt index f50904eb..3c6961f2 100644 --- a/Sources/Plasma/FeatureLib/pfMoviePlayer/CMakeLists.txt +++ b/Sources/Plasma/FeatureLib/pfMoviePlayer/CMakeLists.txt @@ -3,10 +3,10 @@ include_directories(../../NucleusLib) include_directories(../../NucleusLib/inc) include_directories(../../PubUtilLib) -if (VIDEO_AVAILABLE) +if(MOVIE_AVAILABLE) include_directories(${VPX_INCLUDE_DIR}) include_directories(${Opus_INCLUDE_DIR}) -endif (VIDEO_AVAILABLE) +endif() set(pfMoviePlayer_SOURCES plMoviePlayer.cpp diff --git a/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.cpp b/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.cpp index 4df8450c..48b6436e 100644 --- a/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.cpp +++ b/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.cpp @@ -42,7 +42,7 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com #include "plMoviePlayer.h" -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE # define VPX_CODEC_DISABLE_COMPAT 1 # include # include @@ -61,6 +61,7 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com #include "plPipeline/hsGDeviceRef.h" #include "plPipeline/plPlates.h" #include "plResMgr/plLocalization.h" +#include "plStatusLog/plStatusLog.h" #include "plPlanarImage.h" #include "webm/mkvreader.hpp" @@ -83,11 +84,12 @@ class VPX { VPX() { } -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE public: vpx_codec_ctx_t codec; - ~VPX() { + ~VPX() + { if (vpx_codec_destroy(&codec)) hsAssert(false, vpx_codec_error_detail(&codec)); } @@ -190,7 +192,7 @@ plMoviePlayer::~plMoviePlayer() if (fPlate) // The plPlate owns the Mipmap Texture, so it destroys it for us plPlateManager::Instance().DestroyPlate(fPlate); -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE if (fReader) { fReader->Close(); @@ -201,7 +203,7 @@ plMoviePlayer::~plMoviePlayer() bool plMoviePlayer::IOpenMovie() { -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE if (!plFileInfo(fMoviePath).Exists()) { hsAssert(false, "Tried to play a movie that doesn't exist"); @@ -254,7 +256,7 @@ bool plMoviePlayer::IOpenMovie() bool plMoviePlayer::ILoadAudio() { -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE // Fetch audio track information const mkvparser::AudioTrack* audio = static_cast(fAudioTrack->GetTrack()); plWAVHeader header; @@ -269,7 +271,7 @@ bool plMoviePlayer::ILoadAudio() // Initialize Opus if (strcmp(audio->GetCodecId(), WEBM_CODECID_OPUS) != 0) { - hsAssert(false, "Not an Opus audio track!"); + plStatusLog::AddLineS("movie.log", "%s: Not an Opus audio track!", fMoviePath.AsString().c_str()); return false; } int error; @@ -284,19 +286,19 @@ bool plMoviePlayer::ILoadAudio() std::vector decoded; decoded.reserve(frames.size() * audio->GetChannels() * maxFrameSize); - for (auto it = frames.begin(); it != frames.end(); ++it) + int16_t* frameData = new int16_t[maxFrameSize * audio->GetChannels()]; + for (const auto& frame : frames) { - const std::unique_ptr& buf = std::get<0>(*it); - int32_t size = std::get<1>(*it); + const std::unique_ptr& buf = std::get<0>(frame); + int32_t size = std::get<1>(frame); - int16_t* pcm = new int16_t[maxFrameSize * audio->GetChannels()]; - int samples = opus_decode(opus, buf.get(), size, pcm, maxFrameSize, 0); + int samples = opus_decode(opus, buf.get(), size, frameData, maxFrameSize, 0); if (samples < 0) hsAssert(false, "opus error"); for (size_t i = 0; i < samples * audio->GetChannels(); i++) - decoded.push_back(pcm[i]); - delete[] pcm; + decoded.push_back(frameData[i]); } + delete[] frameData; fAudioSound->FillSoundBuffer(reinterpret_cast(decoded.data()), decoded.size() * sizeof(int16_t)); opus_decoder_destroy(opus); @@ -316,15 +318,15 @@ bool plMoviePlayer::ICheckLanguage(const mkvparser::Track* track) void plMoviePlayer::IProcessVideoFrame(const std::vector& frames) { -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE vpx_image_t* img = nullptr; // We have to decode all the frames, but we only want to display the most recent one to the user. - for (auto it = frames.begin(); it != frames.end(); ++it) + for (const auto& frame : frames) { - const std::unique_ptr& buf = std::get<0>(*it); - int32_t size = std::get<1>(*it); - img = fVpx->Decode(buf.get(), static_cast(size)); + const std::unique_ptr& buf = std::get<0>(frame); + uint32_t size = static_cast(std::get<1>(frame)); + img = fVpx->Decode(buf.get(), size); } if (img) @@ -355,7 +357,7 @@ bool plMoviePlayer::Start() if (fPlaying) return false; -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE if (!IOpenMovie()) return false; hsAssert(fVideoTrack, "nil video track -- expect bad things to happen!"); @@ -364,7 +366,7 @@ bool plMoviePlayer::Start() const mkvparser::VideoTrack* video = static_cast(fVideoTrack->GetTrack()); if (strcmp(video->GetCodecId(), WEBM_CODECID_VP9) != 0) { - hsAssert(false, "Not a VP9 video track!"); + plStatusLog::AddLineS("movie.log", "%s: Not a VP9 video track!", fMoviePath.AsString().c_str()); return false; } if (VPX* vpx = VPX::Create()) @@ -397,7 +399,7 @@ bool plMoviePlayer::Start() return true; #else return false; -#endif // VIDEO_AVAILABLE +#endif // MOVIE_AVAILABLE } bool plMoviePlayer::NextFrame() @@ -412,7 +414,7 @@ bool plMoviePlayer::NextFrame() if (fPaused) return true; -#ifdef VIDEO_AVAILABLE +#ifdef MOVIE_AVAILABLE // Get our current timecode fMovieTime += frameTimeDelta; @@ -430,7 +432,7 @@ bool plMoviePlayer::NextFrame() return true; #else return false; -#endif // VIDEO_AVAILABLE +#endif // MOVIE_AVAILABLE } bool plMoviePlayer::Pause(bool on) @@ -451,8 +453,8 @@ bool plMoviePlayer::Stop() if (fPlate) fPlate->SetVisible(false); - for (int i = 0; i < fCallbacks.size(); i++) - fCallbacks[i]->Send(); + for (auto cb : fCallbacks) + cb->Send(); fCallbacks.clear(); return true; } diff --git a/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.h b/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.h index 023dd3f1..23a5ff85 100644 --- a/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.h +++ b/Sources/Plasma/FeatureLib/pfMoviePlayer/plMoviePlayer.h @@ -66,8 +66,6 @@ typedef std::tuple, int32_t> blkbuf_t; class plMoviePlayer { protected: - friend class TrackMgr; - class plPlate* fPlate; class plMipmap* fTexture;