Deep-Symmetry / dysentery

Exploring ways to participate in a Pioneer Pro DJ Link network
Eclipse Public License 1.0
196 stars 24 forks source link

pitch calculations docs are a little complicated #32

Open teknopaul opened 3 years ago

teknopaul commented 3 years ago

Pitch adjust is documented in a slightly complicated way when really the adjustment is trivial

multiplier = pitch / 0x100000

BPM is simply bpm * multiplier

Some how we have ended up with

(byte[5a] x 100 + byte[5b]) x (byte[55] x 10000 + byte[56] x 100 + byte[57]) / 6400000

which took me a while to grok what was going on.

I see that to represent to a user +-5% percentage is convenient but we seem to complicate that in the docs too

for docs wouldn't

100 x multiplier - 100;

be easier to understand?

These explanations map directly to code.


double multiplier(int pitch) {
    return (double) pitch / (double) 0x100000;
}
double pitch_percentage(int pitch) {
    return 100.0 * multiplier(pitch) - 100.0;
}

thoughts?

brunchboy commented 3 years ago

Thanks again for thinking about this stuff, but I am having a bit of difficulty tracking exactly what you are talking about. I don’t spend my day to day life working on these documents, so links to the actual section would have been helpful to avoid me having to remember where a discussion takes place and hunt for it.

When you say “BPM is simply bpm * multiplier”, I can’t make sense of that. You use “bpm” twice in the same equation without them being equal to each other. Can you clarify what you mean?

The long equation is necessary to explain exactly which bytes go into the calculation and how, because the Pitch values are strange. However, if we explain the endianness ahead of time, and that the high-order byte is unused, perhaps I see what you are getting at. I still think it would be a bit unusual to have a hex number involved in a floating-point equation like that.

The reason I talk about pitch percentages, like +/- 5%, is that on the players that were in common use when I first created this document, that value was prominently displayed on the screen. And this was showing how the players computed it. I think today it is less prominent.

So I am sure you are right, this can be made easier to understand, but I would need to look at the final proposed language before I could really judge whether I felt it was an improvement or not. Jumping around with partially-formed ideas like this is too vague and confusing to me, especially when there has suddenly been a big uptick in questions on the Beat Link Trigger channel that are consuming most of my available open-source time.

brunchboy commented 3 years ago

I should also say that this is probably some of the oldest documentation that exists, and was written when we had only figured out a handful of bytes in these packets, and were struggling to assign meaning to anything. I am sure it could be made simpler and more clear now that we are more confident about what everything means. I am leaving this ticket open until I have some time to work on that, and would certainly welcome additional ideas.

teknopaul commented 3 years ago

No pressure just chucking ideas around as they crop up. I'm sure the players write code in C and, unlike java, have unsigned int types. The pitch code is very simple in C if you read the pitch field as a shifted uint32_t and bpm as shifted uint16_t. To calculate actual bpm playing it's pitch / 0x100000 x bpm. you don't need to process the bytes individually if you read the data as unsigned values and you don't have to convert each term to decimal if it's not convenient. It looks to me that all the data is either uint8_t uint16_t or uint32_t or ascii strings which are handy in C and a bit complicated in Java, (google has handy unsigned code libs for java) . Precision is handled with shift, bpm shifted a convinient decimal value, pitch a convinient hex value. Presuming you write code in C. For docs it does not matter what language it might get implemented in so it feels like explaining thing in terms of java types muddies the waters. I'm sure processing bpm and pitch as signed bytes is not what pioneer code is doing.

brunchboy commented 3 years ago

That depends on the native byte order of your processor, however, as well.

brunchboy commented 3 years ago

The idea of this documentation is to explain it without reference to any language or processor architecture whatsoever. I didn’t try to address the annoying fact that in the JVM there are no unsigned bytes, that would have led to even more complicated and ugly expressions! 😆