Hi Eric,
Thanks for your your answer. I have been checking the code and doing some
testing with on that and I realized that pcmpeak would never take negative
values just for one particular case... so the signed casting is no
necesary... I explain what I've seen below the code
delay = lim->delay;
i = lim->idx;
for (pcmptr = pcmbuf; pcmptr < pcmbuf + nsamples; pcmptr += 2) {
unsigned int pcmleft = abs(pcmptr[LEFT]) ;
unsigned int pcmright = abs(pcmptr[RIGHT]) ;
/* peak detect */
unsigned int pcmpeak = MAX(pcmleft, pcmright) ;
/* compute the required attenuation */
if (pcmpeak == 0x80000000UL)
atten = MulShift31(FIX_ONE, lim->threshold) ;
else if (pcmpeak > lim->threshold)
atten = MulDiv64(FIX_ONE, lim->threshold, pcmpeak);
else
atten = FIX_ONE;
1) There are 4 bytes to represent signed or unsigned integer data, and
negative values are represented by binary 2 complement
2) pcmptr[] is a pointer to a signed int value, and it is placed into
pcmleft and pcmright which are unsigned int variables, and it is placed by
doing an absolute value to the pcmptr[] pointed data
3) the absolute value of a negative signed int (pcmptr[]) will clear the
most significant bit of the 4 bytes, and by 2 complements will get its
equivalent to positive... with the most significant value in zero
4) up to now, the value pointed to pcmptr[] is assigned into another
unsigned variable and there is no registry if it was originally assigned
from a positive or negative value.
3) After that the max value of pcmleft and pcmright is copied to the
unsigned int pcmpeak (the variable with the signed cast). The same bit set
is copied to pcmpeak, up to now, with the most significant bit in zero
6) There is just one exception, if bit set of the pointed value to which
pcmptr[] is pointing to, would be 0x80000000 the 2 complement of that
negative value is just the same bit set representation, but now assigned to
an unsigned int, which will be interpreted as positive. Then this value is
copied int another unsigned int (pcmpeak) and by making a signed casting to
this value, would return the original negative value. But this case will
never get to the casting, because there is an explicit if to that value:
if (pcmpeak == 0x80000000UL)
Probably the person who wrote this modules, was trying originally to prevent
that indivudual case of the 0x80000000, by placing a signed cast first and
then placed the prior "if" and forgetting to remove the signed cast.
So I think is not problem erasing 'signed'.
Thank you
Edgar
-----Original Message-----
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] Behalf Of Eric Hyche
Sent: Tuesday, April 26, 2005 12:23 PM
To: 'Felipe Lugo'; [email protected]
Cc: 'Ken Cooke'; 'Jon Recker'
Subject: RE: [Audio-dev] CR-Client-Resend: Remove "comparission
betweensignedand unsigned"and other warnings
Felipe:
I have checked in the audio/device/fakeaudiodevice.cpp
changes to HEAD and 150Cay. The audio-session_mmf.cpp changes
are moot now, since RuleToFlag() no longer exists in that file.
And I'm uncomfortable making the audio/limiter/limiter.c changes
until I understand more what the original code is doing.
Two values in question are both unsigned, so whoever put
the "(signed)" cast there clearly did it intentionally.
Jon/Ken/Hans: could one of you take a look at the
(signed) cast in the following code in LimiterMono()
and LimiterStereo() in audio/limiter/limiter.c and
tell me why it's there?
/* compute the required attenuation */
if (pcmpeak == 0x80000000UL)
atten = MulShift31(FIX_ONE, lim->threshold) ;
=====>>>>> else if ((signed)pcmpeak > lim->threshold)
atten = MulDiv64(FIX_ONE, lim->threshold, pcmpeak);
else
atten = FIX_ONE;
Eric
> -----Original Message-----
> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED] On Behalf Of Felipe Lugo
> Sent: Monday, April 25, 2005 6:11 PM
> To: [email protected]
> Subject: [Audio-dev] CR-Client-Resend: Remove "comparission
> between signedand unsigned"and other warnings
>
>
>
> -----Original Message-----
> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED] Behalf Of Alberto Meza
> Sent: Wednesday, April 06, 2005 2:03 PM
> To: audio-dev
> Subject: [Audio-dev] CR: Remove "comparission between signed and
> unsigned"and other warnings
>
>
> Modified by:[EMAIL PROTECTED]
> Reviewed by:
> Date:4/6/2005
> Project:Helix Symbian Port
>
> Synopsis: Remove "comparission between signed and
> unsigned" and other
> warnings
>
> device/fakeaudiodevice.cpp
> device/platform/symbian/audiosvr/mmf/audio_session-mmf.cpp
> limiter/limiter.c
>
> Branch: 150Cay and HEAD
>
> Copyright assignment:
> I agree to assign to RealNetworks full copyright ownership of the code
> represented by the attached patch. I warrant that I am
> legally entitled to
> grant the copyright assignment and that my contribution does
> not violate any
> law or breach any contract. I understand that RealNetworks
> may license this
> code under RPSL, RCSL, and/or any other license at RealNetworks' sole
> discretion.
>
>
>
> Index: device/fakeaudiodevice.cpp
> ===================================================================
> RCS file: /cvsroot/audio/device/fakeaudiodevice.cpp,v
> retrieving revision 1.2
> diff -u -w -r1.2 fakeaudiodevice.cpp
> --- device/fakeaudiodevice.cpp 14 Mar 2005 19:43:21
> -0000 1.2
> +++ device/fakeaudiodevice.cpp 6 Apr 2005 00:04:07 -0000
> @@ -65,7 +65,7 @@
>
> CHXFakeAudioDevice::CHXFakeAudioDevice()
> {
> - fprintf(stderr,
> "0x%08x::CHXFakeAudioDevice::CHXFakeAudioDevice()\n",
> this);
> + fprintf(stderr,
> "0x%08x::CHXFakeAudioDevice::CHXFakeAudioDevice()\n",
> (unsigned int)this);
> m_pContext = NULL;
> m_pScheduler = NULL;
> m_lRefCount = 0;
> @@ -84,7 +84,7 @@
>
> CHXFakeAudioDevice::~CHXFakeAudioDevice()
> {
> - fprintf(stderr,
> "0x%08x::CHXFakeAudioDevice::~CHXFakeAudioDevice()\n",
> this);
> + fprintf(stderr,
> "0x%08x::CHXFakeAudioDevice::~CHXFakeAudioDevice()\n",(unsigne
> d int) this);
> Close(TRUE);
> HX_RELEASE(m_pContext);
> HX_RELEASE(m_pScheduler);
> @@ -138,7 +138,7 @@
>
> STDMETHODIMP CHXFakeAudioDevice::Open(const HXAudioFormat*
> pAudioFormat,
> IHXAudioDeviceResponse* pStreamResponse)
> {
> - fprintf(stderr, "0x%08x::CHXFakeAudioDevice::Open()\n", this);
> + fprintf(stderr, "0x%08x::CHXFakeAudioDevice::Open()\n", (unsigned
> int)this);
> HX_RESULT retVal = HXR_FAIL;
>
> if (pAudioFormat && pStreamResponse && m_pContext && m_eState ==
> StateClosed)
> @@ -162,7 +162,7 @@
>
> STDMETHODIMP CHXFakeAudioDevice::Close(const HXBOOL bFlush)
> {
> - fprintf(stderr, "0x%08x::CHXFakeAudioDevice::Close(%lu)\n", this,
> bFlush);
> + fprintf(stderr,
> "0x%08x::CHXFakeAudioDevice::Close(%u)\n", (unsigned
> int)this, bFlush);
> // Clear any pending callbacks
> ClearCallback();
> // Release the response interface
> @@ -175,7 +175,7 @@
>
> STDMETHODIMP CHXFakeAudioDevice::Resume()
> {
> - fprintf(stderr, "0x%08x::CHXFakeAudioDevice::Resume()\n", this);
> + fprintf(stderr,
> "0x%08x::CHXFakeAudioDevice::Resume()\n", (unsigned
> int)this);
> HX_RESULT retVal = HXR_FAIL;
>
> if (m_pScheduler && m_pResponse && m_pCallbackTime &&
> @@ -206,7 +206,7 @@
>
> STDMETHODIMP CHXFakeAudioDevice::Pause()
> {
> - fprintf(stderr, "0x%08x::CHXFakeAudioDevice::Pause()\n", this);
> + fprintf(stderr, "0x%08x::CHXFakeAudioDevice::Pause()\n",
> (unsigned
> int)this);
> HX_RESULT retVal = HXR_OK;
>
> if (m_eState == StateResumed)
> Index: device/platform/symbian/audiosvr/mmf/audio_session-mmf.cpp
> ===================================================================
> RCS file:
> /cvsroot/audio/device/platform/symbian/audiosvr/mmf/audio_sess
> ion-mmf.cpp,v
> retrieving revision 1.6
> diff -u -w -r1.6 audio_session-mmf.cpp
> ---
> device/platform/symbian/audiosvr/mmf/audio_session-mmf.cpp
> 4 Apr 2005
> 23:08:45 -0000 1.6
> +++
> device/platform/symbian/audiosvr/mmf/audio_session-mmf.cpp
> 6 Apr 2005
> 00:04:08 -0000
> @@ -165,7 +165,7 @@
> const char * StringifyKErr(TInt err) { return 0; }
> #endif
>
> -static TInt FlagToNumber(TMMFSampleRate flag)
> +TInt FlagToNumber(TMMFSampleRate flag)
> {
> switch( flag )
> {
> Index: limiter/limiter.c
> ===================================================================
> RCS file: /cvsroot/audio/limiter/limiter.c,v
> retrieving revision 1.5
> diff -u -w -r1.5 limiter.c
> --- limiter/limiter.c 9 Jul 2004 18:36:57 -0000 1.5
> +++ limiter/limiter.c 6 Apr 2005 00:06:26 -0000
> @@ -271,7 +271,7 @@
> /* compute the required attenuation */
> if (pcmpeak == 0x80000000UL)
> atten = MulShift31(FIX_ONE, lim->threshold) ;
> - else if ((signed)pcmpeak > lim->threshold)
> + else if (pcmpeak > lim->threshold)
> atten = MulDiv64(FIX_ONE,
> lim->threshold, pcmpeak);
> else
> atten = FIX_ONE;
> @@ -374,7 +374,7 @@
> /* compute the required attenuation */
> if (pcmpeak == 0x80000000UL)
> atten = MulShift31(FIX_ONE, lim->threshold) ;
> - else if ((signed)pcmpeak > lim->threshold)
> + else if (pcmpeak > lim->threshold)
> atten = MulDiv64(FIX_ONE,
> lim->threshold, pcmpeak);
> else
> atten = FIX_ONE;
>
>
> _______________________________________________
> Audio-dev mailing list
> [email protected]
> http://lists.helixcommunity.org/mailman/listinfo/audio-dev
>
_______________________________________________
Audio-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/audio-dev
_______________________________________________
Audio-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/audio-dev
