GitHub PullRequestReviewCommentEvent at 2014-06-06 21:32:50
Repository: mixxx
Actor: daschuer
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments/13511681","id":13511681,"diff_hunk":"@@ -191,10 +187,6 @@ void EngineFilterButterworth8Band::process(const CSAMPLE* pIn,\n     for (int i=0; i < iBufferSize; i += 2) {\n         pOutput[i] = _processBandpass(m_coef, m_buf1, pIn[i]);\n         pOutput[i+1] = _processBandpass(m_coef, m_buf2, pIn[i+1]);\n-        if(pOutput[i] != pOutput[i])    //Check for NaN","path":"src/engine/enginefilterbutterworth8.cpp","position":240,"original_position":226,"commit_id":"04b28e9d4aac4bff1e58020bea104b831d302689","original_commit_id":"3c4cd7c025ee0f8e197db1c95eabd20df69c9e28","user":{"login":"daschuer","id":1777442,"avatar_url":"https://avatars.githubusercontent.com/u/1777442?","gravatar_id":"4ad11d074e0bd254af763102c6824efc","url":"https://api.github.com/users/daschuer","html_url":"https://github.com/daschuer","followers_url":"https://api.github.com/users/daschuer/followers","following_url":"https://api.github.com/users/daschuer/following{/other_user}","gists_url":"https://api.github.com/users/daschuer/gists{/gist_id}","starred_url":"https://api.github.com/users/daschuer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/daschuer/subscriptions","organizations_url":"https://api.github.com/users/daschuer/orgs","repos_url":"https://api.github.com/users/daschuer/repos","events_url":"https://api.github.com/users/daschuer/events{/privacy}","received_events_url":"https://api.github.com/users/daschuer/received_events","type":"User","site_admin":false},"body":"NaN can be produced by \r\n* division by zero \r\n* root of a negative value \r\n* invalid arc operations \r\n\r\nIf I see this right, there is nothing like that in  _processBandpass().\r\nSo if we ever see this, it is cause by a NaN value in the coefs.\r\nCan this happen? If yes, we should check the coefs for NaN, \r\n\r\nIn any case, setting the NaN value Zero is as wrong as the NaN value itself.\r\n\r\nIMHO NaN is only an special value above 1.0 we clamp anyway before passing it to the soundcard. \r\n\r\nThe check was introduced here: \r\nbea641fd3eac9f76cc24630ae920475c1c120053\r\nwhen introducing the tweak able corner frequency \r\nMaybe it was because of the Bug Nicu has just solved. \r\n\r\nConclusion: I am am fine with removing NaN check. \r\n\r\n\r\n\r\n\r\n ","created_at":"2014-06-06T21:32:50Z","updated_at":"2014-06-06T21:32:50Z","html_url":"https://github.com/mixxxdj/mixxx/pull/270#discussion_r13511681","pull_request_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270","_links":{"self":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments/13511681"},"html":{"href":"https://github.com/mixxxdj/mixxx/pull/270#discussion_r13511681"},"pull_request":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270"}}},"pull_request":{"url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270","id":16796887,"html_url":"https://github.com/mixxxdj/mixxx/pull/270","diff_url":"https://github.com/mixxxdj/mixxx/pull/270.diff","patch_url":"https://github.com/mixxxdj/mixxx/pull/270.patch","issue_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/270","number":270,"state":"open","title":"Constant crackling in EngineFilterButterworth8Band::process()","user":{"login":"badescunicu","id":2550546,"avatar_url":"https://avatars.githubusercontent.com/u/2550546?","gravatar_id":"72024a581a3aa2be2a952e475ae8e4b2","url":"https://api.github.com/users/badescunicu","html_url":"https://github.com/badescunicu","followers_url":"https://api.github.com/users/badescunicu/followers","following_url":"https://api.github.com/users/badescunicu/following{/other_user}","gists_url":"https://api.github.com/users/badescunicu/gists{/gist_id}","starred_url":"https://api.github.com/users/badescunicu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/badescunicu/subscriptions","organizations_url":"https://api.github.com/users/badescunicu/orgs","repos_url":"https://api.github.com/users/badescunicu/repos","events_url":"https://api.github.com/users/badescunicu/events{/privacy}","received_events_url":"https://api.github.com/users/badescunicu/received_events","type":"User","site_admin":false},"body":"This is a fix for https://bugs.launchpad.net/mixxx/+bug/1326001\r\n\r\nBelow you can see the performance differences.\r\n\r\n_My notebook (with RT Scheduling disabled):_\r\nWithout the patch:\r\nStat(\"_processBandpass()\",\"count=2.10739e+06,sum=1.89391e+09ns,average=898.699ns,min=461ns,max=422348ns,variance=1.01698e+06ns^2,stddev=1008.45ns\")\r\n\r\nWith the patch:\r\nStat(\"_processBandpass()\",count=2.61939e+06,sum=2.50817e+09ns,average=957.538ns,min=477ns,max=663356ns,variance=1.04328e+06ns^2,stddev=1021.41ns\")\r\n\r\n---\r\n\r\n_Asus eeePC with RT Scheduling enabled:_\r\nWithout the patch:\r\nStat(\"_processBandpass()\",\"count=3.43757e+06,sum=8.52755e+09ns,average=2480.69ns,min=1467ns,max=138215ns,variance=1.43556e+06ns^2,stddev=1198.15ns\")\r\n\r\nWith the patch:\r\nStat(\"_processBandpass()\",\"count=1.85549e+06,sum=4.71517e+09ns,average=2541.2ns,min=1536ns,max=131022ns,variance=1.88703e+06ns^2,stddev=1373.69ns\")\r\n\r\n","created_at":"2014-06-05T23:05:27Z","updated_at":"2014-06-06T21:32:50Z","closed_at":null,"merged_at":null,"merge_commit_sha":"c3efb89f9807fefba9eab6286168c2a6127e3309","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270/commits","review_comments_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270/comments","review_comment_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments/{number}","comments_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/270/comments","statuses_url":"https://api.github.com/repos/mixxxdj/mixxx/statuses/04b28e9d4aac4bff1e58020bea104b831d302689","head":{"label":"badescunicu:bug_1326001","ref":"bug_1326001","sha":"04b28e9d4aac4bff1e58020bea104b831d302689","user":{"login":"badescunicu","id":2550546,"avatar_url":"https://avatars.githubusercontent.com/u/2550546?","gravatar_id":"72024a581a3aa2be2a952e475ae8e4b2","url":"https://api.github.com/users/badescunicu","html_url":"https://github.com/badescunicu","followers_url":"https://api.github.com/users/badescunicu/followers","following_url":"https://api.github.com/users/badescunicu/following{/other_user}","gists_url":"https://api.github.com/users/badescunicu/gists{/gist_id}","starred_url":"https://api.github.com/users/badescunicu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/badescunicu/subscriptions","organizations_url":"https://api.github.com/users/badescunicu/orgs","repos_url":"https://api.github.com/users/badescunicu/repos","events_url":"https://api.github.com/users/badescunicu/events{/privacy}","received_events_url":"https://api.github.com/users/badescunicu/received_events","type":"User","site_admin":false},"repo":{"id":13149386,"name":"mixxx","full_name":"badescunicu/mixxx","owner":{"login":"badescunicu","id":2550546,"avatar_url":"https://avatars.githubusercontent.com/u/2550546?","gravatar_id":"72024a581a3aa2be2a952e475ae8e4b2","url":"https://api.github.com/users/badescunicu","html_url":"https://github.com/badescunicu","followers_url":"https://api.github.com/users/badescunicu/followers","following_url":"https://api.github.com/users/badescunicu/following{/other_user}","gists_url":"https://api.github.com/users/badescunicu/gists{/gist_id}","starred_url":"https://api.github.com/users/badescunicu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/badescunicu/subscriptions","organizations_url":"https://api.github.com/users/badescunicu/orgs","repos_url":"https://api.github.com/users/badescunicu/repos","events_url":"https://api.github.com/users/badescunicu/events{/privacy}","received_events_url":"https://api.github.com/users/badescunicu/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/badescunicu/mixxx","description":"Mixxx is Free DJ software that gives you everything you need to perform live mixes.","fork":true,"url":"https://api.github.com/repos/badescunicu/mixxx","forks_url":"https://api.github.com/repos/badescunicu/mixxx/forks","keys_url":"https://api.github.com/repos/badescunicu/mixxx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/badescunicu/mixxx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/badescunicu/mixxx/teams","hooks_url":"https://api.github.com/repos/badescunicu/mixxx/hooks","issue_events_url":"https://api.github.com/repos/badescunicu/mixxx/issues/events{/number}","events_url":"https://api.github.com/repos/badescunicu/mixxx/events","assignees_url":"https://api.github.com/repos/badescunicu/mixxx/assignees{/user}","branches_url":"https://api.github.com/repos/badescunicu/mixxx/branches{/branch}","tags_url":"https://api.github.com/repos/badescunicu/mixxx/tags","blobs_url":"https://api.github.com/repos/badescunicu/mixxx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/badescunicu/mixxx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/badescunicu/mixxx/git/refs{/sha}","trees_url":"https://api.github.com/repos/badescunicu/mixxx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/badescunicu/mixxx/statuses/{sha}","languages_url":"https://api.github.com/repos/badescunicu/mixxx/languages","stargazers_url":"https://api.github.com/repos/badescunicu/mixxx/stargazers","contributors_url":"https://api.github.com/repos/badescunicu/mixxx/contributors","subscribers_url":"https://api.github.com/repos/badescunicu/mixxx/subscribers","subscription_url":"https://api.github.com/repos/badescunicu/mixxx/subscription","commits_url":"https://api.github.com/repos/badescunicu/mixxx/commits{/sha}","git_commits_url":"https://api.github.com/repos/badescunicu/mixxx/git/commits{/sha}","comments_url":"https://api.github.com/repos/badescunicu/mixxx/comments{/number}","issue_comment_url":"https://api.github.com/repos/badescunicu/mixxx/issues/comments/{number}","contents_url":"https://api.github.com/repos/badescunicu/mixxx/contents/{+path}","compare_url":"https://api.github.com/repos/badescunicu/mixxx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/badescunicu/mixxx/merges","archive_url":"https://api.github.com/repos/badescunicu/mixxx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/badescunicu/mixxx/downloads","issues_url":"https://api.github.com/repos/badescunicu/mixxx/issues{/number}","pulls_url":"https://api.github.com/repos/badescunicu/mixxx/pulls{/number}","milestones_url":"https://api.github.com/repos/badescunicu/mixxx/milestones{/number}","notifications_url":"https://api.github.com/repos/badescunicu/mixxx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/badescunicu/mixxx/labels{/name}","releases_url":"https://api.github.com/repos/badescunicu/mixxx/releases{/id}","created_at":"2013-09-27T12:01:58Z","updated_at":"2014-06-06T17:46:00Z","pushed_at":"2014-06-06T17:46:00Z","git_url":"git://github.com/badescunicu/mixxx.git","ssh_url":"git@github.com:badescunicu/mixxx.git","clone_url":"https://github.com/badescunicu/mixxx.git","svn_url":"https://github.com/badescunicu/mixxx","homepage":"http://mixxx.org","size":220894,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"mixxxdj:master","ref":"master","sha":"8c98e6c32d5a99aa8477611943be2fcdf8faafbb","user":{"login":"mixxxdj","id":1743769,"avatar_url":"https://avatars.githubusercontent.com/u/1743769?","gravatar_id":"c81bbd7f450afcdd452e0ccf9294868f","url":"https://api.github.com/users/mixxxdj","html_url":"https://github.com/mixxxdj","followers_url":"https://api.github.com/users/mixxxdj/followers","following_url":"https://api.github.com/users/mixxxdj/following{/other_user}","gists_url":"https://api.github.com/users/mixxxdj/gists{/gist_id}","starred_url":"https://api.github.com/users/mixxxdj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mixxxdj/subscriptions","organizations_url":"https://api.github.com/users/mixxxdj/orgs","repos_url":"https://api.github.com/users/mixxxdj/repos","events_url":"https://api.github.com/users/mixxxdj/events{/privacy}","received_events_url":"https://api.github.com/users/mixxxdj/received_events","type":"Organization","site_admin":false},"repo":{"id":10126031,"name":"mixxx","full_name":"mixxxdj/mixxx","owner":{"login":"mixxxdj","id":1743769,"avatar_url":"https://avatars.githubusercontent.com/u/1743769?","gravatar_id":"c81bbd7f450afcdd452e0ccf9294868f","url":"https://api.github.com/users/mixxxdj","html_url":"https://github.com/mixxxdj","followers_url":"https://api.github.com/users/mixxxdj/followers","following_url":"https://api.github.com/users/mixxxdj/following{/other_user}","gists_url":"https://api.github.com/users/mixxxdj/gists{/gist_id}","starred_url":"https://api.github.com/users/mixxxdj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mixxxdj/subscriptions","organizations_url":"https://api.github.com/users/mixxxdj/orgs","repos_url":"https://api.github.com/users/mixxxdj/repos","events_url":"https://api.github.com/users/mixxxdj/events{/privacy}","received_events_url":"https://api.github.com/users/mixxxdj/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mixxxdj/mixxx","description":"Mixxx is Free DJ software that gives you everything you need to perform live mixes.","fork":false,"url":"https://api.github.com/repos/mixxxdj/mixxx","forks_url":"https://api.github.com/repos/mixxxdj/mixxx/forks","keys_url":"https://api.github.com/repos/mixxxdj/mixxx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mixxxdj/mixxx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mixxxdj/mixxx/teams","hooks_url":"https://api.github.com/repos/mixxxdj/mixxx/hooks","issue_events_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/events{/number}","events_url":"https://api.github.com/repos/mixxxdj/mixxx/events","assignees_url":"https://api.github.com/repos/mixxxdj/mixxx/assignees{/user}","branches_url":"https://api.github.com/repos/mixxxdj/mixxx/branches{/branch}","tags_url":"https://api.github.com/repos/mixxxdj/mixxx/tags","blobs_url":"https://api.github.com/repos/mixxxdj/mixxx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mixxxdj/mixxx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mixxxdj/mixxx/git/refs{/sha}","trees_url":"https://api.github.com/repos/mixxxdj/mixxx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mixxxdj/mixxx/statuses/{sha}","languages_url":"https://api.github.com/repos/mixxxdj/mixxx/languages","stargazers_url":"https://api.github.com/repos/mixxxdj/mixxx/stargazers","contributors_url":"https://api.github.com/repos/mixxxdj/mixxx/contributors","subscribers_url":"https://api.github.com/repos/mixxxdj/mixxx/subscribers","subscription_url":"https://api.github.com/repos/mixxxdj/mixxx/subscription","commits_url":"https://api.github.com/repos/mixxxdj/mixxx/commits{/sha}","git_commits_url":"https://api.github.com/repos/mixxxdj/mixxx/git/commits{/sha}","comments_url":"https://api.github.com/repos/mixxxdj/mixxx/comments{/number}","issue_comment_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/comments/{number}","contents_url":"https://api.github.com/repos/mixxxdj/mixxx/contents/{+path}","compare_url":"https://api.github.com/repos/mixxxdj/mixxx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mixxxdj/mixxx/merges","archive_url":"https://api.github.com/repos/mixxxdj/mixxx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mixxxdj/mixxx/downloads","issues_url":"https://api.github.com/repos/mixxxdj/mixxx/issues{/number}","pulls_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls{/number}","milestones_url":"https://api.github.com/repos/mixxxdj/mixxx/milestones{/number}","notifications_url":"https://api.github.com/repos/mixxxdj/mixxx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mixxxdj/mixxx/labels{/name}","releases_url":"https://api.github.com/repos/mixxxdj/mixxx/releases{/id}","created_at":"2013-05-17T15:14:38Z","updated_at":"2014-06-06T09:07:54Z","pushed_at":"2014-06-03T22:09:31Z","git_url":"git://github.com/mixxxdj/mixxx.git","ssh_url":"git@github.com:mixxxdj/mixxx.git","clone_url":"https://github.com/mixxxdj/mixxx.git","svn_url":"https://github.com/mixxxdj/mixxx","homepage":"http://mixxx.org","size":292088,"stargazers_count":132,"watchers_count":132,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":false,"forks_count":122,"mirror_url":null,"open_issues_count":29,"forks":122,"open_issues":29,"watchers":132,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270"},"html":{"href":"https://github.com/mixxxdj/mixxx/pull/270"},"issue":{"href":"https://api.github.com/repos/mixxxdj/mixxx/issues/270"},"comments":{"href":"https://api.github.com/repos/mixxxdj/mixxx/issues/270/comments"},"review_comments":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270/comments"},"review_comment":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/270/commits"},"statuses":{"href":"https://api.github.com/repos/mixxxdj/mixxx/statuses/04b28e9d4aac4bff1e58020bea104b831d302689"}}}}