Further comments:

@@ -201,6 +204,14 @@
 
 UINT16 CAudioOutUNIX::_Imp_GetVolume()
 {
+#if defined(_THREADED_AUDIO) && defined(_UNIX_THREADS_SUPPORTED)
+                //We want to sleep as a function of device buffer size.
+                //If we have a small m_ulDeviceBufferSize we can only 
+                //afford to sleep just a little while.
+                HX_ASSERT( m_ulDeviceBufferSize != 0 );
+                m_ulSleepTime = 
(((float)m_ulDeviceBufferSize/(float)m_uSampFrameSize)/
+                                 (float)m_unSampleRate) * 1000 / 
(float)m_unNumChannels;
+#endif
     if (!m_bMixerPresent)
         _OpenMixer();
 
@@ -271,14 +282,6 @@
             }
             else
             {
-#if defined(_THREADED_AUDIO) && defined(_UNIX_THREADS_SUPPORTED)
-                //We want to sleep as a function of device buffer size.
-                //If we have a small m_ulDeviceBufferSize we can only 
-                //afford to sleep just a little while.
-                HX_ASSERT( m_ulDeviceBufferSize != 0 );
-                m_ulSleepTime = 
(((float)m_ulDeviceBufferSize/(float)m_uSampFrameSize)/
-                                 (float)m_unSampleRate) * 1000 / 
(float)m_unNumChannels;
-#endif
                 if (!m_bMixerPresent)
                     _OpenMixer();   
 

Was there some reason you decided to move this
from _Imp_Open() to _Imp_GetVolume()?

Rest looks good to me. Greg: do you have any
further comments?

Eric

=============================================
Eric Hyche ([EMAIL PROTECTED])
Technical Lead
RealNetworks, Inc.  

> -----Original Message-----
> From: Rusty Lynch [mailto:[EMAIL PROTECTED] 
> Sent: Thursday, April 17, 2008 7:40 PM
> To: audio-dev
> Cc: midplayer-private-dev; Eric Hyche; Greg Wright
> Subject: Take 3 => CR: Remove polling inside UNIX audio thread
> 
> Ahh!  Just took a another look at my patch and realized that I was
> adding an extra unneeded micro sleep when exiting.  Not a big deal in
> the grand scheme of things, but not very clean.
> 
> This version of the patch now only calls micro sleep if the 
> bReadyToExit
> is false and there is data to write.
> 
>     --rusty
> 
> On Thu, 2008-04-17 at 16:22 -0700, Rusty Lynch wrote:
> > On Thu, 2008-04-17 at 17:03 -0400, Eric Hyche wrote:
> > > Rusty,
> > > 
> > > Here are my comments. Greg may want to comment further.
> > > 
> > > @@ -142,6 +141,7 @@
> > >   CreateInstanceCCF(CLSID_IHXMutex, 
> (void**)&m_mtxWriteListPlayStateLock, m_pContext);
> > >   CreateInstanceCCF(CLSID_IHXMutex, 
> (void**)&m_mtxDeviceStateLock, m_pContext);
> > >   CreateInstanceCCF(CLSID_IHXThread, 
> (void**)&m_audioThread, m_pContext);
> > > + HXEvent::MakeEvent(m_pAvailableDataEvent, "Available 
> Audio Data", FALSE);
> > > 
> > > You should create an IHXEvent just like the above
> > > code creates IHXThread or IHXMutex:
> > > 
> > > +     CreateInstanceCCF(CLSID_IHXEvent, (void**) 
> &m_pAvailableDataEvent, m_pContext);
> > > 
> > > and of course, m_pAvailableDataEvent should be an IHXEvent rather
> > > than an HXEvent:
> > > 
> > > +    IHXEvent*   m_pAvailableDataEvent;
> > > 
> > > and you should release it rather than delete it:
> > > 
> > > +    HX_RELEASE( m_pAvailableDataEvent );
> > > 
> > 
> > Ok, I started using IHXEvent, and since I wanted a infinite 
> wait I am
> > calling:
> > 
> > m_pAvailableDataEvent->Wait(ALLFS);
> > 
> > ... which I am just guessing (from looking at other code) 
> is cool, but
> > ALLFS isn't translating to anything sensible in my brain, 
> so let me know
> > if that is not something I behavior I can count on.
> > 
> > > 
> > > What would happen if the audio device was full and we
> > > still had data to write? It seems like we would just
> > > spin in a tight loop until the audio device had space
> > > for us to write. Not really a failure case, but
> > > seems like we should wait in that case as well.
> > > 
> > > Eric
> > > 
> > 
> > On an ALSA system _WriteBytes will eventually come down to an
> > snd_pcm_writei(), and since we are setting the audio device 
> to blocking
> > in _OpenAudio(), then snd_pcm_writei() will block until the 
> device is
> > able to consume all the bits.  
> > 
> > 
> http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.ht
> ml#gf13067c0ebde29118ca05af76e5b17a9
> > 
> > Can I count on other UNIX systems having a blocking write 
> behavior?  I
> > don't know.  I think esd (esound) blocks on write, but I'm 
> having a hard
> > time finding any API documentation to verify that.
> > 
> > hmm... looks like Solaris is opening the device node using 
> O_NONBLOCK.
> > 
> > Ok, here is a version of the patch that will do a 
> microsleep if there is
> > data to write, but only if HELIX_FEATURE_ALSA is defined.
> > 
> > The block of code in question now looks more like...
> > 
> > if(bReadyToExit == FALSE && 
> >    (that->m_pWriteList->GetCount() == 0 || 
> >     that->m_wState == RA_AOS_OPEN_PAUSED))
> > {
> >      that->m_pAvailableDataEvent->Wait(ALLFS);
> > }
> > else
> > {
> > #if !defined(HELIX_FEATURE_ALSA)
> >     microsleep(that->m_ulSleepTime/4);
> > #endif
> > }
> > 
> > 
> > 
> > _______________________________________________
> > Midplayer-private-dev mailing list
> > [EMAIL PROTECTED]
> > 
> http://lists.helixcommunity.org/mailman/listinfo/midplayer-private-dev
> 


_______________________________________________
Audio-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/audio-dev

Reply via email to