{"action":"created","issue":{"url":"https://api.github.com/repos/supercollider/sc3-plugins/issues/25","labels_url":"https://api.github.com/repos/supercollider/sc3-plugins/issues/25/labels{/name}","comments_url":"https://api.github.com/repos/supercollider/sc3-plugins/issues/25/comments","events_url":"https://api.github.com/repos/supercollider/sc3-plugins/issues/25/events","html_url":"https://github.com/supercollider/sc3-plugins/issues/25","id":50849236,"number":25,"title":"MdaPiano should round frequency to midi note conversion rather than truncate","user":{"login":"rogerallen","id":1237285,"avatar_url":"https://avatars.githubusercontent.com/u/1237285?v=3","gravatar_id":"","url":"https://api.github.com/users/rogerallen","html_url":"https://github.com/rogerallen","followers_url":"https://api.github.com/users/rogerallen/followers","following_url":"https://api.github.com/users/rogerallen/following{/other_user}","gists_url":"https://api.github.com/users/rogerallen/gists{/gist_id}","starred_url":"https://api.github.com/users/rogerallen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rogerallen/subscriptions","organizations_url":"https://api.github.com/users/rogerallen/orgs","repos_url":"https://api.github.com/users/rogerallen/repos","events_url":"https://api.github.com/users/rogerallen/events{/privacy}","received_events_url":"https://api.github.com/users/rogerallen/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2014-12-03T15:12:07Z","updated_at":"2015-07-08T16:51:45Z","closed_at":null,"body":"There is an issue with the MdaPiano synth that has been reported to the Overtone project.  I think it is an issue for this project to address.  See discussion at https://github.com/overtone/overtone/issues/264\r\n\r\nRepeating my comment from there...\r\n\r\nThe MdaPiano code here\r\nhttps://github.com/supercollider/sc3-plugins/blob/master/source/MdaUGens/MdaUGens.cpp#L119\r\ntakes the freq in and converts it immediately to an integer note with\r\nsc_cpsmidi().  Due to how sc_cpsmidi() is written, we are on the\r\nknife's edge of precision issues as people above have found out.  If\r\nthe input frequency is off by any amount towards zero, the integer\r\nnote returned is one lower than desired.\r\n\r\nIn my opinion, MdaPiano should be changed to allow for some slight\r\nerrors in the input frequency and use rounding instead of truncation\r\nafter calculating sc_cpsmidi().  Simply adding 0.5f prior to converting to an integer would do this.\r\n\r\nThe sc_cpsmidi and midicps functions are defined in SuperCollider here\r\nand you can see they involve some magic constants:\r\nhttps://github.com/supercollider/supercollider/blob/master/include/plugin_interface/SC_InlineUnaryOp.h\r\n\r\nHere is a test program to show what is happening with any slight error.\r\n\r\n```c\r\n/* lets test Supercollider midicps\r\n *\r\nclang -o cpstest cpstest.cpp\r\n */\r\n\r\n#include <stdio.h>\r\n#include <cmath>\r\n#define float32 float\r\nconst double rlog2  = 1./std::log(2.);\r\n\r\ninline float32 sc_log2(float32 x)\r\n{\r\n#if __cplusplus >= 201103L\r\n\treturn std::log2(x);\r\n#elif defined(HAVE_C99)\r\n\treturn ::log2f(std::abs(x));\r\n#else\r\n\treturn static_cast<float32>(std::log(std::abs(x)) * (float32)rlog2);\r\n#endif\r\n}\r\n\r\ninline float32 sc_midicps(float32 note)\r\n{\r\n\treturn (float32)440. * std::pow((float32)2., (note - (float32)69.) * (float32)0.083333333333);\r\n}\r\n\r\ninline float32 sc_cpsmidi(float32 freq)\r\n{\r\n\treturn sc_log2(freq * (float32)0.0022727272727) * (float32)12. + (float32)69.;\r\n}\r\n\r\nint main(int argc, char **argv) {\r\n    float32 epsilon = 0.0001;\r\n    int t = 64;\r\n    float32 f = sc_midicps((float32)t);\r\n    printf(\"convert midi to freq...\\n\");\r\n    printf(\"%d => %10.6f\\n\",t,f);\r\n    int t0 = (int)sc_cpsmidi(f-epsilon);\r\n    int t1 = (int)sc_cpsmidi(f);\r\n    int t2 = (int)sc_cpsmidi(f+epsilon);\r\n    printf(\"add a bit of noise to convert freq back to midi...\\n\");\r\n    printf(\"%10.6f => %d\\n\",f-epsilon,t0);\r\n    printf(\"%10.6f => %d\\n\",f,t1);\r\n    printf(\"%10.6f => %d\\n\",f+epsilon,t2);\r\n    printf(\"use rounding instead of truncation\\n\");\r\n    t0 = (int)(sc_cpsmidi(f-epsilon) + 0.5f);\r\n    t1 = (int)(sc_cpsmidi(f) + 0.5f);\r\n    t2 = (int)(sc_cpsmidi(f+epsilon) + 0.5f);\r\n    printf(\"%10.6f => %d\\n\",f-epsilon,t0);\r\n    printf(\"%10.6f => %d\\n\",f,t1);\r\n    printf(\"%10.6f => %d\\n\",f+epsilon,t2);\r\n}\r\n```\r\n\r\n```sh\r\n> clang -o cpstest cpstest.cpp\r\n> ./cpstest \r\nconvert midi to freq...\r\n64 => 329.627563\r\nadd a bit of noise to convert freq back to midi...\r\n329.627472 => 63  # OFF\r\n329.627563 => 64\r\n329.627655 => 64\r\nuse rounding instead of truncation\r\n329.627472 => 64\r\n329.627563 => 64\r\n329.627655 => 64\r\n```\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/supercollider/sc3-plugins/issues/comments/119656911","html_url":"https://github.com/supercollider/sc3-plugins/issues/25#issuecomment-119656911","issue_url":"https://api.github.com/repos/supercollider/sc3-plugins/issues/25","id":119656911,"user":{"login":"telephon","id":1810559,"avatar_url":"https://avatars.githubusercontent.com/u/1810559?v=3","gravatar_id":"","url":"https://api.github.com/users/telephon","html_url":"https://github.com/telephon","followers_url":"https://api.github.com/users/telephon/followers","following_url":"https://api.github.com/users/telephon/following{/other_user}","gists_url":"https://api.github.com/users/telephon/gists{/gist_id}","starred_url":"https://api.github.com/users/telephon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/telephon/subscriptions","organizations_url":"https://api.github.com/users/telephon/orgs","repos_url":"https://api.github.com/users/telephon/repos","events_url":"https://api.github.com/users/telephon/events{/privacy}","received_events_url":"https://api.github.com/users/telephon/received_events","type":"User","site_admin":false},"created_at":"2015-07-08T16:51:44Z","updated_at":"2015-07-08T16:51:44Z","body":"The standard frequency inputs in supercollider are floats and are kept as floats. So this plugin is a special case indeed, and should convert correctly internally."}}