Qt: RTP audio stream fixups.

Make sure audio_stream_ is non-NULL before we try to use it. Delete
audio_stream_ more gracefully and add a note about mutexes on OS X and
Windows.

Bug: 12166
Change-Id: I12e76c49e631bc1de813c5c7d82c7d928c71237e
Reviewed-on: https://code.wireshark.org/review/15759
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>

3 files changed, 14 insertions, 7 deletions

diff --git a/ui/qt/rtp_audio_stream.cpp b/ui/qt/rtp_audio_stream.cpp
index 6d0dfc72e1..7bd072ef52 100644
--- a/ui/qt/rtp_audio_stream.cpp
+++ b/ui/qt/rtp_audio_stream.cpp
@@ -532,7 +532,7 @@ void RtpAudioStream::startPlaying()
 
     audio_output_ = new QAudioOutput(format, this);
     audio_output_->setNotifyInterval(65); // ~15 fps
-    connect(audio_output_, SIGNAL(stateChanged(QAudio::State)), this, SLOT(outputStateChanged()));
+    connect(audio_output_, SIGNAL(stateChanged(QAudio::State)), this, SLOT(outputStateChanged(QAudio::State)));
     connect(audio_output_, SIGNAL(notify()), this, SLOT(outputNotify()));
     tempfile_->seek(0);
     audio_output_->start(tempfile_);
@@ -540,7 +540,7 @@ void RtpAudioStream::startPlaying()
     // QTBUG-6548 StoppedState is not always emitted on error, force a cleanup
     // in case playback fails immediately.
     if (audio_output_ && audio_output_->state() == QAudio::StoppedState) {
-        outputStateChanged();
+        outputStateChanged(QAudio::StoppedState);
     }
 }
 
@@ -566,12 +566,18 @@ void RtpAudioStream::writeSilence(int samples)
     visual_samples_ += visual_fill;
 }
 
-void RtpAudioStream::outputStateChanged()
+void RtpAudioStream::outputStateChanged(QAudio::State new_state)
 {
-    switch (audio_output_->state()) {
+    if (!audio_output_) return;
+
+    // On some platforms including OS X and Windows, the stateChanged signal
+    // is emitted while a QMutexLocker is active. As a result we shouldn't
+    // delete audio_output_ here.
+    switch (new_state) {
     case QAudio::StoppedState:
         // RTP_STREAM_DEBUG("stopped %f", audio_output_->processedUSecs() / 100000.0);
-        delete audio_output_;
+        audio_output_->disconnect();
+        audio_output_->deleteLater();
         audio_output_ = NULL;
         emit finishedPlaying();
         break;
diff --git a/ui/qt/rtp_audio_stream.h b/ui/qt/rtp_audio_stream.h
index 063332c0d1..e60e5e7ebe 100644
--- a/ui/qt/rtp_audio_stream.h
+++ b/ui/qt/rtp_audio_stream.h
@@ -185,7 +185,7 @@ private:
     void writeSilence(int samples);
 
 private slots:
-    void outputStateChanged();
+    void outputStateChanged(QAudio::State new_state);
     void outputNotify();
 };
 
diff --git a/ui/qt/rtp_player_dialog.cpp b/ui/qt/rtp_player_dialog.cpp
index 3dc9184312..161132973b 100644
--- a/ui/qt/rtp_player_dialog.cpp
+++ b/ui/qt/rtp_player_dialog.cpp
@@ -61,7 +61,7 @@ Q_DECLARE_METATYPE(QCPGraph *)
 //   + dst port + ssrc. This means that we can have multiple rtp_stream_info
 //   structs per RtpAudioStream. Should we make them 1:1 instead?
 
-// Current RTP player bugs:
+// Current and former RTP player bugs. Many have attachments that can be usef for testing.
 // Bug 3368 - The timestamp line in a RTP or RTCP packet display's "Not Representable"
 // Bug 3952 - VoIP Call RTP Player: audio played is corrupted when RFC2833 packets are present
 // Bug 4960 - RTP Player: Audio and visual feedback get rapidly out of sync
@@ -72,6 +72,7 @@ Q_DECLARE_METATYPE(QCPGraph *)
 // Bug 10613 - RTP audio player crashes
 // Bug 11125 - RTP Player does not show progress in selected stream in Window 7
 // Bug 11409 - Wireshark crashes when using RTP player
+// Bug 12166 - RTP audio player crashes
 
 // XXX It looks like we duplicate some functionality here and in the RTP
 // analysis code, which has its own routines for writing audio data to a