Skip to content

Commit

Permalink
Fix a crash caused by background animated image loading.
Browse files Browse the repository at this point in the history 
The background animated image loader was calling SetImages() which
triggers non thread-safe routines in MythUI.  Instead of calling
SetImages() in the background thread, return the image QVector
in the ImageLoadEvent and set the images and delays in the UI
thread similar to how we set non-animated images.

Fixes #9388.

NOTE: This changes the binary API, so make clean, etc., whatever
you normally do.
  • Loading branch information
@cpinkham
cpinkham committed Oct 18, 2011
1 parent 607206b commit 02f8352
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 33 deletions.
2 changes: 1 addition & 1 deletion mythtv/libs/libmythbase/mythversion.h
View file
Expand Up @@ -12,7 +12,7 @@
/// Update this whenever the plug-in API changes. /// Update this whenever the plug-in API changes.
/// Including changes in the libmythbase, libmyth, libmythtv, libmythav* and /// Including changes in the libmythbase, libmyth, libmythtv, libmythav* and
/// libmythui class methods used by plug-ins. /// libmythui class methods used by plug-ins.
#define MYTH_BINARY_VERSION "0.25.20111018-2" #define MYTH_BINARY_VERSION "0.25.20111018-3"


/** \brief Increment this whenever the MythTV network protocol changes. /** \brief Increment this whenever the MythTV network protocol changes.
* *
Expand Down
6 changes: 6 additions & 0 deletions mythtv/libs/libmythui/mythimage.cpp
View file
Expand Up @@ -432,6 +432,12 @@ MythImageReader::MythImageReader(const QString &fileName)
} }
else if (!m_fileName.isEmpty()) else if (!m_fileName.isEmpty())
{ {
if (!m_fileName.startsWith("/") && !QFile::exists(m_fileName))
{
QString tmpFile = GetMythUI()->GetThemeDir() + '/' + m_fileName;
if (QFile::exists(tmpFile))
m_fileName = tmpFile;
}
setFileName(m_fileName); setFileName(m_fileName);
} }
} }
Expand Down
143 changes: 113 additions & 30 deletions mythtv/libs/libmythui/mythuiimage.cpp
View file
Expand Up @@ -42,7 +42,16 @@ class ImageLoadEvent : public QEvent
int number) int number)
: QEvent(kEventType), : QEvent(kEventType),
m_parent(parent), m_image(image), m_basefile(basefile), m_parent(parent), m_image(image), m_basefile(basefile),
m_filename(filename), m_number(number) { } m_filename(filename), m_number(number),
m_images(NULL), m_delays(NULL) { }

ImageLoadEvent(MythUIImage *parent, QVector<MythImage *> *images,
QVector<int> *delays, const QString &basefile,
const QString &filename)
: QEvent(kEventType),
m_parent(parent), m_image(NULL), m_basefile(basefile),
m_filename(filename), m_number(0),
m_images(images), m_delays(delays) { }


MythUIImage *GetParent() const MythUIImage *GetParent() const
{ {
Expand All @@ -64,6 +73,14 @@ class ImageLoadEvent : public QEvent
{ {
return m_number; return m_number;
} }
QVector<MythImage *> *GetImages() const
{
return m_images;
}
QVector<int> *GetDelays() const
{
return m_delays;
}


static Type kEventType; static Type kEventType;


Expand All @@ -73,6 +90,10 @@ class ImageLoadEvent : public QEvent
QString m_basefile; QString m_basefile;
QString m_filename; QString m_filename;
int m_number; int m_number;

// Animated Images
QVector<MythImage *> *m_images;
QVector<int> *m_delays;
}; };


QEvent::Type ImageLoadEvent::kEventType = QEvent::Type ImageLoadEvent::kEventType =
Expand Down Expand Up @@ -107,8 +128,16 @@ class ImageLoadThread : public QRunnable


if (imageReader.supportsAnimation()) if (imageReader.supportsAnimation())
{ {
QVector<MythImage *> *images = new QVector<MythImage *>;
QVector<int> *delays = new QVector<int>;

m_parent->LoadAnimatedImage( m_parent->LoadAnimatedImage(
imageReader, m_filename, m_ForceSize, m_cacheMode); imageReader, m_filename, m_ForceSize, m_cacheMode,
images, delays);

ImageLoadEvent *le = new ImageLoadEvent(m_parent, images, delays,
m_basefile, m_filename);
QCoreApplication::postEvent(m_parent, le);
} }
else else
{ {
Expand Down Expand Up @@ -412,7 +441,7 @@ void MythUIImage::SetImage(MythImage *img)
* Use is strongly discouraged, use SetFilepattern() instead. * Use is strongly discouraged, use SetFilepattern() instead.
* *
*/ */
void MythUIImage::SetImages(QVector<MythImage *> &images) void MythUIImage::SetImages(QVector<MythImage *> *images)
{ {
Clear(); Clear();


Expand All @@ -421,7 +450,7 @@ void MythUIImage::SetImages(QVector<MythImage *> &images)


QVector<MythImage *>::iterator it; QVector<MythImage *>::iterator it;


for (it = images.begin(); it != images.end(); ++it) for (it = images->begin(); it != images->end(); ++it)
{ {
MythImage *im = (*it); MythImage *im = (*it);


Expand Down Expand Up @@ -462,6 +491,7 @@ void MythUIImage::SetImages(QVector<MythImage *> &images)
SetSize(aSize); SetSize(aSize);


m_CurPos = 0; m_CurPos = 0;
m_animatedImage = true;
SetRedraw(); SetRedraw();
} }


Expand Down Expand Up @@ -873,7 +903,8 @@ MythImage *MythUIImage::LoadImage(
*/ */
bool MythUIImage::LoadAnimatedImage( bool MythUIImage::LoadAnimatedImage(
MythImageReader &imageReader, const QString &imFile, MythImageReader &imageReader, const QString &imFile,
QSize bForceSize, int cacheMode) QSize bForceSize, int cacheMode, QVector<MythImage *> *images,
QVector<int> *delays)
{ {
bool result = false; bool result = false;
m_loadingImagesLock.lock(); m_loadingImagesLock.lock();
Expand All @@ -899,14 +930,25 @@ bool MythUIImage::LoadAnimatedImage(


QString filename = QString("frame-%1-") + imFile; QString filename = QString("frame-%1-") + imFile;
QString frameFilename; QString frameFilename;
QVector<MythImage *> images; QVector<MythImage *> *myImages = NULL;
QVector<int> delays; QVector<int> *myDelays = NULL;
int imageCount = 1; int imageCount = 1;
QString imageLabel; QString imageLabel;


int w = -1; int w = -1;
int h = -1; int h = -1;


if (images)
{
myImages = images;
myDelays = delays;
}
else
{
myImages = new QVector<MythImage *>;
myDelays = new QVector<int>;
}

if (!bForceSize.isNull()) if (!bForceSize.isNull())
{ {
if (bForceSize.width() != -1) if (bForceSize.width() != -1)
Expand All @@ -927,21 +969,27 @@ bool MythUIImage::LoadAnimatedImage(
if (!im) if (!im)
break; break;


images.append(im); myImages->append(im);
delays.append(imageReader.nextImageDelay()); myDelays->append(imageReader.nextImageDelay());
imageCount++; imageCount++;
} }


if (images.size()) if (myImages->size())
{ {
m_animatedImage = true; // Only SetImages() if not returning them via the passed-in QVector
SetImages(images); if (!images)

if ((m_Delay == -1) &&
(imageReader.supportsAnimation()) &&
(delays.size()))
{ {
SetDelays(delays); SetImages(myImages);

if ((m_Delay == -1) &&
(imageReader.supportsAnimation()) &&
(myDelays->size()))
{
SetDelays(*myDelays);
}

delete myImages;
delete myDelays;
} }


result = true; result = true;
Expand Down Expand Up @@ -1316,7 +1364,7 @@ void MythUIImage::CopyFrom(MythUIType *base)
m_animationCycle = im->m_animationCycle; m_animationCycle = im->m_animationCycle;
m_animatedImage = im->m_animatedImage; m_animatedImage = im->m_animatedImage;


//SetImages(im->m_Images); //SetImages(&im->m_Images);


MythUIType::CopyFrom(base); MythUIType::CopyFrom(base);


Expand Down Expand Up @@ -1389,37 +1437,72 @@ void MythUIImage::LoadNow(void)
*/ */
void MythUIImage::customEvent(QEvent *event) void MythUIImage::customEvent(QEvent *event)
{ {
MythImage *image = NULL;
QVector<MythImage *> *images = NULL;
QVector<int> *delays = NULL;
int number = 0;

if (event->type() == ImageLoadEvent::kEventType) if (event->type() == ImageLoadEvent::kEventType)
{ {
ImageLoadEvent *le = dynamic_cast<ImageLoadEvent *>(event); ImageLoadEvent *le = dynamic_cast<ImageLoadEvent *>(event);


if (le->GetParent() != this) if (le->GetParent() != this)
return; return;


MythImage *image = le->GetImage(); image = le->GetImage();

number = le->GetNumber();
if (!image) images = le->GetImages();
return; delays = le->GetDelays();


d->m_UpdateLock.lockForRead(); d->m_UpdateLock.lockForRead();


if (le->GetBasefile() != m_Filename) if (le->GetBasefile() != m_Filename)
{ {
d->m_UpdateLock.unlock(); d->m_UpdateLock.unlock();
#if 0
LOG(VB_GUI | VB_FILE, LOG_DEBUG, LOC + if (image)
QString("customEvent(): Expecting '%2', got '%3'") image->DownRef();
.arg(m_Filename).arg(le->GetBasefile()));
#endif if (images)
image->DownRef(); {
QVector<MythImage *>::iterator it;

for (it = images->begin(); it != images->end(); ++it)
{
MythImage *im = (*it);
im->DownRef();
}

delete images;
delete delays;
}

return; return;
} }


d->m_UpdateLock.unlock(); d->m_UpdateLock.unlock();
}

if (images)
{
if (images->size())
{
SetImages(images);

if (delays && delays->size())
SetDelays(*delays);
}


QString filename = le->GetFilename(); delete images;
int number = le->GetNumber();


if (delays)
delete delays;

return;
}

if (image)
{
d->m_UpdateLock.lockForWrite(); d->m_UpdateLock.lockForWrite();


if (m_ForceSize.isNull()) if (m_ForceSize.isNull())
Expand Down
6 changes: 4 additions & 2 deletions mythtv/libs/libmythui/mythuiimage.h
View file
Expand Up @@ -47,7 +47,7 @@ class MUI_PUBLIC MythUIImage : public MythUIType
/** Should not be used unless absolutely necessary since it bypasses the /** Should not be used unless absolutely necessary since it bypasses the
* image caching and threaded loaded mechanisms. * image caching and threaded loaded mechanisms.
*/ */
void SetImages(QVector<MythImage *> &images); void SetImages(QVector<MythImage *> *images);


void SetDelay(int delayms); void SetDelay(int delayms);
void SetDelays(QVector<int> delays); void SetDelays(QVector<int> delays);
Expand All @@ -68,7 +68,9 @@ class MUI_PUBLIC MythUIImage : public MythUIType
MythImage* LoadImage(MythImageReader &imageReader, const QString &imFile, MythImage* LoadImage(MythImageReader &imageReader, const QString &imFile,
QSize bForceSize, int cacheMode); QSize bForceSize, int cacheMode);
bool LoadAnimatedImage(MythImageReader &imageReader, const QString &imFile, bool LoadAnimatedImage(MythImageReader &imageReader, const QString &imFile,
QSize bForceSize, int mode); QSize bForceSize, int mode,
QVector<MythImage *> *images = NULL,
QVector<int> *delays = NULL);
void customEvent(QEvent *event); void customEvent(QEvent *event);


virtual bool ParseElement( virtual bool ParseElement(
Expand Down

0 comments on commit 02f8352

Please sign in to comment.