Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#12902 closed Bug Report - Crash (fixed)

Cannot make QOpenGLContext current in a different thread (occurred while watching live tv)

Reported by: William L. DeRieux IV <WilliamDeRieux@…> Owned by: Peter Bennett
Priority: minor Milestone: 30.0
Component: MythTV - Video Playback Version: 0.28.0
Severity: medium Keywords:
Cc: Ticket locked: no

Description

I got this error when while I was watching live tv.

Cannot make QOpenGLContext current in a different thread

I found some documentation for QOpenGLContext from here: http://doc.qt.io/qt-5/qopenglcontext.html#thread-affinity

Some info about my system

          Distribution: Debian 
               Version:  (debian version: stretch/sid)
     Support Lifecycle: N\A
              Codename:  
              Fullname: Debian GNU/Linux stretch/sid 

           Kernel Name: Linux
        Kernel Release: 3.16.0-4-amd64
        Kernel Version: #1 SMP Debian 3.16.36-1+deb8u1 (2016-09-03)
  Machine Architecture: x86_64
      Operating System: GNU/Linux
              Hostname: desktop1

Built using gcc-6

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/6/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 6.2.0-6' --with-bugurl=file:///usr/share/doc/gcc-6/README.Bugs --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-6 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-6-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-6-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-6-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 6.2.0 20161010 (Debian 6.2.0-6)

Using version 0.28

$ mythfrontend --version

Please attach all output as a file in bug reports.
MythTV Version : v0.28
MythTV Branch : 
Network Protocol : 88
Library API : 0.28.20160309-1
QT Version : 5.6.1
Options compiled in:
 linux debug use_hidesyms using_alsa using_oss using_pulse using_pulseoutput using_backend using_bdjava using_bindings_perl using_bindings_python using_bindings_php using_crystalhd using_dvb using_firewire using_frontend using_hdhomerun using_vbox using_ceton using_hdpvr using_ivtv using_joystick_menu using_libcec using_libcrypto using_libdns_sd using_libfftw3 using_libxml2 using_lirc using_mheg using_opengl using_opengl_video using_opengl_themepainter using_qtwebkit using_qtscript using_qtdbus using_sdl using_taglib using_v4l2 using_x11 using_xrandr using_xv using_debugtype using_mythlogserver using_bdjava using_bindings_perl using_bindings_python using_bindings_php using_fontconfig using_freetype2 using_mythtranscode using_opengl using_vaapi using_vdpau using_ffmpeg_threads using_mheg using_libass using_libxml2

Backtrace:

$ gdb mythfrontend
Cannot make QOpenGLContext current in a different thread

Thread 50 "Decoder" received signal SIGABRT, Aborted.
[Switching to Thread 0x7ffe63541700 (LWP 9919)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:58
58	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff35defdf in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:58
#1  0x00007ffff35e040a in __GI_abort () at abort.c:89
#2  0x00007ffff4498dd1 in QMessageLogger::fatal(char const*, ...) const (context=..., message=<synthetic pointer>) at global/qlogging.cpp:1648
#3  0x00007ffff4498dd1 in QMessageLogger::fatal(char const*, ...) const (this=this@entry=0x7ffe635402a0, msg=msg@entry=0x7ffff50f1d08 "Cannot make QOpenGLContext current in a different thread") at global/qlogging.cpp:790
#4  0x00007ffff4d75073 in QOpenGLContext::makeCurrent(QSurface*) (this=0xd73ea0, surface=0xde2070) at kernel/qopenglcontext.cpp:954
#5  0x00007fffec7302f2 in QGLContext::makeCurrent() (this=<optimized out>) at qgl.cpp:3574
#6  0x00007ffff64c7dce in MythRenderOpenGL::makeCurrent() (this=0xdde340) at mythrender_opengl.cpp:294
#7  0x00007ffff64c6094 in OpenGLLocker::OpenGLLocker(MythRenderOpenGL*) (this=0x7ffe63540380, render=0xdde340) at mythrender_opengl.cpp:50
#8  0x00007ffff75b84e2 in VideoOutputOpenGL::SetupDeinterlace(bool, QString const&) (this=0x15abd10, interlaced=false, overridefilter=...) at videoout_opengl.cpp:762
#9  0x00007ffff752ba1e in VideoOutput::FallbackDeint() (this=0x15abd10) at videooutbase.cpp:666
#10 0x00007ffff74485fb in MythPlayer::ChangeSpeed() (this=0x1fc4f90) at mythplayer.cpp:3741
#11 0x00007ffff743feea in MythPlayer::FileChangedCallback() (this=0x1fc4f90) at mythplayer.cpp:2708
#12 0x00007ffff74abbcd in DecoderBase::FileChanged() (this=0xdaa500) at decoderbase.cpp:902
#13 0x00007ffff74b6ed1 in NuppelDecoder::GetFrame(DecodeTypes) (this=0xdaa500, decodetype=kDecodeAV) at nuppeldecoder.cpp:1069
#14 0x00007ffff74465de in MythPlayer::DecoderGetFrame(DecodeTypes, bool) (this=0x1fc4f90, decodetype=kDecodeAV, unsafe=false) at mythplayer.cpp:3469
#15 0x00007ffff7445ef1 in MythPlayer::DecoderLoop(bool) (this=0x1fc4f90, pause=true) at mythplayer.cpp:3384
#16 0x00007ffff742ad1c in DecoderThread::run() (this=0x1513a90) at mythplayer.cpp:97
#17 0x00007ffff67c749d in MThreadInternal::run() (this=0x15990d0) at mthread.cpp:79
#18 0x00007ffff44b1d78 in QThreadPrivate::start(void*) (arg=0x15990d0) at thread/qthread_unix.cpp:341
#19 0x00007ffff41ee464 in start_thread (arg=0x7ffe63541700) at pthread_create.c:333
#20 0x00007ffff369497f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:105

Attachments (1)

20181203_1625_filechanged.patch (2.3 KB) - added by Peter Bennett 6 years ago.
Possible fix

Download all attachments as: .zip

Change History (30)

comment:1 Changed 8 years ago by William L. DeRieux IV <WilliamDeRieux@…>

I just realized that this happened while I was watching a video in my library and not while watching live tv as originally reported.

comment:2 Changed 8 years ago by WilliamDeRieux@…

As it turns out this happens both when watching video and live tv.

comment:3 Changed 8 years ago by Peter Bennett

Can you supply a short video file that has this problem. Also please let us know what your setting is for video playback profile.

comment:4 in reply to:  3 ; Changed 8 years ago by WilliamDeRieux@…

Replying to pbennett:

Can you supply a short video file that has this problem. Also please let us know what your setting is for video playback profile.

I wouldn't be able to supply any video due to copyright/broadcast liscensing (even a short one)

I use OpenGL for both painting and rendering....the playback profile that I use now is 'OpenGL Slim' but I was using 'OpenGL High Quality'. It only seemed to happen when using OpenGL -- the other painters/renders were fine.

I also fixed the issue in my copy of the source.
MythPlayer::ChangeSpeed?() will call VideoOutput::FallbackDeint?() before then calling VideoOutputOpenGL::SetupDeinterlace?
The problem seems to be in the deinterlaing methods because it will call QOpenGLContext::makeCurrent() on the wrong thread.

-- I simply commented out the body of the FallbackDeint?() method since I don't use deinterlaing and as a result I haven't had this issue occur after.

this what that method looks like now:

/** \fn VideoOutput::FallbackDeint(void)
 *  \brief Fallback to non-frame-rate-doubling deinterlacing method.
 */
void VideoOutput::FallbackDeint(void)
{
    /*SetupDeinterlace(false);
    if (db_vdisp_profile)
        SetupDeinterlace(true, db_vdisp_profile->GetFallbackDeinterlacer());*/
}

comment:5 in reply to:  4 Changed 8 years ago by WilliamDeRieux@…

Replying to WilliamDeRieux@…:

Replying to pbennett:

Can you supply a short video file that has this problem. Also please let us know what your setting is for video playback profile.

I wouldn't be able to supply any video due to copyright/broadcast liscensing (even a short one)

I use OpenGL for both painting and rendering....the playback profile that I use now is 'OpenGL Slim' but I was using 'OpenGL High Quality'. It only seemed to happen when using OpenGL -- the other painters/renders were fine.

I also fixed the issue in my copy of the source.
MythPlayer::ChangeSpeed?() will call VideoOutput::FallbackDeint?() before then calling VideoOutputOpenGL::SetupDeinterlace?
The problem seems to be in the deinterlaing methods because it will call QOpenGLContext::makeCurrent() on the wrong thread.

-- I simply commented out the body of the FallbackDeint?() method since I don't use deinterlaing and as a result I haven't had this issue occur after.

this what that method looks like now:

/** \fn VideoOutput::FallbackDeint(void)
 *  \brief Fallback to non-frame-rate-doubling deinterlacing method.
 */
void VideoOutput::FallbackDeint(void)
{
    /*SetupDeinterlace(false);
    if (db_vdisp_profile)
        SetupDeinterlace(true, db_vdisp_profile->GetFallbackDeinterlacer());*/
}

Also, this issue is not specific to a single video file.
The ultimate solution to the issue would be to make the call to QOpenGLContext::makeCurrent() happen on the correct thread (maybe my calling QOpenGLContext::moveToThread() as per the QT documentation that I linked to in the original post)

comment:6 Changed 8 years ago by Peter Bennett

You can extract a small part of a video, a minute or two. If we receive a takedown request from the copyright owner we will be sure to take its URL down. I do not think anybody will object to a two minute video clip that we will use for debugging purposes. I normally share a clip using dropbox. I have currently 4 video clips in my dropbox that I have shared in various tickets and nobody has sent me a takedown request. You can extract 200MB from a recording by using, for example

dd bs=1MB count=201 skip=4889 if=3819_20160207020000.mpg of=extracted_with_dd.mpg

I would not attempt to fix a bug unless I could first reproduce it. I do not have an analog recorder so I cannot create any nuv files, and even if I did, I don't know the conditions that will cause the error.

It may not be specific to a video, but when it does happen, is it reproducible in occurring at the same place every time you play the video?

comment:7 in reply to:  6 ; Changed 8 years ago by WilliamDeRieux@…

Replying to pbennett:

You can extract a small part of a video, a minute or two. If we receive a takedown request from the copyright owner we will be sure to take its URL down. I do not think anybody will object to a two minute video clip that we will use for debugging purposes. I normally share a clip using dropbox. I have currently 4 video clips in my dropbox that I have shared in various tickets and nobody has sent me a takedown request. You can extract 200MB from a recording by using, for example

dd bs=1MB count=201 skip=4889 if=3819_20160207020000.mpg of=extracted_with_dd.mpg

I would not attempt to fix a bug unless I could first reproduce it. I do not have an analog recorder so I cannot create any nuv files, and even if I did, I don't know the conditions that will cause the error.

It may not be specific to a video, but when it does happen, is it reproducible in occurring at the same place every time you play the video?

Ok....your are misunderstanding the problem.

this is a purely a QT5 rendering issue

Supplying you with a video wouldn't make any difference.

This is the setup I use:
1) ATI TV Wonder Pro 500 (analog capture)
2) I use QT 5.7.1
3) I also use OpenGL for video rendering (not OpenGLES)
-- the only relevant thing here is using OpenGL 5 rendering and the setup of the deinterlacer

What version of QT are you using?

Do you use OpenGL (and not OpenGLES)?


The problem occurs when VideoOutputOpenGL::SetupDeinterlace? is called
and then the code does this:

OpenGLLocker ctx_lock(gl_context);

This causes the following to be called on the wrong thread and it shouldn't matter what the video content is.

OpenGLLocker::OpenGLLocker(MythRenderOpenGL *render) : m_render(render)
{
    if (m_render)
        m_render->makeCurrent();
}

This is what the MythRenderOpenGL::makeCurrent method looks like (and is where the issue is occuring)
Apparently: QGLContext::makeCurrent() will then makes a call to QOpenGLContext::makeCurrent -- I don't know if the call has a valid reference to a surface.
And USE_OPENGL_QT5 is only defined if USING_OPENGLES is also defined

#if defined USING_OPENGLES && QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
#define USE_OPENGL_QT5
void MythRenderOpenGL::makeCurrent()
{
    m_lock.lock();
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
    // Testing MythRenderOpenGL::currentContext is not reliable
    if (!m_lock_level++)
    {
#ifdef USE_OPENGL_QT5
        QOpenGLContext::makeCurrent(m_window);
#else
            QGLContext::makeCurrent();
#endif
    }
#else
    if (this != MythRenderOpenGL::currentContext())
        QGLContext::makeCurrent();
    m_lock_level++;
 #endif
}

comment:8 in reply to:  7 Changed 8 years ago by WilliamDeRieux@…

Replying to WilliamDeRieux@…:

Replying to pbennett:

You can extract a small part of a video, a minute or two. If we receive a takedown request from the copyright owner we will be sure to take its URL down. I do not think anybody will object to a two minute video clip that we will use for debugging purposes. I normally share a clip using dropbox. I have currently 4 video clips in my dropbox that I have shared in various tickets and nobody has sent me a takedown request. You can extract 200MB from a recording by using, for example

dd bs=1MB count=201 skip=4889 if=3819_20160207020000.mpg of=extracted_with_dd.mpg

I would not attempt to fix a bug unless I could first reproduce it. I do not have an analog recorder so I cannot create any nuv files, and even if I did, I don't know the conditions that will cause the error.

It may not be specific to a video, but when it does happen, is it reproducible in occurring at the same place every time you play the video?

Also, after commenting out this function's body:

/** \fn VideoOutput::FallbackDeint(void)
 *  \brief Fallback to non-frame-rate-doubling deinterlacing method.
 */
void VideoOutput::FallbackDeint(void)
{
    /*SetupDeinterlace(false);
    if (db_vdisp_profile)
        SetupDeinterlace(true, db_vdisp_profile->GetFallbackDeinterlacer());*/
}
    QString GetFallbackDeinterlacer(void) const
        { return GetPreference("pref_deint1"); }

I can set the video scan mode to 'Interlaced (Reverse)' by bringing up the menu by pressing M -> Video -> Advanced -> Video Scan -> Interlaced (Revered)

Most of my videos are Auto-detected as Progressive video -- so the deinterlacer isn't available in that case.

After that I have access to the deinterlacer M -> Video -> Advanced -> Deinterlacer Where I can choose from several different deinterlacers all of which work correctly.

Apparently the issue happens when the Fallback Deinterlacer is chosen.

Perhaps this is not properly set when the code calls: db_vdisp_profile->GetFallbackDeinterlacer?() from the VideoDisplayProfile? class.

If GetFallbackDeinterlacer? returns the preference for: pref_deint1 -- this might not be set in the preferences. I don't know where to set this default value through the user interface.

The point here is that commenting out this function: FallbackDeint? fixes the problem and deinterlacing still continues to work if set from the menu.

comment:9 Changed 8 years ago by Peter Bennett

Primary and Fallback deinterlacers are set in the video playback profile.

Front end setup->Video->playback->playback profile (3/8)->Edit->Next

So I suppose you can go in there and change the fallback deinterlacer to another value or to "None". That may work around your problem without needing a code change.

I want to see if the playback can work with a code fix that does not prevent fallback deinterlacers from being used. Perhaps the "makeCurrent" can be omitted or the OpenGL thread can be changed as you had noted.

I do not have an ATI wonder. (I had one sitting in a box for years and threw it out about a year ago). I only use a Ceton which records digitally. I use VDPAU rendering, but OpenGL also works fine for me. My QT version is 5.5.1.

It seems from your stack trace that the fallback deinterlacer is being called as a result of some change in the video, maybe it changes resolution or speed. I have one video that changes resolution, recorded from a channel that changed from SD to HD between programs. I tested it with OpenGL and it plays perfectly. Videos that change resolution fail with VAAPI rendering because of an issue with a wrong thread being used, but on OpenGL they are normally OK.

comment:10 in reply to:  9 ; Changed 8 years ago by WilliamDeRieux@…

Replying to pbennett:

Primary and Fallback deinterlacers are set in the video playback profile.

Front end setup->Video->playback->playback profile (3/8)->Edit->Next

So I suppose you can go in there and change the fallback deinterlacer to another value or to "None". That may work around your problem without needing a code change.

I want to see if the playback can work with a code fix that does not prevent fallback deinterlacers from being used. Perhaps the "makeCurrent" can be omitted or the OpenGL thread can be changed as you had noted.

I do not have an ATI wonder. (I had one sitting in a box for years and threw it out about a year ago). I only use a Ceton which records digitally. I use VDPAU rendering, but OpenGL also works fine for me. My QT version is 5.5.1.

It seems from your stack trace that the fallback deinterlacer is being called as a result of some change in the video, maybe it changes resolution or speed. I have one video that changes resolution, recorded from a channel that changed from SD to HD between programs. I tested it with OpenGL and it plays perfectly. Videos that change resolution fail with VAAPI rendering because of an issue with a wrong thread being used, but on OpenGL they are normally OK.

I could try setting the default deinterlacer, but I'm not totally convinced that it would impact/solve the QT threading issue.

Also, I would suspect -- especially with live tv -- that the file changed not because of resolution or speed but because I was watching it while it was being actively recorded (the file contents would, naturally, change every time a captured frame was flushed to disk).

When you say that the recording changed from SD to HD I assume that you mean the frame size actually changed.

With the ATI TV Wonder the capture resolution is 720x480 (1.5:1) and seems to be resized to 480x480 (1:1 -> 4:3) -- occasionally I get a 16:9 letterbox (top and bottom borders), but the frame size of recorded video is always 480x480. I'm not sure if the recorder would detect a change in aspect ratio when the framesize is the same.

Furthermore, I think the real solution to this issue would be to make sure that the call to makeCurrent happens on the correct thread.

I don't think that makeCurrent can be omitted -- it is necessary for rendering.

Also I can see an issue in the makeCurrent method:
qgl.h -> QGLContext::makeCurrent() [QT4/QT5]
qopenglcontext.h -> QOpenGLContext::makeCurrent(QSurface *surface) [QT5 only]

void MythRenderOpenGL::makeCurrent()
{
    m_lock.lock();
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
    // Testing MythRenderOpenGL::currentContext is not reliable
    if (!m_lock_level++)
    {
#ifdef USE_OPENGL_QT5
        QOpenGLContext::makeCurrent(m_window);
#else
            QGLContext::makeCurrent();
#endif
    }
#else
    if (this != MythRenderOpenGL::currentContext())
        QGLContext::makeCurrent();
    m_lock_level++;
 #endif
}

It's already testing if the version of QT is greater than or equal to 5.4, but because USE_OPENGL_QT5 is not defined unless OpenGLES is being used -- the QT5 version of makeCurrent(m_window) is never called -- it defaults to calling QGLContext::makeCurrent(); with no surface. Also, there is no point in comparing the context to MythRenderOpenGL::currentContext() -- since it will just end up calling QGLContext::makeCurrent() anyways

And since I am using QT 5.7 -- it would make since to call the QT5 version. I think the method would better written like this (but I have not tried this variant):

void MythRenderOpenGL::makeCurrent()
{
    m_lock.lock();
    // Testing MythRenderOpenGL::currentContext is not reliable
    if (!m_lock_level++)
    {
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
        QOpenGLContext::makeCurrent(m_window);
#else
            QGLContext::makeCurrent();
#endif
    }
    m_lock_level++;
}

comment:11 in reply to:  10 Changed 8 years ago by WilliamDeRieux@…

Replying to WilliamDeRieux@…:

It's already testing if the version of QT is greater than or equal to 5.4, but because USE_OPENGL_QT5 is not defined unless OpenGLES is being used -- the QT5 version of makeCurrent(m_window) is never called -- it defaults to calling QGLContext::makeCurrent(); with no surface. Also, there is no point in comparing the context to MythRenderOpenGL::currentContext() -- since it will just end up calling QGLContext::makeCurrent() anyways

And since I am using QT 5.7 -- it would make since to call the QT5 version. I think the method would better written like this (but I have not tried this variant):

void MythRenderOpenGL::makeCurrent()
{
    m_lock.lock();
    // Testing MythRenderOpenGL::currentContext is not reliable
    if (!m_lock_level++)
    {
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
        QOpenGLContext::makeCurrent(m_window);
#else
            QGLContext::makeCurrent();
#endif
    }
    m_lock_level++;
}

I can see that the modification to this method (that I just mentioned) would not take into account version(s) of QT less than 5.4 and as a result would no longer call this part:

    if (this != MythRenderOpenGL::currentContext())
        QGLContext::makeCurrent();
    m_lock_level++;

However, I suspect that versions of QT less than 5.4 should be deprecated, especially since the use 5.4 and later should be widespread by now.

I don't know if that would be an issue.

comment:12 Changed 8 years ago by WilliamDeRieux@…

I just tested this again without my modifications. I forced MythPlayer::ChangeSpeed?() to be called by doing the following 1) Play video or live tv 2) Menu -> Playback -> Adjust Time Stretch -> set to anything other than 1.0x

And I had no issue occur.

The problem must be the way in which MythPlayer::ChangeSpeed?() is called....from the decoder thread. Also, I seem to remember this issue taking a little while to take effect (maybe watiching live tv for at least an hour or so)

#10 0x00007ffff74485fb in MythPlayer::ChangeSpeed() (this=0x1fc4f90) at mythplayer.cpp:3741
#11 0x00007ffff743feea in MythPlayer::FileChangedCallback() (this=0x1fc4f90) at mythplayer.cpp:2708
#12 0x00007ffff74abbcd in DecoderBase::FileChanged() (this=0xdaa500) at decoderbase.cpp:902
#13 0x00007ffff74b6ed1 in NuppelDecoder::GetFrame(DecodeTypes) (this=0xdaa500, decodetype=kDecodeAV) at nuppeldecoder.cpp:1069
#14 0x00007ffff74465de in MythPlayer::DecoderGetFrame(DecodeTypes, bool) (this=0x1fc4f90, decodetype=kDecodeAV, unsafe=false) at mythplayer.cpp:3469
#15 0x00007ffff7445ef1 in MythPlayer::DecoderLoop(bool) (this=0x1fc4f90, pause=true) at mythplayer.cpp:3384

comment:13 Changed 8 years ago by Stuart Auchterlonie

Right, we definitely need to resolve this as it's a Qt5 specific issue.

It looks like either the fallback de-interlacer or the change speed code is doing the wrong thing. Based on comment#12 i suspect it's the fallback de-interlacer code.

comment:14 in reply to:  13 ; Changed 8 years ago by WilliamDeRieux@…

Replying to stuarta:

Right, we definitely need to resolve this as it's a Qt5 specific issue.

It looks like either the fallback de-interlacer or the change speed code is doing the wrong thing. Based on comment#12 i suspect it's the fallback de-interlacer code.

You said it is related to #comment:12 -- that was actually a link #ticket:12 -- and that it is a problem with the fallback de-interlacer code.

MythPlayer::ChangeSpeed?() will call VideoOutput::FallbackDeint?() which passes the fallback de-interlacer to VideoOutputOpenGL::SetupDeinterlace?(bool, QString const&) this function then directly calls OpenGLLocker::OpenGLLocker(MythRenderOpenGL*) which causes the error 'Cannot make QOpenGLContext current in a different thread'.

From that, I guess, the issue occurs when the de-interlacer is setup on a thread that does not belong to the QOpenGLContext.

The faulting line in SetupDeinterlace? is this:

OpenGLLocker ctx_lock(gl_context);

which calls:

OpenGLLocker::OpenGLLocker(MythRenderOpenGL *render) : m_render(render)
{
    if (m_render)
        m_render->makeCurrent();
}

I suppose that there needs to be some sort of check in the OpenGLLocker constructor that

1) checks if the the call is on the correct thread.

or

2) makes a call to m_render->moveToThread(QThread *targetThread) http://doc.qt.io/qt-5/qobject.html#moveToThread

This is a quote from the QT 5 Documentation:

http://doc.qt.io/qt-5/qopenglcontext.html#thread-affinity

Thread Affinity

QOpenGLContext can be moved to a different thread with moveToThread(). Do not 
call makeCurrent() from a different thread than the one to which the 
QOpenGLContext object belongs. A context can only be current in one thread 
and against one surface at a time, and a thread only has one context current 
at a time.

Also, given the above, I suppose it is actually a problem with QT5 and that it has nothing to do with video decoding or anything else.

Thanks.

comment:15 in reply to:  14 Changed 8 years ago by WilliamDeRieux@…

Replying to WilliamDeRieux@…:

Replying to stuarta:

Right, we definitely need to resolve this as it's a Qt5 specific issue.

It looks like either the fallback de-interlacer or the change speed code is doing the wrong thing. Based on comment#12 i suspect it's the fallback de-interlacer code.


Also from the original backtrace it is occurring within the context of the "Decoder" thread

comment:16 Changed 8 years ago by Roger Siddons

The stack trace suggests that playback was not 1x speed at the time.

I can't test atm but I suspect a quick fix is to revert 7b4402def. It's not wholly wrong but exposes deeper issues within the player code.

IMO:

Prior to 2010 ChangeSpeed() was only used by UI thread as a "handle effects of changing speed" and safely called rendering.

59f10b398 caused it to be called from decoder thread (by FileChangedCallback). That appears to be mainly due to changing LiveTv programme ("new programme so reset everything").

That seemed ok because:

  1. If previous programme couldn't support a 2x deinterlacer then then it's unlikely the new one will, so BestDeint() will never be invoked by decoder thread.
  2. Prior to 7b4402def, if playback is not 1x speed then we cannot be currently using a 2x deinterlacer, so FallbackDeint() will never be invoked by decoder thread.

I suspect the issue will only occur when using 2x deinterlacer & playback is not 1x. Changing speed via UI will never invoke it directly (UI thread does the work). However a subsequent decoder event will. LiveTv programme transition will probably trigger it. If a video is triggering it then something else is also causing the decoder to reset.

The underlying issue is that the decoder thread should be signalling the UI thread to effect the deinterlacer change.

Maybe FileChangedCallback should somehow get the UI to invoke ChangeSpeed() ? It's not clear whether that would cause other issues.

Changing the FallbackDeint() condition to be the same as 7b4402def should work also. Note that the ChangeSpeed() code is almost a cut/paste of ForceDeinterlacer(). That would restore the previous "hopefully this will never get invoked by wrong thread" situation. Some comments would be nice! Although ugly, an explicit thread-check here is warranted to signal the underlying dodginess.

Disallowing user selection of 2x interlacers when playback is not 1x simply avoids the problem arising. Arguably it is the same solution by a different (more opaque) means.

comment:17 Changed 8 years ago by Stuart Auchterlonie

Based on that analysis, that makes me think that we should be using Qt's signals and slots to signal that up to the UI thread.

On a side note, when looking at this, I noted that the VAAPI code does a check to see if it's actually on the UI thread before doing a makeCurrent()

https://code.mythtv.org/cgit/mythtv/tree/mythtv/libs/libmythtv/videoout_openglvaapi.cpp#n137

which may be a potential way to identify issues like this.

comment:18 in reply to:  17 Changed 8 years ago by WilliamDeRieux@…

Replying to stuarta:

Based on that analysis, that makes me think that we should be using Qt's signals and slots to signal that up to the UI thread.

On a side note, when looking at this, I noted that the VAAPI code does a check to see if it's actually on the UI thread before doing a makeCurrent()

https://code.mythtv.org/cgit/mythtv/tree/mythtv/libs/libmythtv/videoout_openglvaapi.cpp#n137

which may be a potential way to identify issues like this.

Yeah, that code in CreateVAAPIContext would probably help to check if the UI thread is active.

if (!gCoreContext->IsUIThread())
{
    LOG(VB_GENERAL, LOG_ERR, LOC + "CreateVAAPIContext called from non-UI thread");
    return false;
}

comment:19 Changed 8 years ago by Stuart Auchterlonie

Milestone: 0.28.10.28.2

Moving remaining open 0.28.1 tickets to 0.28.2

comment:20 Changed 7 years ago by Stuart Auchterlonie

Resolution: Won't Fix
Status: newclosed

Closing any remaining tickets for 0.28, if the issue persists, feel free to reopen and align to v29 or master

comment:21 Changed 6 years ago by gigem

Resolution: Won't Fix
Status: closednew

I'm reopening this as it's a serious issue for my mom on her Nvidia Shield. It's actually been a serious issue for quite some time but I didn't know how to reproduce it. As it turns out, it's trivial to reproduce. You just need to be patient. To do that, tune live TV to an interlaced channel and leave it there. It might take several hours but it will eventually hit this problem.

comment:22 Changed 6 years ago by Peter Bennett

Owner: set to Peter Bennett
Status: newaccepted

comment:23 Changed 6 years ago by Peter Bennett

Component: Qt5 issuesMythTV - Video Playback
Milestone: 0.28.230.0
Priority: criticalminor
Severity: highmedium

comment:24 Changed 6 years ago by Peter Bennett

As Roger has pointed out, the decoder thread calls MythPlayer::FileChangedCallback which then lands up calling the OpenGL API from the wrong thread. The solution looks to be for the decoder thread to set an indicator that the player thread will check in EventLoop and call MythPlayer::FileChangedCallback from the player thread.

Changed 6 years ago by Peter Bennett

Possible fix

comment:25 Changed 6 years ago by Peter Bennett

Please see attached patch, which now uses the player thread for the code from FileChangeCallback.

In testing live TV transitions I could not get FileChangeCallback to execute, in either current or patched code. My tests never crashed.

If there is a crash with a different backtrace than the one presented, please attach it.

This patch should prevent a crash if it occurs in the place where the above back trace was taken.

Let me know if the patch fixes the issue and I can commit it.

comment:26 Changed 6 years ago by dizygotheca

Looks like FileChangeCallback only gets called for discontiguous transitions, ie. changing channel, inputs etc. HDPVR's appear more likely to generate them (CardUtil::IsChannelChangeDiscontinuous) Channel-surfing with a HDPVR is probably the best test.

comment:27 Changed 6 years ago by Peter Bennett

I only have a ceton recorder. I did test with channel changes, including changing between a 720p channel and a 1080i channel, and FileChangeCallback was not called.

comment:28 Changed 6 years ago by Peter Bennett <pbennett@…>

Resolution: fixed
Status: acceptedclosed

In 79b086899b/mythtv:

Playback: Fix crash caused by OpenGL call from wrong thread

MythPlayer::FileChangedCallback? was being called from the decoder thread,
and it in turn sometimes invokes code to update the OpenGL
filters. Changed it to ensure that code is called from the
playback thread.

Fixes #12902

comment:29 Changed 6 years ago by Peter Bennett <pbennett@…>

In 79b086899b/mythtv:

Playback: Fix crash caused by OpenGL call from wrong thread

MythPlayer::FileChangedCallback? was being called from the decoder thread,
and it in turn sometimes invokes code to update the OpenGL
filters. Changed it to ensure that code is called from the
playback thread.

Fixes #12902

Note: See TracTickets for help on using tickets.