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