Mythfrontend/VDPAU crashes in v35-pre

[angelaschmid]

• Platform: Ubuntu 22.04.5 LTS (GNU/Linux 6.6.4 x86_64)

• MythTV version: v35-pre, latest master 8258345d41aa8aa8218817e908b048f2e6246e2f

• Package version: self compiled

• Component: mythfrontend / VDPAU

What steps will reproduce the bug?

Mythfrontend with VDPAU crashes opening recording/livetv/video.

After reverting it works: commit 728caf282f09f52c5dc388bea0098e7c15d6eb71 Author: David Hampton mythtv@love2code.net Date: Wed Jul 31 15:49:10 2024 -0400

tidy: Move assignments to constructor member initialization list. (libs 2)

Promote member initialization by assignment in the body of the
constructor to be part of the constructor initialization list.  If
possible, further promote these values to default member initializers.

Changes made by clang-tidy.  White space cleanup and some reordering
by hand.

https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.html
https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html

Additional information

#0  MythVDPAUHelper::InitProcs() (this=0x555557df2b00) at decoders/mythvdpauhelper.cpp:291
#1  0x00007ffff7a27b46 in MythVDPAUHelper::MythVDPAUHelper(AVVDPAUDeviceContext*) (this=this@entry=0x555557df2b00, Context=Context@entry=0x555558eb0700) at decoders/mythvdpauhelper.cpp:237
#2  0x00007ffff7a59278 in MythVDPAUInterop::InitNV(AVVDPAUDeviceContext*) (this=0x55555c7859e0, DeviceContext=0x555558eb0700) at opengl/mythvdpauinterop.cpp:136
#3  0x00007ffff7a5b1d6 in MythVDPAUInterop::Acquire(MythRenderOpenGL*, MythVideoColourSpace*, MythVideoFrame*, FrameScanType) (this=0x55555c7859e0, Context=<optimized out>, ColourSpace=0x55555cade508, Frame=0x55555cb02c60, Scan=<optimized out>) at opengl/mythvdpauinterop.cpp:274
#4  0x00007ffff7a4ad23 in MythOpenGLInterop::Retrieve(MythRenderOpenGL*, MythVideoColourSpace*, MythVideoFrame*, FrameScanType) (Context=0x555555da53c0, ColourSpace=0x55555cade508, Frame=Frame@entry=0x55555cb02c60, Scan=Scan@entry=kScan_Progressive) at opengl/mythopenglinterop.cpp:114
#5  0x00007ffff7a44248 in MythOpenGLVideo::RenderFrame(MythVideoFrame*, bool, FrameScanType, StereoscopicMode, bool) (this=0x55555ba40d80, Frame=0x55555cb02c60, TopFieldFirst=<optimized out>, Scan=<optimized out>, StereoOverride=<optimized out>, DrawBorder=<optimized out>) at opengl/mythopenglvideo.cpp:706
#6  0x00007ffff7a49511 in MythVideoOutputOpenGL::RenderFrame(MythVideoFrame*, FrameScanType) (this=0x55555cade3f0, Frame=0x55555cb02c60, Scan=<optimized out>) at opengl/mythvideooutopengl.cpp:325
#7  0x00007ffff7929443 in MythPlayerUI::RenderVideoFrame(MythVideoFrame*, FrameScanType, bool, std::chrono::duration<long, std::ratio<1l, 1000000l> >) (this=0x55555b7f8740, Frame=0x55555cb02c60, Scan=kScan_Progressive, Prepare=<optimized out>, Wait=...) at mythplayerui.cpp:532
#8  0x00007ffff7929a36 in MythPlayerUI::DoDisplayVideoFrame(MythVideoFrame*, std::chrono::duration<long, std::ratio<1l, 1000000l> >) (this=0x55555b7f8740, Frame=0x55555cb02c60, Due=...) at mythplayerui.cpp:613
#9  0x00007ffff7929d7f in MythPlayerUI::DisplayNormalFrame(bool) (this=0x55555b7f8740, CheckPrebuffer=<optimized out>) at mythplayerui.cpp:711
#10 0x00007ffff7926f92 in MythPlayerUI::VideoLoop() (this=0x55555b7f8740) at mythplayerui.cpp:501
#11 0x00007ffff78ce190 in TV::PlaybackLoop() (this=this@entry=0x55555adc75f0) at tv_play.cpp:1283
#12 0x00007ffff78edaab in TV::StartTV(ProgramInfo*, unsigned int, std::vector<ChannelInfo, std::allocator<ChannelInfo> > const&) (TVRec=TVRec@entry=0x7fffffffd110, Flags=Flags@entry=16, Selection=std::vector of length 0, capacity 0) at tv_play.cpp:357
#13 0x000055555560afce in PlaybackBox::Play(ProgramInfo const&, bool, bool, bool, bool, bool) (this=0x555557bc4fc0, rec=<optimized out>, inPlaylist=<optimized out>, ignoreBookmark=<optimized out>, ignoreProgStart=<optimized out>, ignoreLastPlayPos=<optimized out>, underNetworkControl=false) at playbackbox.cpp:2446
#14 0x000055555560b2b2 in PlaybackBox::PlayX(ProgramInfo const&, bool, bool, bool, bool) (this=0x555557bc4fc0, pginfo=..., ignoreBookmark=false, ignoreProgStart=<optimized out>, ignoreLastPlayPos=<optimized out>, underNetworkControl=false) at playbackbox.cpp:2210
#15 0x00007ffff22f1793 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#16 0x00007ffff6bf60d6 in MythUIButtonList::itemClicked(MythUIButtonListItem*) (this=this@entry=0x5555574fb700, _t1=<optimized out>, _t1@entry=0x555556f099a0) at moc/moc_mythuibuttonlist.cpp:224
#17 0x00007ffff6b3680c in MythUIButtonList::keyPressEvent(QKeyEvent*) (this=0x5555574fb700, event=<optimized out>) at mythuibuttonlist.cpp:2655
#18 0x000055555562adce in PlaybackBox::keyPressEvent(QKeyEvent*) (this=0x555557bc4fc0, event=0x55555adc4440) at playbackbox.cpp:3811
#19 0x00007ffff6a9e792 in MythMainWindow::eventFilter(QObject*, QEvent*) (this=0x555555c7d0c0, Watched=0x555555c7d0c0, Event=<optimized out>) at mythmainwindow.cpp:1697
#20 0x00007ffff22b9b9a in QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#21 0x00007ffff636c702 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#22 0x00007ffff637449e in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#23 0x00007ffff22b9e3a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#24 0x00007ffff6b9908b in MythInputDeviceHandler::customEvent(QEvent*) (this=<optimized out>, Event=<optimized out>) at devices/mythinputdevicehandler.cpp:266
#25 0x00007ffff22e733f in QObject::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x00007ffff636c713 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#27 0x00007ffff22b9e3a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#28 0x00007ffff22bcf27 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#29 0x00007ffff2313a67 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#30 0x00007fffed79cd3b in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#31 0x00007fffed7f22b8 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#32 0x00007fffed79a3e3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#33 0x00007ffff23130b8 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#34 0x00007ffff22b875b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#35 0x00007ffff22c0cf4 in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#36 0x00005555555e1610 in main(int, char**) (argc=<optimized out>, argv=<optimized out>) at mythfrontend.cpp:2333

[angelaschmid]

With master and the following diff, it works.

diff --git a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
index c11c948f7e..41c8cb3b21 100644
--- a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
+++ b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
@@ -234,10 +234,10 @@ static void vdpau_preemption_callback(VdpDevice /*unused*/, void* Opaque)
  * \brief A simple wrapper around VDPAU functionality.
 */
 MythVDPAUHelper::MythVDPAUHelper(AVVDPAUDeviceContext* Context)
-  : m_valid(InitProcs()),
-    m_device(Context->device),
+  : m_device(Context->device),
     m_vdpGetProcAddress(Context->get_proc_address)
 {
+    m_valid = InitProcs();
     if (m_valid)
     {
         INIT_ST

InitProcs() is a non-static member function. The initializer list is executed before the constructor body, meaning the function isn't ready to be accessed yet.

[linuxdude42]

The problem is that the GET_PROC macro in InitProcs() uses m_device. I'm planning to swap the order that m_valid and m_device are defined in the header file, so I can swap the order they are initialized in the function. That should solve the problem.

[linuxdude42]

I tried to test that function last night, but it appears that a Nvidia video card is required. (My system has Intel video.) Can you try this patch and see if it fixes the crash?

diff --git a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
index c11c948f7e..7deee54c5a 100644
--- a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
+++ b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
@@ -234,8 +234,8 @@ static void vdpau_preemption_callback(VdpDevice /*unused*/, void* Opaque)
  * \brief A simple wrapper around VDPAU functionality.
 */
 MythVDPAUHelper::MythVDPAUHelper(AVVDPAUDeviceContext* Context)
-  : m_valid(InitProcs()),
-    m_device(Context->device),
+  : m_device(Context->device),
+    m_valid(InitProcs()),
     m_vdpGetProcAddress(Context->get_proc_address)
 {
     if (m_valid)
@@ -254,8 +254,8 @@ static const char* DummyGetError(VdpStatus /*status*/)
 }

 MythVDPAUHelper::MythVDPAUHelper(void)
-  : m_createdDevice(true),
-    m_display(MythXDisplay::OpenMythXDisplay(false))
+  : m_display(MythXDisplay::OpenMythXDisplay(false)),
+    m_createdDevice(true)
 {
     if (!m_display)
         return;
diff --git a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h
index 244e18cb50..dfb8736731 100644
--- a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h
+++ b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h
@@ -100,10 +100,10 @@ class MythVDPAUHelper : public QObject
     bool   InitProcs(void);

   private:
-    bool                              m_valid                            { false   };
-    bool                              m_createdDevice                    { false   };
     VdpDevice                         m_device                           { 0       };
     MythXDisplay                     *m_display                          { nullptr };
+    bool                              m_createdDevice                    { false   };
+    bool                              m_valid                            { false   };

     VdpGetProcAddress                *m_vdpGetProcAddress                { nullptr };
     VdpGetErrorString                *m_vdpGetErrorString                { nullptr };

[angelaschmid]

(gdb) bt
#0  MythVDPAUHelper::InitProcs() (this=0x5555572dfad0) at decoders/mythvdpauhelper.cpp:291
#1  0x00007ffff7a27b67 in MythVDPAUHelper::MythVDPAUHelper(AVVDPAUDeviceContext*) (this=this@entry=0x5555572dfad0, Context=Context@entry=0x555559374b00) at decoders/mythvdpauhelper.cpp:238
#2  0x00007ffff7a592a8 in MythVDPAUInterop::InitNV(AVVDPAUDeviceContext*) (this=0x55555bf46b60, DeviceContext=0x555559374b00) at opengl/mythvdpauinterop.cpp:136
#3  0x00007ffff7a5b206 in MythVDPAUInterop::Acquire(MythRenderOpenGL*, MythVideoColourSpace*, MythVideoFrame*, FrameScanType) (this=0x55555bf46b60, Context=<optimized out>, ColourSpace=0x55555cca3b48, Frame=0x55555be6ab00, Scan=<optimized out>) at opengl/mythvdpauinterop.cpp:274
#4  0x00007ffff7a4ad53 in MythOpenGLInterop::Retrieve(MythRenderOpenGL*, MythVideoColourSpace*, MythVideoFrame*, FrameScanType) (Context=0x555555d9e990, ColourSpace=0x55555cca3b48, Frame=Frame@entry=0x55555be6ab00, Scan=Scan@entry=kScan_Progressive) at opengl/mythopenglinterop.cpp:114
#5  0x00007ffff7a44278 in MythOpenGLVideo::RenderFrame(MythVideoFrame*, bool, FrameScanType, StereoscopicMode, bool) (this=0x55555bcd24c0, Frame=0x55555be6ab00, TopFieldFirst=<optimized out>, Scan=<optimized out>, StereoOverride=<optimized out>, DrawBorder=<optimized out>) at opengl/mythopenglvideo.cpp:706
#6  0x00007ffff7a49541 in MythVideoOutputOpenGL::RenderFrame(MythVideoFrame*, FrameScanType) (this=0x55555cca3a30, Frame=0x55555be6ab00, Scan=<optimized out>) at opengl/mythvideooutopengl.cpp:325
#7  0x00007ffff7929443 in MythPlayerUI::RenderVideoFrame(MythVideoFrame*, FrameScanType, bool, std::chrono::duration<long, std::ratio<1l, 1000000l> >) (this=0x55555ba895a0, Frame=0x55555be6ab00, Scan=kScan_Progressive, Prepare=<optimized out>, Wait=...) at mythplayerui.cpp:532
#8  0x00007ffff7929a36 in MythPlayerUI::DoDisplayVideoFrame(MythVideoFrame*, std::chrono::duration<long, std::ratio<1l, 1000000l> >) (this=0x55555ba895a0, Frame=0x55555be6ab00, Due=...) at mythplayerui.cpp:613
#9  0x00007ffff7929d7f in MythPlayerUI::DisplayNormalFrame(bool) (this=0x55555ba895a0, CheckPrebuffer=<optimized out>) at mythplayerui.cpp:711
#10 0x00007ffff7926f92 in MythPlayerUI::VideoLoop() (this=0x55555ba895a0) at mythplayerui.cpp:501
#11 0x00007ffff78ce190 in TV::PlaybackLoop() (this=this@entry=0x55555a79e530) at tv_play.cpp:1283
#12 0x00007ffff78edaab in TV::StartTV(ProgramInfo*, unsigned int, std::vector<ChannelInfo, std::allocator<ChannelInfo> > const&) (TVRec=TVRec@entry=0x7fffffffd120, Flags=Flags@entry=16, Selection=std::vector of length 0, capacity 0) at tv_play.cpp:357
#13 0x000055555560afce in PlaybackBox::Play(ProgramInfo const&, bool, bool, bool, bool, bool) (this=0x555557c45a90, rec=<optimized out>, inPlaylist=<optimized out>, ignoreBookmark=<optimized out>, ignoreProgStart=<optimized out>, ignoreLastPlayPos=<optimized out>, underNetworkControl=false) at playbackbox.cpp:2446
#14 0x000055555560b2b2 in PlaybackBox::PlayX(ProgramInfo const&, bool, bool, bool, bool) (this=0x555557c45a90, pginfo=..., ignoreBookmark=false, ignoreProgStart=<optimized out>, ignoreLastPlayPos=<optimized out>, underNetworkControl=false) at playbackbox.cpp:2210
#15 0x00007ffff22f1793 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#16 0x00007ffff6bf60d6 in MythUIButtonList::itemClicked(MythUIButtonListItem*) (this=this@entry=0x5555575008b0, _t1=<optimized out>, _t1@entry=0x555556f0ce50) at moc/moc_mythuibuttonlist.cpp:224
#17 0x00007ffff6b3680c in MythUIButtonList::keyPressEvent(QKeyEvent*) (this=0x5555575008b0, event=<optimized out>) at mythuibuttonlist.cpp:2655
#18 0x000055555562adce in PlaybackBox::keyPressEvent(QKeyEvent*) (this=0x555557c45a90, event=0x55555921d5d0) at playbackbox.cpp:3811
#19 0x00007ffff6a9e792 in MythMainWindow::eventFilter(QObject*, QEvent*) (this=0x555555c7c990, Watched=0x555555c7c990, Event=<optimized out>) at mythmainwindow.cpp:1697
#20 0x00007ffff22b9b9a in QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#21 0x00007ffff636c702 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#22 0x00007ffff637449e in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#23 0x00007ffff22b9e3a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#24 0x00007ffff6b9908b in MythInputDeviceHandler::customEvent(QEvent*) (this=<optimized out>, Event=<optimized out>) at devices/mythinputdevicehandler.cpp:266
#25 0x00007ffff22e733f in QObject::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x00007ffff636c713 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#27 0x00007ffff22b9e3a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#28 0x00007ffff22bcf27 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#29 0x00007ffff2313a67 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#30 0x00007fffed79cd3b in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#31 0x00007fffed7f22b8 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#32 0x00007fffed79a3e3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#33 0x00007ffff23130b8 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#34 0x00007ffff22b875b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#35 0x00007ffff22c0cf4 in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#36 0x00005555555e1610 in main(int, char**) (argc=<optimized out>, argv=<optimized out>) at mythfrontend.cpp:2333

My opinion:

InitProcs() is a non-static member function. The initializer list is executed before the constructor body, meaning the function isn't ready to be accessed yet.

this is not yet fully constructed, so accessing class members is not yet possible. At the time of the call, m_vdpGetProcAddress is not set yet, and this can lead to function pointer retrieval failures. m_vdpGetProcAddress is not initialized until after InitProcs() in the initializer list.

[linuxdude42]

I should have looked more closely. That member variable is set in the very next line. How about this patch instead:

diff --git a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
index c11c948f7e..750137d6f8 100644
--- a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
+++ b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.cpp
@@ -234,9 +234,9 @@ static void vdpau_preemption_callback(VdpDevice /*unused*/, void* Opaque)
  * \brief A simple wrapper around VDPAU functionality.
 */
 MythVDPAUHelper::MythVDPAUHelper(AVVDPAUDeviceContext* Context)
-  : m_valid(InitProcs()),
-    m_device(Context->device),
-    m_vdpGetProcAddress(Context->get_proc_address)
+  : m_device(Context->device),
+    m_vdpGetProcAddress(Context->get_proc_address),
+    m_valid(InitProcs())
 {
     if (m_valid)
     {
@@ -254,8 +254,8 @@ static const char* DummyGetError(VdpStatus /*status*/)
 }

 MythVDPAUHelper::MythVDPAUHelper(void)
-  : m_createdDevice(true),
-    m_display(MythXDisplay::OpenMythXDisplay(false))
+  : m_display(MythXDisplay::OpenMythXDisplay(false)),
+    m_createdDevice(true)
 {
     if (!m_display)
         return;
diff --git a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h
index 244e18cb50..988cbe998a 100644
--- a/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h
+++ b/mythtv/libs/libmythtv/decoders/mythvdpauhelper.h
@@ -100,8 +100,6 @@ class MythVDPAUHelper : public QObject
     bool   InitProcs(void);

   private:
-    bool                              m_valid                            { false   };
-    bool                              m_createdDevice                    { false   };
     VdpDevice                         m_device                           { 0       };
     MythXDisplay                     *m_display                          { nullptr };

@@ -123,6 +121,9 @@ class MythVDPAUHelper : public QObject
     VdpOutputSurfaceDestroy          *m_vdpOutputSurfaceDestroy          { nullptr };
     VdpVideoSurfaceGetParameters     *m_vdpVideoSurfaceGetParameters     { nullptr };
     VdpPreemptionCallbackRegister    *m_vdpPreemptionCallbackRegister    { nullptr };
+
+    bool                              m_createdDevice                    { false   };
+    bool                              m_valid                            { false   };
 };

 #endif // MYTHVDPAUHELPER_H

[angelaschmid]

David, that fixed the issue. Great.

[linuxdude42]

Thanks for testing. I'll commit it.

[kmdewaal]

What suprises me is that the code changes suggested by clang-tidy are just commited while they in this case do introduce a bug. I really expect clang-tidy to generate code changes that can blindly be trusted. I am afraid that given the apparent unreliability of clang-tidy this way of working this will not be the last bug to be introduced by this kind of code changes.