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

Revision history for this message
Albert Santoni (gamegod) wrote :

enginebuffer.cpp:383
        // TODO(rryan) : review this touch sensitive business with the Mixxx
        // team to see if this is what we actually want.
===
I guess we're just going to take the "see if anyone complains that we broke something" approach for that one? I don't think either of us has a controller that uses that touch wheelie thing.

enginebufferscalelinear.cpp:151
(not quoting the line here, read it and you'll understand :))
===
Is that experiment still relevant?
Can you add some more comments to the loops in scale()?

loopingcontrol.cpp
===
Can you please comment getTrigger and nextTrigger? Should we keep those qDebugs in there?

ratecontrol.cpp:102
    // FIXME: This should be dependent on sample rate/block size or something
    m_pJogFilter->setFilterLength(5);
===
Yeah, this looks really awkward to fix. All of these accumulators (Rotary.cpp?) are dependent on the block size. I wonder if we should just hack some static setBufferSize() function into Rotary...
Can you write a short paragraph describing what RateControl does at the top of ratecontrol.cpp, just so the next guys that come along know what it does and why we need it?

readaheadmanager.cpp
===
Can you please write a description of what this class does and why at the top of the file? I also think the functions in this file need comments above them (eg. what getSoonestTrigger() is doing doesn't look super obvious to the untrained eye). :)

cachingreader.cpp
===
Lines 15-20: You talk a lot about how to calculate the chunk length, but you didn't define what a "chunk" is. Can you explain it (for the next guys that come along)?
This file also wouldn't hurt from a general explanation at the top...

Lines 69-78: I think you might need a Q_ASSERT in there making sure kChunkLength is less than memory_to_use...

Line 89: Toss some comments in here explaining how the initialization of your m_chunks and m_freeChunks lists work.

Can you add comments above the functions in this file too?

cachingreader.h
===
Please comment the Hint and Chunk structs including their data fields.

review: Needs Fixing

« Back to merge proposal