From f184d8f390f084a99cd2276d9ea1c6bad5ea4848 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 15 Apr 2024 00:02:12 +0200 Subject: [PATCH] Use RAII for AVIOContext, AVFormatContext, AVCodecContext and AVFrame pointers --- apps/openmw/mwsound/ffmpeg_decoder.cpp | 200 +++++++++++-------------- apps/openmw/mwsound/ffmpeg_decoder.hpp | 35 ++++- 2 files changed, 122 insertions(+), 113 deletions(-) diff --git a/apps/openmw/mwsound/ffmpeg_decoder.cpp b/apps/openmw/mwsound/ffmpeg_decoder.cpp index a6f3d0336f3..9e7a2be3a8e 100644 --- a/apps/openmw/mwsound/ffmpeg_decoder.cpp +++ b/apps/openmw/mwsound/ffmpeg_decoder.cpp @@ -1,15 +1,41 @@ #include "ffmpeg_decoder.hpp" -#include - #include +#include #include +#include #include #include namespace MWSound { + void AVIOContextDeleter::operator()(AVIOContext* ptr) const + { + if (ptr->buffer != nullptr) + av_freep(&ptr->buffer); + +#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(57, 80, 100) + avio_context_free(&ptr); +#else + av_free(ptr); +#endif + } + + void AVFormatContextDeleter::operator()(AVFormatContext* ptr) const + { + avformat_close_input(&ptr); + } + + void AVCodecContextDeleter::operator()(AVCodecContext* ptr) const + { + avcodec_free_context(&ptr); + } + + void AVFrameDeleter::operator()(AVFrame* ptr) const + { + av_frame_free(&ptr); + } int FFmpeg_Decoder::readPacket(void* user_data, uint8_t* buf, int buf_size) { @@ -75,7 +101,7 @@ namespace MWSound return false; std::ptrdiff_t stream_idx = mStream - mFormatCtx->streams; - while (av_read_frame(mFormatCtx, &mPacket) >= 0) + while (av_read_frame(mFormatCtx.get(), &mPacket) >= 0) { /* Check if the packet belongs to this stream */ if (stream_idx == mPacket.stream_index) @@ -102,12 +128,12 @@ namespace MWSound do { /* Decode some data, and check for errors */ - int ret = avcodec_receive_frame(mCodecCtx, mFrame); + int ret = avcodec_receive_frame(mCodecCtx.get(), mFrame.get()); if (ret == AVERROR(EAGAIN)) { if (mPacket.size == 0 && !getNextPacket()) return false; - ret = avcodec_send_packet(mCodecCtx, &mPacket); + ret = avcodec_send_packet(mCodecCtx.get(), &mPacket); av_packet_unref(&mPacket); if (ret == 0) continue; @@ -187,137 +213,95 @@ namespace MWSound close(); mDataStream = mResourceMgr->get(fname); - if ((mFormatCtx = avformat_alloc_context()) == nullptr) + AVIOContextPtr ioCtx(avio_alloc_context(nullptr, 0, 0, this, readPacket, writePacket, seek)); + if (ioCtx == nullptr) + throw std::runtime_error("Failed to allocate AVIO context"); + + AVFormatContext* formatCtx = avformat_alloc_context(); + if (formatCtx == nullptr) throw std::runtime_error("Failed to allocate context"); - try + formatCtx->pb = ioCtx.get(); + + // avformat_open_input frees user supplied AVFormatContext on failure + if (avformat_open_input(&formatCtx, fname.c_str(), nullptr, nullptr) != 0) + throw std::runtime_error("Failed to open input"); + + AVFormatContextPtr formatCtxPtr(std::exchange(formatCtx, nullptr)); + + if (avformat_find_stream_info(formatCtxPtr.get(), nullptr) < 0) + throw std::runtime_error("Failed to find stream info"); + + AVStream** stream = nullptr; + for (size_t j = 0; j < formatCtxPtr->nb_streams; j++) { - mFormatCtx->pb = avio_alloc_context(nullptr, 0, 0, this, readPacket, writePacket, seek); - if (!mFormatCtx->pb || avformat_open_input(&mFormatCtx, fname.c_str(), nullptr, nullptr) != 0) + if (formatCtxPtr->streams[j]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { - // "Note that a user-supplied AVFormatContext will be freed on failure". - if (mFormatCtx) - { - if (mFormatCtx->pb != nullptr) - { - if (mFormatCtx->pb->buffer != nullptr) - { - av_free(mFormatCtx->pb->buffer); - mFormatCtx->pb->buffer = nullptr; - } - av_free(mFormatCtx->pb); - mFormatCtx->pb = nullptr; - } - avformat_free_context(mFormatCtx); - } - mFormatCtx = nullptr; - throw std::runtime_error("Failed to allocate input stream"); + stream = &formatCtxPtr->streams[j]; + break; } + } - if (avformat_find_stream_info(mFormatCtx, nullptr) < 0) - throw std::runtime_error("Failed to find stream info in " + fname); + if (stream == nullptr) + throw std::runtime_error("No audio streams"); - for (size_t j = 0; j < mFormatCtx->nb_streams; j++) - { - if (mFormatCtx->streams[j]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) - { - mStream = &mFormatCtx->streams[j]; - break; - } - } - if (!mStream) - throw std::runtime_error("No audio streams in " + fname); + const AVCodec* codec = avcodec_find_decoder((*stream)->codecpar->codec_id); + if (codec == nullptr) + throw std::runtime_error("No codec found for id " + std::to_string((*stream)->codecpar->codec_id)); - const AVCodec* codec = avcodec_find_decoder((*mStream)->codecpar->codec_id); - if (!codec) - { - std::string ss = "No codec found for id " + std::to_string((*mStream)->codecpar->codec_id); - throw std::runtime_error(ss); - } + AVCodecContext* codecCtx = avcodec_alloc_context3(codec); + if (codecCtx == nullptr) + throw std::runtime_error("Failed to allocate codec context"); - AVCodecContext* avctx = avcodec_alloc_context3(codec); - avcodec_parameters_to_context(avctx, (*mStream)->codecpar); + avcodec_parameters_to_context(codecCtx, (*stream)->codecpar); // This is not needed anymore above FFMpeg version 4.0 #if LIBAVCODEC_VERSION_INT < 3805796 - av_codec_set_pkt_timebase(avctx, (*mStream)->time_base); + av_codec_set_pkt_timebase(avctx, (*stream)->time_base); #endif - mCodecCtx = avctx; - - if (avcodec_open2(mCodecCtx, codec, nullptr) < 0) - throw std::runtime_error(std::string("Failed to open audio codec ") + codec->long_name); + AVCodecContextPtr codecCtxPtr(std::exchange(codecCtx, nullptr)); - mFrame = av_frame_alloc(); + if (avcodec_open2(codecCtxPtr.get(), codec, nullptr) < 0) + throw std::runtime_error(std::string("Failed to open audio codec ") + codec->long_name); - if (mCodecCtx->sample_fmt == AV_SAMPLE_FMT_U8P) - mOutputSampleFormat = AV_SAMPLE_FMT_U8; - // FIXME: Check for AL_EXT_FLOAT32 support - // else if (mCodecCtx->sample_fmt == AV_SAMPLE_FMT_FLT || mCodecCtx->sample_fmt == AV_SAMPLE_FMT_FLTP) - // mOutputSampleFormat = AV_SAMPLE_FMT_S16; - else - mOutputSampleFormat = AV_SAMPLE_FMT_S16; + AVFramePtr frame(av_frame_alloc()); + if (frame == nullptr) + throw std::runtime_error("Failed to allocate frame"); - mOutputChannelLayout = (*mStream)->codecpar->channel_layout; - if (mOutputChannelLayout == 0) - mOutputChannelLayout = av_get_default_channel_layout(mCodecCtx->channels); + if (codecCtxPtr->sample_fmt == AV_SAMPLE_FMT_U8P) + mOutputSampleFormat = AV_SAMPLE_FMT_U8; + // FIXME: Check for AL_EXT_FLOAT32 support + // else if (codecCtxPtr->sample_fmt == AV_SAMPLE_FMT_FLT || codecCtxPtr->sample_fmt == AV_SAMPLE_FMT_FLTP) + // mOutputSampleFormat = AV_SAMPLE_FMT_S16; + else + mOutputSampleFormat = AV_SAMPLE_FMT_S16; - mCodecCtx->channel_layout = mOutputChannelLayout; - } - catch (...) - { - if (mStream) - avcodec_free_context(&mCodecCtx); - mStream = nullptr; + mOutputChannelLayout = (*stream)->codecpar->channel_layout; + if (mOutputChannelLayout == 0) + mOutputChannelLayout = av_get_default_channel_layout(codecCtxPtr->channels); - if (mFormatCtx != nullptr) - { - if (mFormatCtx->pb->buffer != nullptr) - { - av_free(mFormatCtx->pb->buffer); - mFormatCtx->pb->buffer = nullptr; - } - av_free(mFormatCtx->pb); - mFormatCtx->pb = nullptr; + codecCtxPtr->channel_layout = mOutputChannelLayout; - avformat_close_input(&mFormatCtx); - } - } + mIoCtx = std::move(ioCtx); + mFrame = std::move(frame); + mFormatCtx = std::move(formatCtxPtr); + mCodecCtx = std::move(codecCtxPtr); + mStream = stream; } void FFmpeg_Decoder::close() { - if (mStream) - avcodec_free_context(&mCodecCtx); mStream = nullptr; + mCodecCtx.reset(); av_packet_unref(&mPacket); av_freep(&mDataBuf); - av_frame_free(&mFrame); + mFrame.reset(); swr_free(&mSwr); - if (mFormatCtx) - { - if (mFormatCtx->pb != nullptr) - { - // mFormatCtx->pb->buffer must be freed by hand, - // if not, valgrind will show memleak, see: - // - // https://trac.ffmpeg.org/ticket/1357 - // - if (mFormatCtx->pb->buffer != nullptr) - { - av_freep(&mFormatCtx->pb->buffer); - } -#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(57, 80, 100) - avio_context_free(&mFormatCtx->pb); -#else - av_freep(&mFormatCtx->pb); -#endif - } - avformat_close_input(&mFormatCtx); - } - + mFormatCtx.reset(); + mIoCtx.reset(); mDataStream.reset(); } @@ -436,10 +420,7 @@ namespace MWSound FFmpeg_Decoder::FFmpeg_Decoder(const VFS::Manager* vfs) : Sound_Decoder(vfs) - , mFormatCtx(nullptr) - , mCodecCtx(nullptr) , mStream(nullptr) - , mFrame(nullptr) , mFrameSize(0) , mFramePos(0) , mNextPts(0.0) @@ -470,5 +451,4 @@ namespace MWSound { close(); } - } diff --git a/apps/openmw/mwsound/ffmpeg_decoder.hpp b/apps/openmw/mwsound/ffmpeg_decoder.hpp index 9d15888fcfc..ed3297403ed 100644 --- a/apps/openmw/mwsound/ffmpeg_decoder.hpp +++ b/apps/openmw/mwsound/ffmpeg_decoder.hpp @@ -32,14 +32,43 @@ extern "C" namespace MWSound { + struct AVIOContextDeleter + { + void operator()(AVIOContext* ptr) const; + }; + + using AVIOContextPtr = std::unique_ptr; + + struct AVFormatContextDeleter + { + void operator()(AVFormatContext* ptr) const; + }; + + using AVFormatContextPtr = std::unique_ptr; + + struct AVCodecContextDeleter + { + void operator()(AVCodecContext* ptr) const; + }; + + using AVCodecContextPtr = std::unique_ptr; + + struct AVFrameDeleter + { + void operator()(AVFrame* ptr) const; + }; + + using AVFramePtr = std::unique_ptr; + class FFmpeg_Decoder final : public Sound_Decoder { - AVFormatContext* mFormatCtx; - AVCodecContext* mCodecCtx; + AVIOContextPtr mIoCtx; + AVFormatContextPtr mFormatCtx; + AVCodecContextPtr mCodecCtx; AVStream** mStream; AVPacket mPacket; - AVFrame* mFrame; + AVFramePtr mFrame; std::size_t mFrameSize; std::size_t mFramePos;