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

QAudioInput: possible change of state without emitting stateChange()

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • P3: Somewhat important
    • 4.6.3
    • 4.6.3
    • Multimedia
    • None
    • qt git repo, clone of branch 4.7 done on March 23, 2010

    Description

      As in the summary above.

      I have a test application: I connect to signals stateChagned() (debug marked below as "slotStateChanged()"), and notify() (marked "slotAudioNotification()") plus I have my own timer which checks in fixed period the state of QAudioInput etc. (marked "slotPullInfo()"). I get the following

      "slotStateChanged(): state changed to  QAudio::ActiveState"                                                                                                     
      "slotAudioNotification(): (0)bytesReady, buffSize(3200), elapsed (28 ms), processed (0 ms), no error, QAudio::ActiveState"   
      "slotPullInfo(): (1400)bytesReady, buffSize(3200), elapsed (17957 ms), processed (17760 ms), No such process, QAudio::ActiveState"
      "slotAudioNotification(): (632)bytesReady, buffSize(3200), elapsed (1028 ms), processed (980 ms), no error, QAudio::ActiveState"
      "slotPullInfo(): (0)bytesReady, buffSize(3200), elapsed (0 ms), processed (0 ms), no error, QAudio::StoppedState"
      "slotPullInfo(): (0)bytesReady, buffSize(3200), elapsed (0 ms), processed (0 ms), no error, QAudio::StoppedState"
      "slotPullInfo(): (0)bytesReady, buffSize(3200), elapsed (0 ms), processed (0 ms), no error, QAudio::StoppedState"
      

      Now, the point is that between the last slotAudioNotification() and the subsequent "slotPullInfo()" the state of QAudioInput did change, but the corresponding signal was not emitted: I did not get "slotStateChanged()" debug.

      =====================================
      It seems that the reason why this happens is that close() and open() in qaudioinput_alsa_p.cpp are not symmetric:

      • upon successful completion neither of them emits signal stateChanged (which is fine)
      • but close() updates deviceState
      • while open() does not update deviceState

      So, after the sequence "close(); open();" in xrun_recovery there is no signal emitted, but ... deviceState gets updated to QAudio::StoppedState. The situation which I described above will thus happen upon every single call to xrun_recovery (I did confirm by adding extra debug to qaudioinput_alsa_p.cpp that this is the call to this function what happens in the case of my application, when the status changes without the corresponding signal being emitted).
      =====================================
      SOLUTION 1:
      The simple solution would be to modify the code of xrun_recovery() to have something like:

          if(reset) {
              close();
              if ( !open() )
              {
                   // in this case open() emits already stateChanged() and updates errorState; just return:
                   return -1; 
              }
              // the key change:
              deviceState  = QAudio::ActiveState;
              snd_pcm_prepare(handle);
              return 0;
          }
      

      instead of current

          if(reset) {
              close();
              open();
              snd_pcm_prepare(handle);
              return 0;
          }
      

      But the change is ugly and my feeling is that it is a bit counter-intuitive to have open() and close() behaving in a not symmetric way, and it may lead to similar errors in the future. Note that open() and close() used to be symmetric -> they both updated deviceState upon success: a corresponding line was removed by Kurt as reaction to QTBUG-8440. Maybe this particular change was not needed? If so, possible SOLUTION 2 would be to roll back the following part of patch QTBUG-8440:

      @@ -413,7 +415,6 @@ bool QAudioInputPrivate::open()
           timer->start(period_time*chunks/2000);
       
           errorState  = QAudio::NoError;
      -    deviceState = QAudio::ActiveState;
       
           totalTimeValue = 0;
      

      This would make both open() and close() change deviceState, which seems quite intuitive.

      The SOLUTION 3, ofc, is to remove line

      deviceState = QAudio::StoppedState;
      

      from close(). The update of deviceState would have to move to the code calling close(). This means that neither open() nor close() would change deviceState. Note that there are two places in qaudioinput_alsa_p.cpp which have "deviceState = ..." just following the call to close(). For example, in read() there are lines

          if (bytesAvailable < 0) {
              close();
              errorState = QAudio::IOError;
              deviceState = QAudio::StoppedState;
              emit stateChanged(deviceState);
              return 0;
          }
      

      => this would suggest that indeed close() was not expected to change deviceState (but this is probably code without such a "deep" meaning).

      ==========================
      I would vote for solutions 2 (Question: what else would need to be changed to avoid reopening QTBUG-8440), my second choice would be solution 3 (a bit ugly, but we are talking about open() and close() of private class, it does not seem too much off to have extra handling [i.e. setting deviceState] after every call to open()/close()). I would really hate if just xrun_recovery was fixed and open() and close() would behave differently as far as setting new value to deviceState is concerned. But ofc, whatever matches the spirit of the rest of the code (and not-alsa parts) best.

      Attachments

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

        Activity

          People

            korbatit Kurt Korbatits (closed Nokia identity) (Inactive)
            wiecko Marek Wieckowski
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes