danomatika / ofxMidi

(maintained) Midi addon for openFrameworks
Other
262 stars 72 forks source link

MidiTimeCode toSeconds truncated double #68

Closed alex-calamar closed 5 years ago

alex-calamar commented 5 years ago

Hi Dan again, I'm back here because I'm still dealing with your addon. ;)

In our last conversations you suggested the use of the toSeconds function inside ofxMidiTimeCodeFrame and multiply it by 1000 to obtain miliseconds. And you were right, it sounded logical; but I found out that the function truncates the decimals in the casting of the miliseconds derived from the frames as you are using the framesToMs function which returns an int... So I would suggest that you should cast as well the return of framesToMs as I show here:

// ----------------------------------------------------------------------------- double ofxMidiTimecodeFrame::toSeconds() const { double time = (double)hours 3600.0; // 60.0 60.0 time += (double)minutes * 60.0; time += (double)seconds; time += (double) ofxMidiTimecode::framesToMs(frames, rate) / 1000; return time; }

Otherwise the framesToMs function could return a double instead...

What do you think? I checked it out and it's truncating it so... Let me know your thoughts.

Thanks again for your time. Alex

danomatika commented 5 years ago

Does it work with your changes?

enohp ym morf tnes

Dan Wilcox danomatika.com robotcowboy.com

On Jan 21, 2019, at 8:34 PM, Alex Ramos notifications@github.com wrote:

Hi Dan again, I'm back here because I'm still dealing with your addon. ;)

In our last conversations you suggested the use of the toSeconds function inside ofxMidiTimeCodeFrame and multiply it by 1000 to obtain miliseconds. And you were right, it sounded logical; but I found out that the function truncates the decimals in the casting of the miliseconds derived from the frames as you are using the framesToMs function which returns an int... So I would suggest that you should cast as well the return of framesToMs as I show here:

// ----------------------------------------------------------------------------- double ofxMidiTimecodeFrame::toSeconds() const { double time = (double)hours 3600.0; // 60.0 60.0 time += (double)minutes * 60.0; time += (double)seconds; time += (double) ofxMidiTimecode::framesToMs(frames, rate) / 1000; return time; }

Otherwise the framesToMs function could return a double instead...

What do you think? I checked it out and it's truncating it so... Let me know your thoughts.

Thanks again for your time. Alex

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

alex-calamar commented 5 years ago

Indeed, just with that casting to double it works much more better... Actually the "milliseconds" calculation can be done finally right!

alex-calamar commented 5 years ago

Should I make a pull-request about this? ;o)

danomatika commented 5 years ago

Yes. My laptop is away for repairs right now...