> 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/
_______________________________________________
live-devel mailing list
live-devel@lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel
