1
0
mirror of https://github.com/cookiengineer/audacity synced 2025-07-05 23:19:06 +02:00

Unreported bug: memory and file handle leak when importing custom mpeg...

I could see this on windows:

Import a file such as .aiff format, while "Files of type:"
reads "FFmpeg-compatible files".
Leave Audacity running.
Attempt to delete the file in Windows Exploerer.
Get a message from Windows that the file is in use by Audacity and cannot be
deleted.
This commit is contained in:
Paul Licameli 2016-04-09 00:18:20 -04:00
parent 0ebc23e3a9
commit 7c0073dd77
5 changed files with 77 additions and 63 deletions

View File

@ -209,12 +209,10 @@ static int64_t ufile_seek(void *opaque, int64_t pos, int whence)
int ufile_close(AVIOContext *pb)
{
wxFile *f = (wxFile *) pb->opaque;
std::unique_ptr<wxFile> f{ (wxFile *)pb->opaque };
if (f) {
if (f)
f->Close();
delete f;
}
return 0;
}
@ -222,16 +220,14 @@ int ufile_close(AVIOContext *pb)
// Open a file with a (possibly) Unicode filename
int ufile_fopen(AVIOContext **s, const wxString & name, int flags)
{
wxFile *f;
wxFile::OpenMode mode;
f = new wxFile;
auto f = std::make_unique<wxFile>();
if (!f) {
return -ENOMEM;
}
if (flags == (AVIO_FLAG_READ | AVIO_FLAG_WRITE)) {
delete f;
return -EINVAL;
} else if (flags == AVIO_FLAG_WRITE) {
mode = wxFile::write;
@ -240,63 +236,76 @@ int ufile_fopen(AVIOContext **s, const wxString & name, int flags)
}
if (!f->Open(name, mode)) {
delete f;
return -ENOENT;
}
*s = avio_alloc_context((unsigned char*)av_malloc(32768), 32768,
flags & AVIO_FLAG_WRITE,
/*opaque*/f,
/*opaque*/f.get(),
ufile_read,
ufile_write,
ufile_seek);
if (!*s) {
delete f;
return -ENOMEM;
}
f.release(); // s owns the file object now
return 0;
}
// Detect type of input file and open it if recognized. Routine
// based on the av_open_input_file() libavformat function.
int ufile_fopen_input(AVFormatContext **ic_ptr, wxString & name)
int ufile_fopen_input(std::unique_ptr<FFmpegContext> &context_ptr, wxString & name)
{
context_ptr.reset();
auto context = std::make_unique<FFmpegContext>();
wxFileName f(name);
wxCharBuffer fname;
const char *filename;
AVIOContext *pb = NULL;
int err;
fname = f.GetFullName().mb_str();
filename = (const char *) fname;
// Open the file to prepare for probing
if ((err = ufile_fopen(&pb, name, AVIO_FLAG_READ)) < 0) {
if ((err = ufile_fopen(&context->pb, name, AVIO_FLAG_READ)) < 0) {
goto fail;
}
*ic_ptr = avformat_alloc_context();
(*ic_ptr)->pb = pb;
context->ic_ptr = avformat_alloc_context();
context->ic_ptr->pb = context->pb;
// And finally, attempt to associate an input stream with the file
err = avformat_open_input(ic_ptr, filename, NULL, NULL);
err = avformat_open_input(&context->ic_ptr, filename, NULL, NULL);
if (err) {
goto fail;
}
// success
context_ptr = std::move(context);
return 0;
fail:
if (pb) {
ufile_close(pb);
return err;
}
FFmpegContext::~FFmpegContext()
{
if (FFmpegLibsInst->ValidLibsLoaded())
{
if (ic_ptr)
avformat_close_input(&ic_ptr);
av_log_set_callback(av_log_default_callback);
}
*ic_ptr = NULL;
return err;
if (pb) {
ufile_close(pb);
av_free(pb);
}
}
streamContext *import_ffmpeg_read_next_frame(AVFormatContext* formatContext,

View File

@ -393,8 +393,18 @@ FFmpegLibs *PickFFmpegLibs();
///! anymore, or just decrements it's reference count
void DropFFmpegLibs();
// This object allows access to the AVFormatContext,
// and its destructor cleans up memory and file handles
struct FFmpegContext {
FFmpegContext() {}
~FFmpegContext();
AVIOContext *pb{};
AVFormatContext *ic_ptr{};
};
int ufile_fopen(AVIOContext **s, const wxString & name, int flags);
int ufile_fopen_input(AVFormatContext **ic_ptr, wxString & name);
int ufile_fopen_input(std::unique_ptr<FFmpegContext> &context_ptr, wxString & name);
int ufile_close(AVIOContext *pb);
struct streamContext;

View File

@ -262,6 +262,7 @@ public:
private:
std::shared_ptr<FFmpegContext> mContext; // An object that does proper IO shutdown in its destructor; may be shared with decoder task.
AVFormatContext *mFormatContext; //!< Format description, also contains metadata and some useful info
int mNumStreams; //!< mNumstreams is less or equal to mFormatContext->nb_streams
ScsPtr mScs; //!< Points to array of pointers to stream contexts, which may be shared with a decoder task.
@ -358,12 +359,20 @@ bool FFmpegImportFileHandle::Init()
av_log_set_callback(av_log_wx_callback);
int err = ufile_fopen_input(&mFormatContext, mName);
if (err < 0)
int err;
{
wxLogError(wxT("FFmpeg : av_open_input_file() failed for file %s"),mName.c_str());
return false;
std::unique_ptr<FFmpegContext> tempContext;
err = ufile_fopen_input(tempContext, mName);
if (err < 0)
{
wxLogError(wxT("FFmpeg : av_open_input_file() failed for file %s"), mName.c_str());
return false;
}
wxASSERT(tempContext.get());
// Move from unique to shared pointer
mContext.reset(tempContext.release());
}
mFormatContext = mContext->ic_ptr;
err = avformat_find_stream_info(mFormatContext, NULL);
if (err < 0)
@ -573,7 +582,7 @@ int FFmpegImportFileHandle::Import(TrackFactory *trackFactory,
for (const auto &stream : mChannels) {
++s;
ODDecodeFFmpegTask* odTask =
new ODDecodeFFmpegTask(mScs, ODDecodeFFmpegTask::FromList(mChannels), mFormatContext, s);
new ODDecodeFFmpegTask(mScs, ODDecodeFFmpegTask::FromList(mChannels), mContext, s);
odTask->CreateFileDecoder(mFilename);
//each stream has different duration. We need to know it if seeking is to be allowed.
@ -852,21 +861,8 @@ void FFmpegImportFileHandle::GetMetadata(Tags *tags, const wxChar *tag, const ch
FFmpegImportFileHandle::~FFmpegImportFileHandle()
{
#ifdef EXPERIMENTAL_OD_FFMPEG
//ODDecodeFFmpegTask takes ownership and deltes it there.
if(!mUsingOD)
{
#endif
if (FFmpegLibsInst->ValidLibsLoaded())
{
if (mFormatContext) avformat_close_input(&mFormatContext);
av_log_set_callback(av_log_default_callback);
}
#ifdef EXPERIMENTAL_OD_FFMPEG
}//mUsingOD
#endif
// Do this before unloading the libraries
mContext.reset();
delete mStreamInfo;

View File

@ -62,7 +62,7 @@ public:
ODFFmpegDecoder(const wxString & fileName,
const ScsPtr &scs,
ODDecodeFFmpegTask::Streams &&channels,
AVFormatContext* formatContext,
const std::shared_ptr<FFmpegContext> &formatContext,
int streamIndex);
virtual ~ODFFmpegDecoder();
@ -99,7 +99,7 @@ private:
ScsPtr mScs; //!< Pointer to array of pointers to stream contexts.
ODDecodeFFmpegTask::Streams mChannels;
AVFormatContext *mFormatContext; //!< Format description, also contains metadata and some useful info
std::shared_ptr<FFmpegContext> mContext; //!< Format description, also contains metadata and some useful info
std::vector<FFMpegDecodeCache*> mDecodeCache;
int mNumSamplesInCache;
sampleCount mCurrentPos; //the index of the next sample to be decoded
@ -128,11 +128,11 @@ auto ODDecodeFFmpegTask::FromList(const std::list<TrackHolders> &channels) -> St
}
//------ ODDecodeFFmpegTask definitions
ODDecodeFFmpegTask::ODDecodeFFmpegTask(const ScsPtr &scs, Streams &&channels, void* formatContext, int streamIndex)
ODDecodeFFmpegTask::ODDecodeFFmpegTask(const ScsPtr &scs, Streams &&channels, const std::shared_ptr<FFmpegContext> &context, int streamIndex)
: mChannels(std::move(channels))
{
mScs=scs;
mFormatContext = formatContext;
mContext = context;
mStreamIndex = streamIndex;
}
ODDecodeFFmpegTask::~ODDecodeFFmpegTask()
@ -142,7 +142,7 @@ ODDecodeFFmpegTask::~ODDecodeFFmpegTask()
std::unique_ptr<ODTask> ODDecodeFFmpegTask::Clone() const
{
auto clone = std::make_unique<ODDecodeFFmpegTask>(mScs, Streams{ mChannels }, mFormatContext, mStreamIndex);
auto clone = std::make_unique<ODDecodeFFmpegTask>(mScs, Streams{ mChannels }, mContext, mStreamIndex);
clone->mDemandSample=GetDemandSample();
//the decoders and blockfiles should not be copied. They are created as the task runs.
@ -157,7 +157,7 @@ ODFileDecoder* ODDecodeFFmpegTask::CreateFileDecoder(const wxString & fileName)
// Open the file for import
auto decoder =
make_movable<ODFFmpegDecoder>(fileName, mScs, ODDecodeFFmpegTask::Streams{ mChannels },
(AVFormatContext*)mFormatContext, mStreamIndex);
mContext, mStreamIndex);
mDecoders.push_back(std::move(decoder));
return mDecoders.back().get();
@ -206,13 +206,15 @@ bool ODFFmpegDecoder::SeekingAllowed()
// url_fseek(mFormatContext->pb,0,SEEK_SET);
std::unique_ptr<FFmpegContext> tempMpegContext;
AVFormatContext* tempContext;
int err;
err = ufile_fopen_input(&tempContext, mFName);
err = ufile_fopen_input(tempMpegContext, mFName);
if (err < 0)
{
goto test_failed;
}
tempContext = tempMpegContext->ic_ptr;
err = avformat_find_stream_info(tempContext, NULL);
if (err < 0)
@ -222,12 +224,9 @@ bool ODFFmpegDecoder::SeekingAllowed()
if(av_seek_frame(tempContext, st->index, 0, 0) >= 0) {
mSeekingAllowedStatus = ODFFMPEG_SEEKING_TEST_SUCCESS;
if (tempContext) avformat_close_input(&tempContext);
return SeekingAllowed();
}
if (tempContext) avformat_close_input(&tempContext);
test_failed:
mSeekingAllowedStatus = ODFFMPEG_SEEKING_TEST_FAILED;
return SeekingAllowed();
@ -239,12 +238,12 @@ test_failed:
ODFFmpegDecoder::ODFFmpegDecoder(const wxString & fileName,
const ScsPtr &scs,
ODDecodeFFmpegTask::Streams &&channels,
AVFormatContext* formatContext,
int streamIndex)
const std::shared_ptr<FFmpegContext> &context,
int streamIndex)
:ODFileDecoder(fileName),
//mSamplesDone(0),
mScs(scs),
mFormatContext(formatContext),
mContext(context),
mNumSamplesInCache(0),
mCurrentLen(0),
mSeekingAllowedStatus(ODFFMPEG_SEEKING_TEST_UNKNOWN),
@ -271,11 +270,8 @@ mStreamIndex(streamIndex)
//we have taken ownership, so DELETE the ffmpeg stuff allocated in ImportFFmpeg that was given to us.
ODFFmpegDecoder::~ODFFmpegDecoder()
{
if (FFmpegLibsInst->ValidLibsLoaded())
{
if (mFormatContext) avformat_close_input(&mFormatContext);
av_log_set_callback(av_log_default_callback);
}
// Do this before unloading libraries
mContext.reset();
//DELETE our caches.
while(mDecodeCache.size())
@ -295,6 +291,8 @@ ODFFmpegDecoder::~ODFFmpegDecoder()
#define kMaxSeekRewindAttempts 8
int ODFFmpegDecoder::Decode(SampleBuffer & data, sampleFormat & format, sampleCount start, sampleCount len, unsigned int channel)
{
auto mFormatContext = mContext->ic_ptr;
auto scs = mScs->get();
auto sci = scs[mStreamIndex].get();
format = sci->m_osamplefmt;
@ -592,7 +590,7 @@ streamContext* ODFFmpegDecoder::ReadNextFrame()
auto scs = mScs->get();
// This reinterpret_cast to array of plain pointers is innocent
return import_ffmpeg_read_next_frame
(mFormatContext, reinterpret_cast<streamContext**>(scs), mChannels.size());
(mContext->ic_ptr, reinterpret_cast<streamContext**>(scs), mChannels.size());
}

View File

@ -21,6 +21,7 @@
#include "ODDecodeTask.h"
#include "ODTaskThread.h"
struct FFmpegContext;
class ODFileDecoder;
class WaveTrack;
/// A class representing a modular task to be used with the On-Demand structures.
@ -33,7 +34,7 @@ public:
static Streams FromList(const std::list<TrackHolders> &channels);
/// Constructs an ODTask
ODDecodeFFmpegTask(const ScsPtr &scs, Streams &&channels, void* formatContext, int streamIndex);
ODDecodeFFmpegTask(const ScsPtr &scs, Streams &&channels, const std::shared_ptr<FFmpegContext> &context, int streamIndex);
virtual ~ODDecodeFFmpegTask();
std::unique_ptr<ODTask> Clone() const override;
@ -49,7 +50,7 @@ protected:
Streams mChannels;
ScsPtr mScs;
void* mFormatContext;
std::shared_ptr<FFmpegContext> mContext;
int mStreamIndex;
};
#endif //__ODDECODEFFMPEGTASK__