Code review comment for lp:~mixxxdevelopers/mixxx/features_trackbeats

- Fix the commented pointer leak in TrackDAO::bindTrackToLibraryInsert()

- What was the final verdict on how adding these large blobs to the database will impact subsequent query performance? It would be good to document this since it likely won't be the last time we have to store blobs in the database. (eg. cover art)

- Probably wouldn't hurt to make those version strings in BeatFactory constants defined in the header.

- For the BeatGrid byte array, would it make sense to pack/unpack the blob as a C-style struct? You've got code like this:
+ pBuffer[0] = m_dBpm;
+ pBuffer[1] = m_dFirstBeat;
... and your schema for the format of this buffer is spread out all over the place. For example, BeatGrid::toByteArray() has a hardcoded sizeof(double) * 2 here:

+ QByteArray* pByteArray = new QByteArray((char*)pBuffer, sizeof(double) * 2);

and similarly, readByteArray() has an assert with the same hardcoded length. Using a C-style struct will probably make that code less fragile (eg. sizeof(MyBeatsStruct)).

- void BeatGrid::readByteArray(QByteArray* pByteArray) ---> Did you mean a const QByteArray* ?
:)

- BeatGrid::findNthBeat(...) might lead to unintended behaviour if dSamples is negative. (???)

- The naming of these classes is very confusing. Matrix and grid are synonyms, so I'm barring you from using those words to describe different classes. Names that may be more clear could be "ExtrapolatedBeatGrid" and "CustomBeatGrid".

Otherwise, code looks pretty good. I hope it works. :)

I'll do some testing next time I have a chance and take another pass at the code.

Thanks guys!
Albert