[Live-devel] Proper bool type for MSVC

[Ross Finlayson]

> The Boolean type in your library is defined as unsigned char. However, VS2008 fully supports proper bool/true/false type.  Could you please consider the following changes in code (library from 2012-05-17)
>  
> 1. Enable bool support for MSVC: in the UsageEnvironment\include\Boolean.hh file replace:
> 
> #ifdef   __BORLANDC__
> #define Boolean bool
> 
> by
> 
> #if defined(__BORLANDC__)  ||  (defined(_MSC_VER) &&  _MSC_VER >= 1400)    // MSVC++ 8.0, Visual Studio 2005 and higher
> #define Boolean bool 
>  
> That enables bool for Boolean in the library.

OK, I'll make this change in the next release of the software.


> After that I've found some places where the Boolean variables are improperly used. Here is the list of places and proposed fixes:
>  
> File MPEG4LATMAudioRTPSource.cpp
>  
> in line 175 replace
>  
> audioMuxVersion = 0;
> allStreamsSameTimeFraming = 1;
>  
> by
> 
> audioMuxVersion = false;
> allStreamsSameTimeFraming = true;

OK (except that it'll be "False" and "True" rather than "false" and "true").


> in line 187-190 change
>  
> audioMuxVersion = (nextByte&0x80)>>7;
> if (audioMuxVersion != 0) break;
> allStreamsSameTimeFraming = (nextByte&0x40)>>6;
>  
> by
>  
> audioMuxVersion = (nextByte&0x80) != 0;
> if (audioMuxVersion) break;
> allStreamsSameTimeFraming = (nextByte&0x40)>>6 != 0; 

OK.


> File MP3InternalsHuffman.cpp
>  
> in line 552 replace
>  
> scaleFactorsLength = getScaleFactorsLength(gr, isMPEG2);
>  
> by
>  
> scaleFactorsLength = getScaleFactorsLength(gr, isMPEG2 != 0);
>  
> Actually it's more accurate to change the isMPEG2 type to Boolean

Yes, that's the right thing to do.


> File MP3Internals.cpp
>  
> in line 176 replace
>  
> hasCRC = ((hdr>>16)&0x1)^0x1;
>  
> by
>  
> hasCRC = ((hdr>>16)&0x1) == 0;
>  
> I hope compiler could optimize it to (hdr & 0x10000) == 0

OK, I'll just change it to that; thanks.


>  in line 227 replace
>  
> framesize /= samplingFreq<<isMPEG2;
>  
> by
>  
> framesize /= samplingFreq<< (isMPEG2 ? 1 : 0);

OK.


> File MediaSession.cpp
>  
> in lines 1114-116 and 1121-1123 replace
>  
> fReadSource =
>   AMRAudioRTPSource::createNew(env(), fRTPSocket, fRTPSource,
>                    fRTPPayloadFormat, 0 /*isWideband*/,
>                    fNumChannels, fOctetalign, fInterleaving,
>                    fRobustsorting, fCRC);
> // Note that fReadSource will differ from fRTPSource in this case
>   } else if (strcmp(fCodecName, "AMR-WB") == 0) { // AMR audio (wideband)
> fReadSource =
>   AMRAudioRTPSource::createNew(env(), fRTPSocket, fRTPSource,
>                    fRTPPayloadFormat, 1 /*isWideband*/,
>                    fNumChannels, fOctetalign, fInterleaving,
>                    fRobustsorting, fCRC);
>  
> by
>  
> fReadSource =
>   AMRAudioRTPSource::createNew(env(), fRTPSocket, fRTPSource,
>                    fRTPPayloadFormat, false /*isWideband*/,
>                    fNumChannels, fOctetalign != 0, fInterleaving,
>                    fRobustsorting != 0, fCRC != 0);
> // Note that fReadSource will differ from fRTPSource in this case
>   } else if (strcmp(fCodecName, "AMR-WB") == 0) { // AMR audio (wideband)
> fReadSource =
>   AMRAudioRTPSource::createNew(env(), fRTPSocket, fRTPSource,
>                    fRTPPayloadFormat, true /*isWideband*/,
>                    fNumChannels, fOctetalign != 0, fInterleaving,
>                    fRobustsorting != 0, fCRC != 0);

OK (except again that it'll be "False" and "True" rather than "false" and "true").


> File H264VideoStreamFramer.cpp
>  
> here is 4 places where the BitVector::get1bit() result is assigned to the Boolean variable. The easiest solution is to assign the get1bit() != 0 to Boolean, but more nicer is to make inline get1BitAsBoolean() member and use it.

Agreed; I'll do this.

Thanks again for the suggestions.  I'll make these changes in the next release of the software.


Ross Finlayson
Live Networks, Inc.
http://www.live555.com/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.live555.com/pipermail/live-devel/attachments/20120526/51c5e137/attachment.html>