Daniel Yek wrote:
Modified by: [EMAIL PROTECTED]
Date: 10/24/2006
Project: Helix Player
Synopsis: Currently, we hear 2 channels only when playing 5.1 channels
content
with Helix and RealPlayer.
This change enables multi-channels playback on ALSA.
Overview:
(1)
Many member functions implementation in Helix Client Core's ALSA audio
device code, audlinux_alsa.cpp, share the same member variable,
m_wLastError.
When an error occurred in a member function, the ending _CloseAudio()
call can wipe out the existing error condition.
The code looked like this:
m_wLastError = RA_AOE_BADFORMAT;
_CloseAudio(); // This function always set: "m_wLastError =
RA_AOE_NOERR;"
return m_wLastError;
So, we always returned RA_AOE_NOERR in _CheckFormat(), even when the
system
can't play the format.
(2)
The implementation was catered for 2-channel playback only.
Playing back 5.1 channels content caused ALSA's "default" PCM device to
invoke its ("plug" and) "route" plugin and shrink the channels to 2
channels,
discarding surround sound content.
The right ALSA PCM device to use to play 5.1 content is the
"plug:surround51" PCM,
which only takes 6-channel input.
If the system isn't set up to play 5.1 content, snd_pcm_open() this PCM
would fail and Helix client would then fallback to 2-channel and use the
"default" PCM.
Note that on some mis-configured system, "plug:surround51" was defined
even though
the system isn't 5.1 capable. This is system configuration problem.
On such system, ALSA would invoke the "plug" plugin, which uses
"route" plugin to
discard surround sound content and playback in 2-channel mode.
On these systems, we really want to have Helix client "preserve" all
channels by
down-mixing it to 2 channels.
So, do we properly handle mis-configured systems? If not, we should. At
least we should have a pref that will let users force 2-channel playback,
if we don't already.
(3)
ALSA now always configures the sound card to 48000Hz.
So, Helix's ALSA audio device _CheckFormat() code now forces Helix client
to use 48000Hz.
Is this the case for *all* ALSA systems out in the wild? If not, we need
to handle that.
I am only reviewing the core parts below. In the future you should
break up the CR so that at least the core parts are seperate. If
someone sees me respond to this CR they may assume that I
looked at all of it and not read it. They you also don't have to
cross post to many lists (helix-client-dev is never used for CRs,
audio-dev would have been fine for just the core parts).
- virtual HX_RESULT _OpenAudio() = 0;
+ virtual HX_RESULT _OpenAudio( const HXAudioFormat* pFormat ) = 0;
I am not sure I like changing this in audUnix.h. If you do, you need
to change it everywhere, not just ALSA and OSS:
D:\cygwin\home\gwright\helix\audio\device\pub\platform\openwave\audopwave.h(205):
virtual HX_RESULT _OpenAudio();
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audhpux.h(100):
virtual HX_RESULT _OpenAudio();
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audirix.h(100):
virtual HX_RESULT _OpenAudio();
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audlinux_alsa.h(99):
virtual HX_RESULT _OpenAudio();
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audlinux_esound.h(118):
virtual HX_RESULT _OpenAudio();
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audlinux_oss.h(110):
virtual HX_RESULT _OpenAudio();
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audSolaris.h(102):
virtual HX_RESULT _OpenAudio();
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audUnix.h(221):
virtual HX_RESULT _OpenAudio() = 0;
D:\cygwin\home\gwright\helix\audio\device\pub\platform\unix\audusound.h(100):
virtual HX_RESULT _OpenAudio();
I think it would be better, since they all inherit from audUnix, that you
just store the format in a member var (maybe a new one in audUnix) and
let the child classes use it. So, in _OpenAudio they already have the
format in, for example, m_pAudioFormat.
+
+ if (m_bCheckFormat)
+ {
+ // Print ALSA PCM device name while doing checking format.
+ HXLOGL4 (HXLOG_ADEV, "Opening ALSA PCM device, %s, to check format!",
szDevice);
+ }
+ else
+ {
+ // Print ALSA PCM device name outside of _CheckFormat() function (that
is, it is actually being used).
+ HXLOGL4(HXLOG_ADEV, "Opening ALSA PCM device, %s, for actual use!",
szDevice);
+ printf("Opening ALSA PCM device %s\n", szDevice);
+ }
I think those should be L2 log statements. It would be helpful to have
those when trying to debug a player out in the field (you only get L1
and L2 in release builds).
-
+ // Output to stdout too because the code is still quite new.
+ printf("Device Configured:\n");
+ printf(" Sample Rate: %u\n", (unsigned int) m_unSampleRate);
+ printf(" Sample Width: %u\n", (unsigned int) m_uSampFrameSize);
+ printf(" Num channels: %u\n", (unsigned int) m_unNumChannels);
+ printf(" Block size: %d\n", m_wBlockSize);
+ printf(" Device buffer size: %lu\n", (unsigned long int)
m_ulDeviceBufferSize);
+ printf(" Supports HW Pause: %d\n", m_bHasHardwarePauseAndResume);
+ printf(" Start threshold: %d\n", (int) start_threshold);
+ printf("\n");
+ fflush(stdout);
only do the above in _DEBUG builds....
+ if(state == SND_PCM_STATE_XRUN)
+ {
+ HXLOGL1 ( HXLOG_ADEV, "XRUN in GetBytesActuallyPlayedUsingDelay()");
+ }
...
if (err < 0)
{
- HXLOGL1 ( HXLOG_ADEV, "snd_pcm_status: %s", snd_strerror(err));
+ HXLOGL1 ( HXLOG_ADEV, "snd_pcm_delay: %s", snd_strerror(err));
+ }
+ else if (frame_delay < 0)
+ {
+ HXLOGL1 ( HXLOG_ADEV, "XRUN! frame_delay: %d", frame_delay);
+ nBytesPlayed = m_ulTotalWritten;
+ retVal = HXR_OK; // Is this really OK?
}
Since those are L1 logs, make sure they only fire during important
error conditions. L1s should fire very rarely or just at startup.
-// HXLOGL4 ( HXLOG_ADEV, "nBytesPlayed: %llu, m_ulTotalWritten: %llu\n",
nBytesPlayed, m_ulTotalWritten);
+ HXLOGL2 ( HXLOG_ADEV, "GetBytesActuallyPlayedUsingDelay() nBytesPlayed: %llu,
m_ulTotalWritten: %llu \n", nBytesPlayed, m_ulTotalWritten);
This looks like it should *NOT* be a L2, L1 and L2 are in all release builds.
They should not fire all the time. Also, don't leave code in that is
commented out, just remove it unless it is really needed to understand
the code.
+ // Add AlsaVaryingSampleRate into Preference.
+ IHXBuffer* pBuffer = new CHXBuffer;
+ pBuffer->AddRef();
+ pBuffer->SetSize(2);
+ SafeSprintf((char*) pBuffer->GetBuffer(),2,"%d", 0); /*
Flawfinder: ignore */
+ z_pIHXPrefs->WritePref("AlsaVaryingSampleRate", pBuffer);
+ pBuffer->Release();
+ }
this seems like overkill. I think you can just do:
WritePrefUINT32( m_pContext, "AlsaVaryingSampleRate", 0 );
Not really part of this CR, but what happens if a user selects ALSA as
the output system and they do not have it installed?
--greg.
_______________________________________________
Audio-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/audio-dev
