Skip to content

Commit

Permalink
Remove global avcodeclock singleton
Browse files Browse the repository at this point in the history 
- this was originally used to protect calls to av_register_all
- av_register_all is no longer required and we don't use it anyway
- over the years it has morphed into a catchall lock around sundry
decoder activities - some of which have been improved in recent commits
- some other FFmpeg calls (e.g. avcode_open2) used to require locking
but this is now handled internally in FFmpeg (as long as pthread support
is available - which it should always be).
- fixes PiP support
- should improve concurrency generally where more than one decoder is
active
  • Loading branch information
@mark-kendall
mark-kendall committed Mar 14, 2020
1 parent 0f1ba0e commit 913a8ff
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 119 deletions.
1 change: 0 additions & 1 deletion mythtv/libs/libmythbase/mythcorecontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ using namespace std;
#define LOC QString("MythCoreContext::%1(): ").arg(__func__)

MythCoreContext *gCoreContext = nullptr;
QMutex *avcodeclock = new QMutex(QMutex::Recursive);

class MythCoreContextPrivate : public QObject
{
Expand Down
3 changes: 0 additions & 3 deletions mythtv/libs/libmythbase/mythcorecontext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,6 @@ class MBASE_PUBLIC MythCoreContext : public QObject, public MythObservable, publ
/// This global variable contains the MythCoreContext instance for the app
extern MBASE_PUBLIC MythCoreContext *gCoreContext;

/// This global variable is used to makes certain calls to avlib threadsafe.
extern MBASE_PUBLIC QMutex *avcodeclock;

#endif

/* vim: set expandtab tabstop=4 shiftwidth=4: */
2 changes: 0 additions & 2 deletions mythtv/libs/libmythtv/Bluray/avformatdecoderbd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,6 @@ int AvFormatDecoderBD::GetAudioLanguage(uint audio_index, uint stream_index)

int AvFormatDecoderBD::ReadPacket(AVFormatContext *ctx, AVPacket* pkt, bool& /*storePacket*/)
{
QMutexLocker locker(avcodeclock);

int result = av_read_frame(ctx, pkt);

/* If we seem to have hit the end of the file, the ringbuffer may
Expand Down
2 changes: 0 additions & 2 deletions mythtv/libs/libmythtv/DVD/avformatdecoderdvd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,7 @@ int AvFormatDecoderDVD::ReadPacket(AVFormatContext *ctx, AVPacket* pkt, bool& st
m_ringBuffer->DVD()->UnblockReading();
}

avcodeclock->lock();
result = av_read_frame(ctx, pkt);
avcodeclock->unlock();

// Make sure we yield. Otherwise other threads may not
// get chance to take the lock. Shouldn't be necessary
Expand Down
41 changes: 16 additions & 25 deletions mythtv/libs/libmythtv/avformatwriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ extern "C" {

AVFormatWriter::~AVFormatWriter()
{
QMutexLocker locker(avcodeclock);

if (m_ctx)
{
(void)av_write_trailer(m_ctx);
Expand Down Expand Up @@ -240,11 +238,7 @@ int AVFormatWriter::WriteVideoFrame(VideoFrame *frame)
pkt.data = nullptr;
pkt.size = 0;
AVCodecContext *avctx = m_codecMap.getCodecContext(m_videoStream);
{
QMutexLocker locker(avcodeclock);
ret = avcodec_encode_video2(avctx, &pkt,
m_picture, &got_pkt);
}
ret = avcodec_encode_video2(avctx, &pkt, m_picture, &got_pkt);

if (ret < 0)
{
Expand Down Expand Up @@ -354,24 +348,21 @@ int AVFormatWriter::WriteAudioFrame(unsigned char *buf, int /*fnum*/, long long

m_bufferedAudioFrameTimes.push_back(timecode);

{
QMutexLocker locker(avcodeclock);
// SUGGESTION
// Now that avcodec_encode_audio2 is deprecated and replaced
// by 2 calls, this could be optimized
// into separate routines or separate threads.
got_packet = false;
ret = avcodec_receive_packet(avctx, &pkt);
if (ret == 0)
got_packet = true;
if (ret == AVERROR(EAGAIN))
ret = 0;
if (ret == 0)
ret = avcodec_send_frame(avctx, m_audPicture);
// if ret from avcodec_send_frame is AVERROR(EAGAIN) then
// there are 2 packets to be received while only 1 frame to be
// sent. The code does not cater for this. Hopefully it will not happen.
}
// SUGGESTION
// Now that avcodec_encode_audio2 is deprecated and replaced
// by 2 calls, this could be optimized
// into separate routines or separate threads.
got_packet = false;
ret = avcodec_receive_packet(avctx, &pkt);
if (ret == 0)
got_packet = true;
if (ret == AVERROR(EAGAIN))
ret = 0;
if (ret == 0)
ret = avcodec_send_frame(avctx, m_audPicture);
// if ret from avcodec_send_frame is AVERROR(EAGAIN) then
// there are 2 packets to be received while only 1 frame to be
// sent. The code does not cater for this. Hopefully it will not happen.

if (ret < 0)
{
Expand Down
37 changes: 7 additions & 30 deletions mythtv/libs/libmythtv/decoders/avformatdecoder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,6 @@ void AvFormatDecoder::CloseCodecs()
{
for (uint i = 0; i < m_ic->nb_streams; i++)
{
QMutexLocker locker(avcodeclock);
AVStream *st = m_ic->streams[i];
m_codecMap.freeCodecContext(st);
}
Expand Down Expand Up @@ -706,8 +705,6 @@ void AvFormatDecoder::SeekReset(long long newKey, uint skipFrames,

DecoderBase::SeekReset(newKey, skipFrames, doflush, discardFrames);

QMutexLocker locker(avcodeclock);

// Discard all the queued up decoded frames
if (discardFrames)
{
Expand Down Expand Up @@ -934,7 +931,6 @@ extern "C" void HandleStreamChange(void *data)

int AvFormatDecoder::FindStreamInfo(void)
{
QMutexLocker lock(avcodeclock);
int retval = avformat_find_stream_info(m_ic, nullptr);
silence_ffmpeg_logging = false;
// ffmpeg 3.0 is returning -1 code when there is a channel
Expand Down Expand Up @@ -2456,16 +2452,12 @@ int AvFormatDecoder::ScanStreams(bool novideo)
ScanATSCCaptionStreams(selTrack);
UpdateATSCCaptionTracks();

LOG(VB_GENERAL, LOG_INFO, LOC +
QString("Using %1 for video decoding").arg(GetCodecDecoderName()));
LOG(VB_GENERAL, LOG_INFO, LOC + QString("Using %1 for video decoding").arg(GetCodecDecoderName()));
m_mythCodecCtx->SetDecoderOptions(enc, codec);
if (!OpenAVCodec(enc, codec))
{
QMutexLocker locker(avcodeclock);
m_mythCodecCtx->SetDecoderOptions(enc, codec);
if (!OpenAVCodec(enc, codec))
{
scanerror = -1;
break;
}
scanerror = -1;
break;
}
break;
}
Expand Down Expand Up @@ -2522,8 +2514,6 @@ int AvFormatDecoder::ScanStreams(bool novideo)

bool AvFormatDecoder::OpenAVCodec(AVCodecContext *avctx, const AVCodec *codec)
{
QMutexLocker locker(avcodeclock);

#ifdef USING_MEDIACODEC
if (QString("mediacodec") == codec->wrapper_name)
av_jni_set_java_vm(QAndroidJniEnvironment::javaVM(), nullptr);
Expand Down Expand Up @@ -2726,7 +2716,6 @@ void AvFormatDecoder::RemoveAudioStreams()
if (!m_audio->HasAudioIn())
return;

QMutexLocker locker(avcodeclock);
for (uint i = 0; i < m_ic->nb_streams;)
{
AVStream *st = m_ic->streams[i];
Expand Down Expand Up @@ -3415,7 +3404,6 @@ bool AvFormatDecoder::ProcessVideoPacket(AVStream *curstream, AVPacket *pkt, boo

bool sentPacket = false;
int ret2 = 0;
avcodeclock->lock();
if (m_privateDec)
{
if (QString(m_ic->iformat->name).contains("avi") || !m_ptsDetected)
Expand Down Expand Up @@ -3464,7 +3452,6 @@ bool AvFormatDecoder::ProcessVideoPacket(AVStream *curstream, AVPacket *pkt, boo
}
}
}
avcodeclock->unlock();

if (ret < 0 || ret2 < 0)
{
Expand Down Expand Up @@ -3958,7 +3945,8 @@ void AvFormatDecoder::ProcessDSMCCPacket(const AVStream *str, const AVPacket *pk
int dataBroadcastId = 0;
unsigned carouselId = 0;
{
QMutexLocker locker(avcodeclock);
// TODO does this still need locking?
//QMutexLocker locker(avcodeclock);
componentTag = str->component_tag;
dataBroadcastId = str->data_id;
carouselId = (unsigned) str->carousel_id;
Expand Down Expand Up @@ -4012,7 +4000,6 @@ bool AvFormatDecoder::ProcessSubtitlePacket(AVStream *curstream, AVPacket *pkt)
{
if (pkt->stream_index == subIdx)
{
QMutexLocker locker(avcodeclock);
m_ringBuffer->DVD()->DecodeSubtitles(&subtitle, &gotSubtitles,
pkt->data, pkt->size, pts);
}
Expand All @@ -4021,7 +4008,6 @@ bool AvFormatDecoder::ProcessSubtitlePacket(AVStream *curstream, AVPacket *pkt)
else if (m_decodeAllSubtitles || pkt->stream_index == subIdx)
{
AVCodecContext *ctx = m_codecMap.getCodecContext(curstream);
QMutexLocker locker(avcodeclock);
avcodec_decode_subtitle2(ctx, &subtitle, &gotSubtitles,
pkt);

Expand Down Expand Up @@ -4646,8 +4632,6 @@ bool AvFormatDecoder::ProcessAudioPacket(AVStream *curstream, AVPacket *pkt,
bool already_decoded = false;
if (!ctx->channels)
{
QMutexLocker locker(avcodeclock);

if (DoPassThrough(curstream->codecpar, false) || !DecoderWillDownmix(ctx))
{
// for passthru or codecs for which the decoder won't downmix
Expand Down Expand Up @@ -4707,8 +4691,6 @@ bool AvFormatDecoder::ProcessAudioPacket(AVStream *curstream, AVPacket *pkt,
}
}
m_firstVPtsInuse = false;

avcodeclock->lock();
data_size = 0;

// Check if the number of channels or sampling rate have changed
Expand Down Expand Up @@ -4767,7 +4749,6 @@ bool AvFormatDecoder::ProcessAudioPacket(AVStream *curstream, AVPacket *pkt,
decoded_size = data_size;
}
}
avcodeclock->unlock();

if (ret < 0)
{
Expand Down Expand Up @@ -5138,17 +5119,13 @@ void AvFormatDecoder::StreamChangeCheck(void)
if (m_streamsChanged)
{
SeekReset(0, 0, true, true);
avcodeclock->lock();
ScanStreams(false);
avcodeclock->unlock();
m_streamsChanged = false;
}
}

int AvFormatDecoder::ReadPacket(AVFormatContext *ctx, AVPacket *pkt, bool &/*storePacket*/)
{
QMutexLocker locker(avcodeclock);

return av_read_frame(ctx, pkt);
}

Expand Down
73 changes: 29 additions & 44 deletions mythtv/libs/libmythtv/decoders/nuppeldecoder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,6 @@ bool NuppelDecoder::InitAVCodecVideo(int codec)
m_mpaVidCtx->extradata_size = m_ffmpegExtraDataSize;
}

QMutexLocker locker(avcodeclock);
if (avcodec_open2(m_mpaVidCtx, m_mpaVidCodec, nullptr) < 0)
{
LOG(VB_GENERAL, LOG_ERR, LOC + "Couldn't find lavc video codec");
Expand All @@ -715,8 +714,6 @@ bool NuppelDecoder::InitAVCodecVideo(int codec)

void NuppelDecoder::CloseAVCodecVideo(void)
{
QMutexLocker locker(avcodeclock);

if (m_mpaVidCodec && m_mpaVidCtx)
avcodec_free_context(&m_mpaVidCtx);
}
Expand Down Expand Up @@ -759,7 +756,6 @@ bool NuppelDecoder::InitAVCodecAudio(int codec)
m_mpaAudCtx->codec_id = (enum AVCodecID)codec;
m_mpaAudCtx->codec_type = AVMEDIA_TYPE_AUDIO;

QMutexLocker locker(avcodeclock);
if (avcodec_open2(m_mpaAudCtx, m_mpaAudCodec, nullptr) < 0)
{
LOG(VB_GENERAL, LOG_ERR, LOC + "Couldn't find lavc audio codec");
Expand All @@ -771,8 +767,6 @@ bool NuppelDecoder::InitAVCodecAudio(int codec)

void NuppelDecoder::CloseAVCodecAudio(void)
{
QMutexLocker locker(avcodeclock);

if (m_mpaAudCodec && m_mpaAudCtx)
avcodec_free_context(&m_mpaAudCtx);
}
Expand Down Expand Up @@ -882,42 +876,37 @@ bool NuppelDecoder::DecodeFrame(struct rtframeheader *frameheader,
pkt.data = lstrm;
pkt.size = frameheader->packetlength;

// if directrendering, writes into buf
bool gotpicture = false;
// SUGGESTION
// Now that avcodec_decode_video2 is deprecated and replaced
// by 2 calls (receive frame and send packet), this could be optimized
// into separate routines or separate threads.
// Also now that it always consumes a whole buffer some code
// in the caller may be able to be optimized.
int ret = avcodec_receive_frame(m_mpaVidCtx, mpa_pic);
if (ret == 0)
gotpicture = true;
if (ret == AVERROR(EAGAIN))
ret = 0;
if (ret == 0)
ret = avcodec_send_packet(m_mpaVidCtx, &pkt);
m_directFrame = nullptr;
// The code assumes that there is always space to add a new
// packet. This seems risky but has always worked.
// It should actually check if (ret == AVERROR(EAGAIN)) and then keep
// the packet around and try it again after processing the frame
// received here.
if (ret < 0)
{
QMutexLocker locker(avcodeclock);
// if directrendering, writes into buf
bool gotpicture = false;
// SUGGESTION
// Now that avcodec_decode_video2 is deprecated and replaced
// by 2 calls (receive frame and send packet), this could be optimized
// into separate routines or separate threads.
// Also now that it always consumes a whole buffer some code
// in the caller may be able to be optimized.
int ret = avcodec_receive_frame(m_mpaVidCtx, mpa_pic);
if (ret == 0)
gotpicture = true;
if (ret == AVERROR(EAGAIN))
ret = 0;
if (ret == 0)
ret = avcodec_send_packet(m_mpaVidCtx, &pkt);
m_directFrame = nullptr;
// The code assumes that there is always space to add a new
// packet. This seems risky but has always worked.
// It should actually check if (ret == AVERROR(EAGAIN)) and then keep
// the packet around and try it again after processing the frame
// received here.
if (ret < 0)
{
char error[AV_ERROR_MAX_STRING_SIZE];
LOG(VB_GENERAL, LOG_ERR, LOC +
QString("video decode error: %1 (%2)")
.arg(av_make_error_string(error, sizeof(error), ret))
.arg(gotpicture));
}
if (!gotpicture)
{
return false;
}
char error[AV_ERROR_MAX_STRING_SIZE];
LOG(VB_GENERAL, LOG_ERR, LOC +
QString("video decode error: %1 (%2)")
.arg(av_make_error_string(error, sizeof(error), ret))
.arg(gotpicture));
}
if (!gotpicture)
return false;

/* XXX: Broken
if (mpa_pic->qscale_table != nullptr && mpa_pic->qstride > 0)
Expand Down Expand Up @@ -1242,8 +1231,6 @@ bool NuppelDecoder::GetFrame(DecodeType decodetype, bool& /*Retry*/)
pkt.data = m_strm;
pkt.size = m_frameHeader.packetlength;

QMutexLocker locker(avcodeclock);

while (pkt.size > 0 && m_audio->HasAudioOut())
{
int data_size = 0;
Expand Down Expand Up @@ -1356,8 +1343,6 @@ void NuppelDecoder::SeekReset(long long newKey, uint skipFrames,
.arg((doFlush) ? "do" : "don't")
.arg((discardFrames) ? "do" : "don't"));

QMutexLocker locker(avcodeclock);

DecoderBase::SeekReset(newKey, skipFrames, doFlush, discardFrames);

if (m_mpaVidCodec && doFlush)
Expand Down
Loading

0 comments on commit 913a8ff

Please sign in to comment.