jamulussoftware / jamulussoftware/jamulus

Improve performance/correctness of FlipNBits in sound.cpp

Aperta
#2,993 15 commenti 0 reazioni 0 assegnatari Vedi su GitHub
bug
Lingua principale
C
Stelle
1.1k
Fork
248
Merge medio
2g 3h
PR unite (30g)
9

Descrizione

I assume that the code for Flip16,32,64 bits in sound.cpp is not efficient (or doesn't do what it should do): https://github.com/jamulussoftware/jamulus/blob/main/src/sound/asio/sound.cpp#L1168

I made some heuristic tests on what the function Flip16Bits outputs (Disclaimer: I haven't fully understood what the code exactly does) and it seems that all it does is a left shift by one. Thus, at least Flip16Bits can be simplified (and I assume it's the same for the other code).
By the function name, it could do a bitwise invert which it doesn't. Also it doesn't seem to flip the whole word around.
What confuses me most, is that the if statement doesn't flip anything. So it might well be a bug.

The following code outputs "ok", which I consider as proof that the function Flip16Bits can be implemented as leftshift by 1 only.

```c++
#include
#include
#include
#include
using namespace std;

int32_t Flip32Bits(int32_t iIn) {

uint32_t iMask = ( static_cast ( 1 ) << 31 );

int32_t iOut = 0;

for ( unsigned int i = 0; i < 32; i++ )

{

// copy current bit to correct position

iOut |= ( iIn & iMask ) ? 1 : 0;

// shift out value and mask by one bit

iOut <<= 1;

iMask >>= 1;

}

return iOut;
}

int32_t Flip32BitHist(int32_t iIn) {
return (iIn<<1);
}

int16_t Flip16Bits ( const int16_t iIn )

{

uint16_t iMask = ( 1 << 15 );

int16_t iOut = 0;

for ( unsigned int i = 0; i < 16; i++ )

{

// copy current bit to correct position

iOut |= ( iIn & iMask ) ? 1 : 0;

// shift out value and mask by one bit

iOut <<= 1;

iMask >>= 1;

}

return iOut;

}

int16_t Flip16BitsHist(const int16_t iIn) {
int16_t iOut = (iIn<< 1);
return iOut;
}

int64_t Flip64Bits ( const int64_t iIn )

{

uint64_t iMask = ( static_cast ( 1 ) << 63 );

int64_t iOut = 0;

for ( unsigned int i = 0; i < 64; i++ )

{

// copy current bit to correct position
iOut |= ( iIn & iMask ) ? 1 : 0;

// shift out value and mask by one bit

iOut <<= 1;

iMask >>= 1;

}

return iOut;

}

int64_t Flip64BitHist(int64_t iIn) {
return (iIn << 1);
}

int main(int argc, char** argv) {
for (uint16_t check = 0;check<=(uint16_t)-1;check++) {
cout << bitset<16> (check) << endl;
//cout << b1 << endl;
//cout << bitset<16>(Flip16BitsHist(check)) << endl;
//cout << bitset<16>(Flip16Bits(check)) << endl << endl;
assert(Flip16BitsHist(check) == Flip16Bits(check));
//assert(Flip32Bits(check) == Flip32BitHist(check));
//assert(Flip64Bits(check) == Flip64BitHist(check));
//cout << check;
if (check == (uint16_t)-1) {// not so elegant termination!
break;
}
}
cout << "ok";
}

```
The other functions can't be tested this way as uint64_t is just too large to get an answer, thus analysis of the code is needed.
(I might run the 32 bit over night)

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Inizia in src/sound/asio/sound.cpp intorno alla riga 1168 e ispeziona Flip16Bits, Flip32Bits e Flip64Bits nel contesto delle rispettive chiamate per determinare la trasformazione dei bit prevista. Confronta questo comportamento con gli output segnalati, quindi aggiungi una copertura mirata per tutte e tre le larghezze; il lavoro è completato quando l’implementazione corrisponde alla trasformazione prevista e i test hanno esito positivo.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
cpp
Ambito
audio-video-rtc
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Tranquilla
Chiarezza
Abbastanza chiara
Idoneità per principianti
48/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.