Envelope 0 is not always the tempo envelope if there are other envelopes on the master track. ```cpp GetTrackEnvelopeByName(masterTrack, __LOCALIZE("Tempo map", "env")) ```

Off-by-one error: `GetTempoTimeSigMarker(count)` does not exist.

Could probably write the new tempo envelope only once after (or before?) processing all regions instead of once for every region.

Since this function overwrites the entire envelope state chunk later on, couldn't it just insert the new markers into it and skip everything else?

These `TempoMarker` are leaking. Could use a `WDL_PtrList_DeleteOnDestroy`, but in any case there's no point in allocating this small data structure on the heap and making a list of their pointers. This `TempoMarker` could be on the stack then copied into a `std::vector<TempoMap>` (whose data is stored in the heap anyway).

Most of these are unused. Since it's just a simple POD struct, direct member access is OK. (Also `protected` instead of `private` implies this class is meant to be inherited.) ```cpp struct TempoMarker { double m_dTimePos; int m_measurePos; double m_dBeatPos, m_dBpm; int m_timesig_num, m_timesig_denom; bool m_bLinearTempo; RawTempoMarkerData m_rawData; // why a separate struct? most of the data overlap }; ```

Why copy the list?

Since it being nullable has no purpose (would be a bug anyway), it could be a reference. ```suggestion bool IsTempoMarkerInInterval(const TempoMarker &tempoMarker, const double rangeStart, const double rangeEnd) { const double pos = tempoMarker.GetTimePosition(); return pos >= rangeStart && pos <= rangeEnd; } ```

`tempoMarker` leaks if false. (Well, it's never freed anyway even if true... as mentioned above) ```cpp else delete tempoMarker; ```

Lots of duplicate code here. Should be extracted & reused.

`quantizationSettings[10]` can overflow here! Better to use `LineParser` (the official parser from Cockos) for reading chunk lines.

These are pretty far from their use and in a wider scope. And since these are most of `TempoMarker` members duplicated, it could just be a `TempoMarker` instead.

The caller code would be completely broken if it passed a null `tempoMarkers`. It should crash instead of silently failing. In this case though it would better to just build a local list and return it: ```cpp WDL_PtrList<TempoMarker> GetAllTempoMarkers(ReaProject* _proj); ``` At the very least, an indentation level could be saved with ```cpp if (!_tempoMarkers) return; ```

A map is overkill for contiguous 0-based integer keys. A simple vector would suffice (and be more efficient, especially if pre-reserving space for `CountTempoTimeSigMarkers` items).

Why `void*`? `WDL_PtrList<TempoMarker>` would be clearer and safer by not requiring a cast on access.