{"action":"created","comment":{"url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments/31139364","id":31139364,"diff_hunk":"@@ -184,26 +178,37 @@ namespace {\n }\n \n void TrackInfoObject::setMetadata(const Mixxx::TrackMetadata& trackMetadata) {\n-    // TODO(XXX): This involves locking the mutex for every setXXX\n-    // method. We should figure out an optimization where there are private\n-    // setters that don't lock the mutex.\n-    setArtist(trackMetadata.getArtist());\n-    setTitle(trackMetadata.getTitle());\n-    setAlbum(trackMetadata.getAlbum());\n-    setAlbumArtist(trackMetadata.getAlbumArtist());\n-    setYear(trackMetadata.getYear());\n-    setGenre(trackMetadata.getGenre());\n-    setComposer(trackMetadata.getComposer());\n-    setGrouping(trackMetadata.getGrouping());\n-    setComment(trackMetadata.getComment());\n-    setTrackNumber(trackMetadata.getTrackNumber());\n-    setChannels(trackMetadata.getChannels());\n-    setSampleRate(trackMetadata.getSampleRate());\n-    setDuration(trackMetadata.getDuration());\n-    setBitrate(trackMetadata.getBitrate());\n-\n-    if (trackMetadata.isReplayGainValid()) {\n-        setReplayGain(trackMetadata.getReplayGain());\n+    {\n+        // enter locking scope\n+        QMutexLocker lock(&m_qMutex);\n+\n+        bool modified = false;\n+        modified |= modifyFieldValue(&m_sArtist, trackMetadata.getArtist());\n+        modified |= modifyFieldValue(&m_sTitle, trackMetadata.getTitle());\n+        modified |= modifyFieldValue(&m_sAlbum, trackMetadata.getAlbum());\n+        modified |= modifyFieldValue(&m_sAlbumArtist, trackMetadata.getAlbumArtist());\n+        modified |= modifyFieldValue(&m_sYear, trackMetadata.getYear());\n+        modified |= modifyFieldValue(&m_sGenre, trackMetadata.getGenre());\n+        modified |= modifyFieldValue(&m_sComposer, trackMetadata.getComposer());\n+        modified |= modifyFieldValue(&m_sGrouping, trackMetadata.getGrouping());\n+        modified |= modifyFieldValue(&m_sComment, trackMetadata.getComment());\n+        modified |= modifyFieldValue(&m_sTrackNumber, trackMetadata.getTrackNumber());\n+        modified |= modifyFieldValue(&m_iChannels, trackMetadata.getChannels());\n+        modified |= modifyFieldValue(&m_iSampleRate, trackMetadata.getSampleRate());\n+        modified |= modifyFieldValue(&m_iDuration, trackMetadata.getDuration());\n+        modified |= modifyFieldValue(&m_iBitrate, trackMetadata.getBitrate());\n+\n+        const bool replayGainUpdated =\n+                modifyFieldValue(&m_fReplayGain, trackMetadata.getReplayGain());\n+        modified |= replayGainUpdated;\n+\n+        if (modified) {\n+            setDirtyAndUnlock(&lock, true);","path":"src/trackinfoobject.cpp","position":126,"original_position":126,"commit_id":"44a9c03759329ff98152a7dbe35adc511615d7a9","original_commit_id":"44a9c03759329ff98152a7dbe35adc511615d7a9","user":{"login":"daschuer","id":1777442,"avatar_url":"https://avatars.githubusercontent.com/u/1777442?v=3","gravatar_id":"","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":"This setDirtyAndUnlock improves the locking a lot but it is a little bit confusion, because the function name hides some details and the lock is sometimes released inside the scope. \r\n\r\nWhat about extend the locker with the dirty emit management. (Not sure if it is an improvement, just an idea) \r\n```\r\n{ // scope \r\nDirtyFlagLocker dirtyFlagLocker (&m_qMutex);\r\n\r\n// ... property changes \r\n\r\ndirtyFlagLocker.setDirty(true) \r\n}\r\nif (replayGainUpdated) {\r\n...\r\n","created_at":"2015-05-27T14:34:15Z","updated_at":"2015-05-27T14:34:15Z","html_url":"https://github.com/mixxxdj/mixxx/pull/592#discussion_r31139364","pull_request_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592","_links":{"self":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments/31139364"},"html":{"href":"https://github.com/mixxxdj/mixxx/pull/592#discussion_r31139364"},"pull_request":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592"}}},"pull_request":{"url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592","id":36108705,"html_url":"https://github.com/mixxxdj/mixxx/pull/592","diff_url":"https://github.com/mixxxdj/mixxx/pull/592.diff","patch_url":"https://github.com/mixxxdj/mixxx/pull/592.patch","issue_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/592","number":592,"state":"open","locked":false,"title":"Reduce locking in TrackInfoObject + Various Fixes","user":{"login":"uklotzde","id":1474154,"avatar_url":"https://avatars.githubusercontent.com/u/1474154?v=3","gravatar_id":"","url":"https://api.github.com/users/uklotzde","html_url":"https://github.com/uklotzde","followers_url":"https://api.github.com/users/uklotzde/followers","following_url":"https://api.github.com/users/uklotzde/following{/other_user}","gists_url":"https://api.github.com/users/uklotzde/gists{/gist_id}","starred_url":"https://api.github.com/users/uklotzde/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uklotzde/subscriptions","organizations_url":"https://api.github.com/users/uklotzde/orgs","repos_url":"https://api.github.com/users/uklotzde/repos","events_url":"https://api.github.com/users/uklotzde/events{/privacy}","received_events_url":"https://api.github.com/users/uklotzde/received_events","type":"User","site_admin":false},"body":"- Avoid double locking in setDirty()\r\n- Reduce locking in set-/getMetadata()\r\n- Fix invalid locking when setting BPM/Beat Grid\r\n- Some minor improvements and fixes","created_at":"2015-05-24T21:28:02Z","updated_at":"2015-05-27T14:34:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"41e92bea1cbf1ae61eeb5bcc8e2b39af8be96439","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592/commits","review_comments_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592/comments","review_comment_url":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mixxxdj/mixxx/issues/592/comments","statuses_url":"https://api.github.com/repos/mixxxdj/mixxx/statuses/44a9c03759329ff98152a7dbe35adc511615d7a9","head":{"label":"uklotzde:TrackInfoObjectImprovements","ref":"TrackInfoObjectImprovements","sha":"44a9c03759329ff98152a7dbe35adc511615d7a9","user":{"login":"uklotzde","id":1474154,"avatar_url":"https://avatars.githubusercontent.com/u/1474154?v=3","gravatar_id":"","url":"https://api.github.com/users/uklotzde","html_url":"https://github.com/uklotzde","followers_url":"https://api.github.com/users/uklotzde/followers","following_url":"https://api.github.com/users/uklotzde/following{/other_user}","gists_url":"https://api.github.com/users/uklotzde/gists{/gist_id}","starred_url":"https://api.github.com/users/uklotzde/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uklotzde/subscriptions","organizations_url":"https://api.github.com/users/uklotzde/orgs","repos_url":"https://api.github.com/users/uklotzde/repos","events_url":"https://api.github.com/users/uklotzde/events{/privacy}","received_events_url":"https://api.github.com/users/uklotzde/received_events","type":"User","site_admin":false},"repo":{"id":11108787,"name":"mixxx","full_name":"uklotzde/mixxx","owner":{"login":"uklotzde","id":1474154,"avatar_url":"https://avatars.githubusercontent.com/u/1474154?v=3","gravatar_id":"","url":"https://api.github.com/users/uklotzde","html_url":"https://github.com/uklotzde","followers_url":"https://api.github.com/users/uklotzde/followers","following_url":"https://api.github.com/users/uklotzde/following{/other_user}","gists_url":"https://api.github.com/users/uklotzde/gists{/gist_id}","starred_url":"https://api.github.com/users/uklotzde/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uklotzde/subscriptions","organizations_url":"https://api.github.com/users/uklotzde/orgs","repos_url":"https://api.github.com/users/uklotzde/repos","events_url":"https://api.github.com/users/uklotzde/events{/privacy}","received_events_url":"https://api.github.com/users/uklotzde/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/uklotzde/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/uklotzde/mixxx","forks_url":"https://api.github.com/repos/uklotzde/mixxx/forks","keys_url":"https://api.github.com/repos/uklotzde/mixxx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/uklotzde/mixxx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/uklotzde/mixxx/teams","hooks_url":"https://api.github.com/repos/uklotzde/mixxx/hooks","issue_events_url":"https://api.github.com/repos/uklotzde/mixxx/issues/events{/number}","events_url":"https://api.github.com/repos/uklotzde/mixxx/events","assignees_url":"https://api.github.com/repos/uklotzde/mixxx/assignees{/user}","branches_url":"https://api.github.com/repos/uklotzde/mixxx/branches{/branch}","tags_url":"https://api.github.com/repos/uklotzde/mixxx/tags","blobs_url":"https://api.github.com/repos/uklotzde/mixxx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/uklotzde/mixxx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/uklotzde/mixxx/git/refs{/sha}","trees_url":"https://api.github.com/repos/uklotzde/mixxx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/uklotzde/mixxx/statuses/{sha}","languages_url":"https://api.github.com/repos/uklotzde/mixxx/languages","stargazers_url":"https://api.github.com/repos/uklotzde/mixxx/stargazers","contributors_url":"https://api.github.com/repos/uklotzde/mixxx/contributors","subscribers_url":"https://api.github.com/repos/uklotzde/mixxx/subscribers","subscription_url":"https://api.github.com/repos/uklotzde/mixxx/subscription","commits_url":"https://api.github.com/repos/uklotzde/mixxx/commits{/sha}","git_commits_url":"https://api.github.com/repos/uklotzde/mixxx/git/commits{/sha}","comments_url":"https://api.github.com/repos/uklotzde/mixxx/comments{/number}","issue_comment_url":"https://api.github.com/repos/uklotzde/mixxx/issues/comments{/number}","contents_url":"https://api.github.com/repos/uklotzde/mixxx/contents/{+path}","compare_url":"https://api.github.com/repos/uklotzde/mixxx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/uklotzde/mixxx/merges","archive_url":"https://api.github.com/repos/uklotzde/mixxx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/uklotzde/mixxx/downloads","issues_url":"https://api.github.com/repos/uklotzde/mixxx/issues{/number}","pulls_url":"https://api.github.com/repos/uklotzde/mixxx/pulls{/number}","milestones_url":"https://api.github.com/repos/uklotzde/mixxx/milestones{/number}","notifications_url":"https://api.github.com/repos/uklotzde/mixxx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/uklotzde/mixxx/labels{/name}","releases_url":"https://api.github.com/repos/uklotzde/mixxx/releases{/id}","created_at":"2013-07-01T22:22:18Z","updated_at":"2015-04-15T22:43:12Z","pushed_at":"2015-05-25T11:51:23Z","git_url":"git://github.com/uklotzde/mixxx.git","ssh_url":"git@github.com:uklotzde/mixxx.git","clone_url":"https://github.com/uklotzde/mixxx.git","svn_url":"https://github.com/uklotzde/mixxx","homepage":"http://mixxx.org","size":247252,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":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":"b3275637200dc7e208d533d3cd005d1370dc9369","user":{"login":"mixxxdj","id":1743769,"avatar_url":"https://avatars.githubusercontent.com/u/1743769?v=3","gravatar_id":"","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?v=3","gravatar_id":"","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":"2015-05-27T06:50:59Z","pushed_at":"2015-05-27T13:06:55Z","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":325239,"stargazers_count":292,"watchers_count":292,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":211,"mirror_url":null,"open_issues_count":40,"forks":211,"open_issues":40,"watchers":292,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592"},"html":{"href":"https://github.com/mixxxdj/mixxx/pull/592"},"issue":{"href":"https://api.github.com/repos/mixxxdj/mixxx/issues/592"},"comments":{"href":"https://api.github.com/repos/mixxxdj/mixxx/issues/592/comments"},"review_comments":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592/comments"},"review_comment":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mixxxdj/mixxx/pulls/592/commits"},"statuses":{"href":"https://api.github.com/repos/mixxxdj/mixxx/statuses/44a9c03759329ff98152a7dbe35adc511615d7a9"}}}}