Code review comment for lp:~rryan/mixxx/atomic-co

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Hi RJ,

Yes, I will merge it into lp:~mixxxdevelopers/mixxx/atomic-co now.

But I have not entirely reviewed and test it for merge into trunk.

I think I have not fully understand your COT changes yet so I have some code read todo.

---

the "scratch2" and "scratch2_enabled" think is definitely a subject for change in near future.
But there might be race conditions if we handle the scratch2_enabled like a property. For me it is a part of the atomic info scratch2 itself. If we introduce a int type control object we can easily reserve on value for invalid
eg -100 .. 100 + 0x7fffffff = invalid. A bool becomes 0 .. 1 + 0x7fffffff = invalid;
Or we can introduce a bitfield struct like this
struct {
   int value : 31;
   int invalid : 1;
}
Or if we don't mind using the ring implementation of ControlValueAtomic:
struct {
   double value;
   bool invalid;
}

In this way we never have an invalid flag on a valid value or vice versa.

review: Approve

« Back to merge proposal