dump1090
dump1090 copied to clipboard
Couple of buglets for you to think about
I've spent the weekend porting this to run under Winfows (Visual C++ 6). Whilst I realise the target is Linux, the following issues would help maintain portability.
-
The return pointer from malloc is (void*). The M$ Compiler complains if you don't explicitly cast this to the type of the receiving pointer variable. Therefore, I'd request that the return values from all malloc's are type cast.
-
The long long variable type isn't supported in the M$ compiler. I'd request that you change all long long to uint64_t, You're already using uint8_t,, uint16_t, and uint32_t, so typedefing uint64_t shouldn't be that difficult?
-
Your Modes.buffer is allocated to be Modes.data_len bytes in size. Your Modes.magnitude is allocated to be (Modes.data_len*2) bytes in size. However, the magnitude buffer doesn't need to be *2, because although the values in it are uint16_t in size, they are calculated from I/Q byte pairs in the Modes.buffer. Therefore it needs 2 bytes from the Modes.buffer in order to generate one uint16_t in the magnitude buffer. Hence the declarations can be the same size. Given the size of thse buffers, you're wasting 258Kb of memory.
Now some tweaks I;ve made.
- Your Modes.maglut buffer is 129x129 uint16_t (33282) bytes in size. You then kludge up the I/Q values in computeMagnitudeVector() by subtracting 127 and then negating the value if it is negative before indexing in into the Modes.maglut buffer to get the vector magnitude. This works ok, but, on slower processors, this is additional work that could be avoided by having a larger Modes.magnitude buffer.
So I/d suggest increasing the Modes.magnitude buffer to be 256x256 .uint16_t in size (131072 bytes) . You can then just pickup the 16 bit I/Q value, directly index into the Modes.magnitude buffer, and get the 16 bit magnitude.
The extra size of the maglut buffer is more than compensated for by memory saved in 3) above. There is additional code to initialise the maglut buffer in the first place, but this is only tun once during initialisation, so doesn't really affect performance.
-
in your maglut initialisation, you're multiplying the sqrt(i_i+q_q) by 360. This is presumably to scale the results up by 1.4 - i.e. root 2? Root 2 is actually 1.414213, so isn't 362 a better value to multiply by?
-
You are copying raw I/Q data supplied by the RTLSDR callback into Modes.data. Then in the other thread you're translating this Modes.data into the Modes.magnitude buffer. You can get rid of the Modes.data buffer all together if instead of just memcpy'ing the I/Q data from the RTLSDR callback buffer, you translate it on the fly straight into the Modes.magnitude buffer.
This will slow down the rtlsdrCallback() thread, but since access to the Modes.data buffer is protected by mutex anyway, the overall performance won't be affected since you won't need to call computeMagnitudeVector() in the main thread. Infact, since you're saving a memcpy(), overall performance will be quicker.
Cheers Malcolm
The above was me.in another incarnation I've uploaded some changes to MalcolmRobb\dump1090, but I can't work out how to get these incorporated into the main antirez fork. I don't want to be accused of stealing others' ideas, so if I've done something wrong, could you please explain how to upload/commit stuff correctly.
I'm running under Windows 7, so all this git command line stuff is a bit of a struggle to understand. Words of one sylable appreciated.