Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-49569

QAudioOutput::start() does not emit stateChanged() on error

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: 5.5.1
    • Fix Version/s: 5.11.0
    • Component/s: Multimedia
    • Labels:
      None
    • Environment:
      Arch Linux x86_64 (pulseaudio)

      Description

      Contrary to what the documentation says, an error during QAudioOutput::start(QIODevice * device) does not result in a state changed signal (because the state was initially StoppedState, and after returning with an error it still is in StoppedState).

      Relevant text from documentation

      void QAudioOutput::start(QIODevice * device)

      ...

      If a problem occurs during this process, error() returns QAudio::OpenError, state() returns QAudio::StoppedState and the stateChanged() signal is emitted.

      This is at least the case for Linux with the pulseaudio backend. (I think the source contains invalid media and therefore opening it failed).

      qaudiooutput_pulse.cpp output:

      QIODevice *QPulseAudioOutput::start()
      {
          setState(QAudio::StoppedState);
          setError(QAudio::NoError);
      
          // Handle change of mode
          if (m_audioSource && !m_pullMode) {
              delete m_audioSource;
              m_audioSource = 0;
          }
      
          close();
      
          if (!open())
              return Q_NULLPTR;
      
          m_audioSource = new PulseOutputPrivate(this);
          m_audioSource->open(QIODevice::WriteOnly|QIODevice::Unbuffered);
          m_pullMode = false;
      
          setState(QAudio::IdleState);
      
          return m_audioSource;
      }
      

      Looks like a bug in the pulseaudio plugin, the alsa backend for example always emits a signal (regardless of errors), the winaudio plugin always emits the StoppedState signal too on errors. The qnx-audio plugin has the same setState function as pulseaudio and is also affected. I did not check others.

      Violation of the documented contract caused a crash in Wireshark which relied on the stateChanged signal for cleanup. Simplified code:

      void RtpAudioStream::startPlaying() {
          audio_output_ = new QAudioOutput(format, this);
          connect(audio_output_, SIGNAL(stateChanged(QAudio::State)), this, SLOT(outputStateChanged()));
          audio_output_->start(tempfile_);
      }
      void RtpAudioStream::stopPlaying() {
          if (audio_output_)
              audio_output_->stop();
      }
      void RtpAudioStream::outputStateChanged() {
          switch (audio_output_->state()) {
          case QAudio::StoppedState:
              delete audio_output_;
              audio_output_ = NULL;
              break;
          case QAudio::IdleState:
              audio_output_->stop();
              break;
          }
      }
      

      It expected that a call to `RtpAudioStream::stopPlaying()` resulted in a cleanup of the QAudioOutput object (via the QAudio::StoppedState change). Calling QAudioOutput::stop() did result in the signalState change though, resulting in a crash when the linked parent widget is gone.

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            valentyn.doroshchuk Valentyn Doroshchuk
            Reporter:
            Lekensteyn Peter Wu
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Gerrit Reviews

                There are no open Gerrit changes