[tim-janik/beast] Most Bus properties ported to C++ (#78)

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list

I managed to port all Bus properties that have a simple type (non-object, non-sequence) to C++. There are two things worth mentioning here.

(1) in bseapi.idl, we have to use non-computed constants, so there is no elegant way of expressing that the maximum is +24dB - however I think we can live with precomputed factors here

for implementing volume_clamp(), it would be nice if we wouldn't have to duplicate the magic numbers. I originally thought the best way to achieve this would be to be able to access the defaults from the parameter, somewhat like

left_volume = CLAMP (val, p_left_volume::min(), p_left_volume::max())

However, while implementing the code, I've got a new idea: we could simply offer a way to access the bseapi.idl constants, like

Const BUS_VOLUME_MIN = 1.58489319246111e-05; /* -96dB */

should define a C++ constant, also, and this could be used to implement volume_clamp().

(2) I think C++ properties are commonly referred to as "left_volume" (underscore instead of minus) - I however found the introspected paramspecs in the ui code contain minus, so I use a hacky translation for finding the pspec - not sure if that is the right thing to do here.


You can view, comment on, or merge this pull request online at:

  https://github.com/tim-janik/beast/pull/78

Commit Summary

  • BSE: bseitem: fix undo resolve crash
  • BSE: Bus::mute: port property to C++
  • BEAST-GTK: bstbuseditor: support both, aida and old style properties
  • BSE: Bus::solo: port property to C++
  • BSE: Bus::sync: port property to C++
  • BSE: Bus::left_volume: port property to C++
  • BEAST-GTK: bstbuseditor: use left_volume C++ property
  • BSE: Bus::right_volume: port property to C++
  • BEAST-GTK: bstbuseditor: use right_volume C++ property
  • BSE: Bus::master_output: port property to C++

File Changes

Patch Links:


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

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"Most Bus properties ported to C++ (#78)"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78", "url": "https://github.com/tim-janik/beast/pull/78", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Most Bus properties ported to C++ (#78)", "sections": [ { "text": "", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ ] }, { "title": "Commit Summary", "facts": [ { "name": "308aab9", "value": "BSE: bseitem: fix undo resolve crash" }, { "name": "ef875f9", "value": "BSE: Bus::mute: port property to C++" }, { "name": "c7146c9", "value": "BEAST-GTK: bstbuseditor: support both, aida and old style properties" }, { "name": "5e09d9f", "value": "BSE: Bus::solo: port property to C++" }, { "name": "80146d7", "value": "BSE: Bus::sync: port property to C++" }, { "name": "2624084", "value": "BSE: Bus::left_volume: port property to C++" }, { "name": "c4a5f95", "value": "BEAST-GTK: bstbuseditor: use left_volume C++ property" }, { "name": "73408a5", "value": "BSE: Bus::right_volume: port property to C++" }, { "name": "c3fb0d4", "value": "BEAST-GTK: bstbuseditor: use right_volume C++ property" }, { "name": "36ae320", "value": "BSE: Bus::master_output: port property to C++" } ] }, { "title": "File Changes", "facts": [ { "name": "Modified", "value": "[beast-gtk/bstbuseditor.cc](https://github.com/tim-janik/beast/pull/78/files#diff-0) (37 changes)" }, { "name": "Modified", "value": "[bse/bseapi.idl](https://github.com/tim-janik/beast/pull/78/files#diff-1) (23 changes)" }, { "name": "Modified", "value": "[bse/bsebus.cc](https://github.com/tim-janik/beast/pull/78/files#diff-2) (363 changes)" }, { "name": "Modified", "value": "[bse/bsebus.hh](https://github.com/tim-janik/beast/pull/78/files#diff-3) (22 changes)" }, { "name": "Modified", "value": "[bse/bseitem.cc](https://github.com/tim-janik/beast/pull/78/files#diff-4) (2 changes)" } ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 78,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 78\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78.patch" } ], "@type": "OpenUri", "name": "View patch" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78.diff" } ], "@type": "OpenUri", "name": "View diff" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list

Thanks Stefan, great to see steady progress here.

But note that travis-ci shows, that your changes broke the syndrum audio test in all build variants, please take a look at that.

About volume clamping, taking a look at other DAWs suggests that this should be done at the UI and doesn't need to be based on consts from the IDL at all.
Also the property descriptios read "Volume adjustment in decibel of left/right bus channel" and that's what we should make them. I.e. we should pass actual dB values, e.g. 96 as property value through the IDL layer. (*1)

About GParamSpec containing minus, that's because GLib enforces '-' instead of '_' in the pspec names, so we're forced to work around that in other places as well. Please take a look at name_to_identifier in bstparam.cc, seems like it's time to move that to bstutils.hh to avoid gratituous duplicatoin everywhere.

About porting object / sequence properties, I'm actually not sure what's missing there atm. If you want to make an attempt at porting such a property and nag me about missing bits in the IDL layer or similar, that'd be ok as well.

+      for (auto& c : pname)

pointers and references are written with *& attached to the variable, i.e. write this as "auto &c"

+ Const BUS_VOLUME_MIN = 1.58489319246111e-05; /* -96dB */

if at all, this should be using '96' as Const value, and the const be defined in terms of dB. but in other DAWs, the dB range displayed is a function of the UI, e.g. the volume meter of a track display my use different limits than for a related bus channel volume meter.

+        self->right_volume = self->left_volume = center_volume (self->right_volume, self->left_volume);

While you're at it, please make that a two liner.
Modern code analysis tools tend to choke on using an assignment as rvalue, and its still very readable if it's just:

    self->left_volume = center_volume (self->right_volume, self->left_volume);
    self->right_volume = self->left_volume;


+      return (self == master);

Please remove extraneous parenthesis like these, they could hide an assignment.

+      auto parent = BSE_ITEM (self)->parent;

Please don't use 'auto' here, but BseItem*, like you did in other places. The reason here is that the variable is the result of a hidden C-style cast (BSE_ITEM()) and used as input for another C-style cast (BSE_SONG), so the actual type used here is completely unclear to the reader and potentially hiding a type bug.

On a side note, it took a while, but I figured again the point of saved_sync. It's clearly trying to preserve the 'sync' state around loading a BSE file (termed 'restore' in the API), and sets that initially to FALSE to allow BSE files to restore different left and right volumes, and syncs thouse only if "sync" is also set or after the restore phase is completed. That heavily depends on the order in which the properties are defined in the BSE file which is somewhat fragile... (*2)

I get the feeling that the old API is a bit too low level with using synced volume factors instead of actual dB values. (*1) and (*2) are both indications that we should probably move towards something like:

  • Factors used internally: left_volume, right_volume - though a bit of compat code will be needed to still load these values from old BSE files; syncing is implemented here (as before)
  • New API: left_volume_db, right_volume_db - values passed in actual dB, these values can still be different when read even if sync is enabled.
  • So essentially, sync, left_volume_db, right_volume_db can all be read/set independently of each other and the internal bus logic adjusts left_volume, right_volume accordingly.


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

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@tim-janik in #78: Thanks Stefan, great to see steady progress here.\r\n\r\nBut note that travis-ci shows, that your changes broke the syndrum audio test in all build variants, please take a look at that.\r\n\r\nAbout volume clamping, taking a look at other DAWs suggests that this should be done at the UI and doesn't need to be based on consts from the IDL at all.\r\nAlso the property descriptios read \"Volume adjustment in decibel of left/right bus channel\" and that's what we should make them. I.e. we should pass actual dB values, e.g. 96 as property value through the IDL layer. (*1)\r\n\r\nAbout GParamSpec containing minus, that's because GLib enforces '-' instead of '_' in the pspec names, so we're forced to work around that in other places as well. Please take a look at name_to_identifier in bstparam.cc, seems like it's time to move that to bstutils.hh to avoid gratituous duplicatoin everywhere.\r\n\r\nAbout porting object / sequence properties, I'm actually not sure what's missing there atm. If you want to make an attempt at porting such a property and nag me about missing bits in the IDL layer or similar, that'd be ok as well.\r\n\r\n\t+ for (auto\u0026 c : pname)\r\n\r\npointers and references are written with *\u0026 attached to the variable, i.e. write this as \"auto \u0026c\"\r\n\r\n\t+ Const BUS_VOLUME_MIN = 1.58489319246111e-05; /* -96dB */\r\n\r\nif at all, this should be using '96' as Const value, and the const be defined in terms of dB. but in other DAWs, the dB range displayed is a function of the UI, e.g. the volume meter of a track display my use different limits than for a related bus channel volume meter.\r\n\r\n\t+ self-\u003eright_volume = self-\u003eleft_volume = center_volume (self-\u003eright_volume, self-\u003eleft_volume);\r\n\r\nWhile you're at it, please make that a two liner.\r\nModern code analysis tools tend to choke on using an assignment as rvalue, and its still very readable if it's just:\r\n\r\n self-\u003eleft_volume = center_volume (self-\u003eright_volume, self-\u003eleft_volume);\r\n self-\u003eright_volume = self-\u003eleft_volume;\r\n\r\n\r\n\t+ return (self == master);\r\n\r\nPlease remove extraneous parenthesis like these, they could hide an assignment.\r\n \r\n\t+ auto parent = BSE_ITEM (self)-\u003eparent;\r\n\r\nPlease don't use 'auto' here, but BseItem*, like you did in other places. The reason here is that the variable is the result of a hidden C-style cast (BSE_ITEM()) and used as input for another C-style cast (BSE_SONG), so the actual type used here is completely unclear to the reader and potentially hiding a type bug.\r\n\r\nOn a side note, it took a while, but I figured again the point of saved_sync. It's clearly trying to preserve the 'sync' state around loading a BSE file (termed 'restore' in the API), and sets that initially to FALSE to allow BSE files to restore different left and right volumes, and syncs thouse only if \"sync\" is also set or after the restore phase is completed. That heavily depends on the order in which the properties are defined in the BSE file which is somewhat fragile... (*2)\r\n\r\nI get the feeling that the old API is a bit too low level with using synced volume factors instead of actual dB values. (*1) and (*2) are both indications that we should probably move towards something like:\r\n\r\n* Factors used internally: left_volume, right_volume - though a bit of compat code will be needed to still load these values from old BSE files; syncing is implemented here (as before)\r\n* New API: left_volume_db, right_volume_db - values passed in actual dB, these values can still be different when read even if sync is enabled.\r\n* So essentially, sync, left_volume_db, right_volume_db can all be read/set independently of each other and the internal bus logic adjusts left_volume, right_volume accordingly."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78#issuecomment-419684838"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78#issuecomment-419684838", "url": "https://github.com/tim-janik/beast/pull/78#issuecomment-419684838", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)", "sections": [ { "text": "", "activityTitle": "**Tim Janik**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@tim-janik", "facts": [ ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 78,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 78\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78#issuecomment-419684838" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

New API: left_volume_db, right_volume_db - values passed in actual dB, these values can still be different when read even if sync is enabled.

Not commenting ot the other issues here, but I think this is also suboptimal. If I have to touch this code at all, and write compat code, I believe the proper way to do it would be

  • volume_db (-96 to 24)
  • panning (-100% to 100%)

What I usually want to do as composer is simply move the bus more to the left or right, without affecting overall volume. To implement this, I think the right way to do it is to make the signal louder if it is not being played on both channels but just on one. So

  • volume_db = 0, panning = 0 -> left_volume = 1, right_volume = 1
  • volume_db = 0, panning = -100 -> left_volume = 1.41, right_volume = 0
  • volume_db = 0, panning = 100 -> left_volume = 0, right_volume = 1.41

At least using (sqrt(2) ~= 3dB) is what I believe is the correct value here, I'd have to double-check that before implementing it. Also sync is no longer needed then, and should be removed.


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

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld in #78: \u003e New API: left_volume_db, right_volume_db - values passed in actual dB, these values can still be different when read even if sync is enabled.\r\n\r\nNot commenting ot the other issues here, but I think this is also suboptimal. If I have to touch this code at all, and write compat code, I believe the proper way to do it would be\r\n- volume_db (-96 to 24)\r\n- panning (-100% to 100%)\r\n\r\nWhat I usually want to do as composer is simply move the bus more to the left or right, without affecting overall volume. To implement this, I think the right way to do it is to make the signal louder if it is not being played on both channels but just on one. So\r\n\r\n- volume_db = 0, panning = 0 -\u003e left_volume = 1, right_volume = 1\r\n- volume_db = 0, panning = -100 -\u003e left_volume = 1.41, right_volume = 0\r\n- volume_db = 0, panning = 100 -\u003e left_volume = 0, right_volume = 1.41\r\n\r\nAt least using (sqrt(2) ~= 3dB) is what I believe is the correct value here, I'd have to double-check that before implementing it. Also sync is no longer needed then, and should be removed."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78#issuecomment-419715703"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78#issuecomment-419715703", "url": "https://github.com/tim-janik/beast/pull/78#issuecomment-419715703", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)", "sections": [ { "text": "", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 78,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 78\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78#issuecomment-419715703" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • bb74147 SFI: provide scanner double/int64 parsing code


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/36ae3204695e50eadc76d6f2f2f9ea23783bd7f0..bb74147fa4377c903048a072987c3a790baeabfb"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/36ae3204695e50eadc76d6f2f2f9ea23783bd7f0..bb74147fa4377c903048a072987c3a790baeabfb", "url": "https://github.com/tim-janik/beast/pull/78/files/36ae3204695e50eadc76d6f2f2f9ea23783bd7f0..bb74147fa4377c903048a072987c3a790baeabfb", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "bb74147", "value": "SFI: provide scanner double/int64 parsing code" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/36ae3204695e50eadc76d6f2f2f9ea23783bd7f0..bb74147fa4377c903048a072987c3a790baeabfb" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • 4084bb6 BSE: fix C++ property loading (float64)


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/bb74147fa4377c903048a072987c3a790baeabfb..4084bb6c5a48259d8c914260ff97b0d768570e9c"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/bb74147fa4377c903048a072987c3a790baeabfb..4084bb6c5a48259d8c914260ff97b0d768570e9c", "url": "https://github.com/tim-janik/beast/pull/78/files/bb74147fa4377c903048a072987c3a790baeabfb..4084bb6c5a48259d8c914260ff97b0d768570e9c", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "4084bb6", "value": "BSE: fix C++ property loading (float64)" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/bb74147fa4377c903048a072987c3a790baeabfb..4084bb6c5a48259d8c914260ff97b0d768570e9c" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

On 09.09.2018 15:21, Stefan Westerfeld via beast wrote:

  • volume_db = 0, panning = 0 -> left_volume = 1, right_volume = 1
  • volume_db = 0, panning = -100 -> left_volume = 1.41, right_volume = 0
  • volume_db = 0, panning = 100 -> left_volume = 0, right_volume = 1.41
    What you write here makes sense for libbse. How's the beast-gtk UI going to look
    for this though?

Instead of two volume sliders we only need one. The two meters could be put next to each other. So this would mean left-to-right volume slider (only one slider), scale numbers (-96..+12), left meter, right meter.

Ideal for pan would be a round knob, but we could for now use a slider above the other widgets left to right (where left is -100%, right is +100%).

At least using (sqrt(2) ~= 3dB) is what I believe is the correct value here, I'd
have to double-check that before implementing it.
The monitoring code for our level meters uses dB SPL, which is 20log10(avg) and
corresponds to the power ratio of the signal, so the value to use here would be
more around 2. See also: https://en.wikipedia.org/wiki/Decibel#Acoustics_2

Lets discuss details not now, but when I've written the code.

Also sync is no longer needed
then, and should be removed.
The sync property is saved im BSE files, so we need compat loading code at
least. If sync is to be removed from the UI depends on how that's designed and
implemented to cope with panning. Thus my question above.

Right, it should be ignored during read (real compat code is just needed to translate the stored left and right factors => volume/pan). Sync == true files will have identical values for left and right volume, so sync is not needed to load the file, and sync == false files will have their panning information in left and right volume, so this needs to be translated.

There need no longer be a widget for sync either, as using the volume slider affects both, and using pan == 0 is the exact behaviour of our previous sync == true mode.


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

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld in #78: \u003e On 09.09.2018 15:21, Stefan Westerfeld via beast wrote:\r\n\u003e * volume_db = 0, panning = 0 -\u003e left_volume = 1, right_volume = 1\r\n\u003e * volume_db = 0, panning = -100 -\u003e left_volume = 1.41, right_volume = 0\r\n\u003e * volume_db = 0, panning = 100 -\u003e left_volume = 0, right_volume = 1.41\r\n\u003e What you write here makes sense for libbse. How's the beast-gtk UI going to look\r\n\u003e for this though?\r\n\r\nInstead of two volume sliders we only need one. The two meters could be put next to each other. So this would mean left-to-right volume slider (only one slider), scale numbers (-96..+12), left meter, right meter.\r\n\r\nIdeal for pan would be a round knob, but we could for now use a slider above the other widgets left to right (where left is -100%, right is +100%).\r\n\r\n\u003e At least using (sqrt(2) ~= 3dB) is what I believe is the correct value here, I'd\r\n\u003e have to double-check that before implementing it.\r\n\u003e The monitoring code for our level meters uses dB SPL, which is 20log10(avg) and\r\n\u003e corresponds to the power ratio of the signal, so the value to use here would be\r\n\u003e more around 2. See also: https://en.wikipedia.org/wiki/Decibel#Acoustics_2\r\n\r\nLets discuss details not now, but when I've written the code.\r\n\r\n\u003e Also sync is no longer needed\r\n\u003e then, and should be removed.\r\n\u003e The sync property is saved im BSE files, so we need compat loading code at\r\n\u003e least. If sync is to be removed from the UI depends on how that's designed and\r\n\u003e implemented to cope with panning. Thus my question above.\r\n\r\nRight, it should be ignored during read (real compat code is just needed to translate the stored left and right factors =\u003e volume/pan). Sync == true files will have identical values for left and right volume, so sync is not needed to load the file, and sync == false files will have their panning information in left and right volume, so this needs to be translated.\r\n\r\nThere need no longer be a widget for sync either, as using the volume slider affects both, and using pan == 0 is the exact behaviour of our previous sync == true mode."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78#issuecomment-420058077"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78#issuecomment-420058077", "url": "https://github.com/tim-janik/beast/pull/78#issuecomment-420058077", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)", "sections": [ { "text": "", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 78,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 78\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78#issuecomment-420058077" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • 446a37c BEAST-GTK: move common name -> identifier conversion to bstutils


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/4084bb6c5a48259d8c914260ff97b0d768570e9c..446a37cadba865069d4fa45f12313e5e082489bf"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/4084bb6c5a48259d8c914260ff97b0d768570e9c..446a37cadba865069d4fa45f12313e5e082489bf", "url": "https://github.com/tim-janik/beast/pull/78/files/4084bb6c5a48259d8c914260ff97b0d768570e9c..446a37cadba865069d4fa45f12313e5e082489bf", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "446a37c", "value": "BEAST-GTK: move common name -\u003e identifier conversion to bstutils" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/4084bb6c5a48259d8c914260ff97b0d768570e9c..446a37cadba865069d4fa45f12313e5e082489bf" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • 36e9df0 BSE: bsebus: avoid chained assignment


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/446a37cadba865069d4fa45f12313e5e082489bf..36e9df0a08702d8be4c88ea10e50928cafb8a772"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/446a37cadba865069d4fa45f12313e5e082489bf..36e9df0a08702d8be4c88ea10e50928cafb8a772", "url": "https://github.com/tim-janik/beast/pull/78/files/446a37cadba865069d4fa45f12313e5e082489bf..36e9df0a08702d8be4c88ea10e50928cafb8a772", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "36e9df0", "value": "BSE: bsebus: avoid chained assignment" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/446a37cadba865069d4fa45f12313e5e082489bf..36e9df0a08702d8be4c88ea10e50928cafb8a772" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • e850a05 BSE: bsebus: minor cosmetic changes


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/36e9df0a08702d8be4c88ea10e50928cafb8a772..e850a0591eb7a301b42426653b15fe9c55e26a67"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/36e9df0a08702d8be4c88ea10e50928cafb8a772..e850a0591eb7a301b42426653b15fe9c55e26a67", "url": "https://github.com/tim-janik/beast/pull/78/files/36e9df0a08702d8be4c88ea10e50928cafb8a772..e850a0591eb7a301b42426653b15fe9c55e26a67", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "e850a05", "value": "BSE: bsebus: minor cosmetic changes" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/36e9df0a08702d8be4c88ea10e50928cafb8a772..e850a0591eb7a301b42426653b15fe9c55e26a67" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

On 09.09.2018 15:21, Stefan Westerfeld via beast wrote:
Instead of two volume sliders we only need one. The two meters could be put next to each other. So this would mean left-to-right volume slider (only one slider), scale numbers (-96..+12), left meter, right meter.

Do you intend to work on this as part of this pull request? For beast-gtk and/or ebeast?
I think that's beyond the scope of a property port, and I wonder if we're getting too side tracked here, especially when we're pumping resources into more beast-gtk UI development...

Ideal for pan would be a round knob, but we could for now use a slider above the other widgets left to right (where left is -100%, right is +100%).

At least using (sqrt(2) ~= 3dB) is what I believe is the correct value here, I'd
have to double-check that before implementing it.
The monitoring code for our level meters uses dB SPL, which is 20log10(avg) and
corresponds to the power ratio of the signal, so the value to use here would be
more around 2. See also: https://en.wikipedia.org/wiki/Decibel#Acoustics_2

Lets discuss details not now, but when I've written the code.

When you convert to/from dB, simply use db SPL everywhere (described in the above WP page) then there's no need for further discussions.

Also sync is no longer needed
then, and should be removed.
The sync property is saved im BSE files, so we need compat loading code at
least. If sync is to be removed from the UI depends on how that's designed and
implemented to cope with panning. Thus my question above.

Right, it should be ignored during read (real compat code is just needed to translate the stored left and right factors => volume/pan). Sync == true files will have identical values for left and right volume, so sync is not needed to load the file, and sync == false files will have their panning information in left and right volume, so this needs to be translated.

We cannot make any assumptions about the values stored in BSE files, about the order or which values are present or not present, e.g. a user can add sync=1 to a bse file that has left=0.5 and right=1. That's why the code currently ensures left + right are synced after parsing if needed.

FWIW, that's one of the reasons things will get easier when we move to XML based BSE files, we don't have to react to property values coming in in random order, but can query left,right,sync from the XML nodes and attributes and configure the object accordingly.


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

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@tim-janik in #78: \u003e \u003e On 09.09.2018 15:21, Stefan Westerfeld via beast wrote:\r\n\u003e Instead of two volume sliders we only need one. The two meters could be put next to each other. So this would mean left-to-right volume slider (only one slider), scale numbers (-96..+12), left meter, right meter.\r\n\r\nDo you intend to work on this as part of this pull request? For beast-gtk and/or ebeast?\r\nI think that's beyond the scope of a property port, and I wonder if we're getting too side tracked here, especially when we're pumping resources into more beast-gtk UI development...\r\n\r\n\u003e Ideal for pan would be a round knob, but we could for now use a slider above the other widgets left to right (where left is -100%, right is +100%).\r\n\u003e \r\n\u003e \u003e At least using (sqrt(2) ~= 3dB) is what I believe is the correct value here, I'd\r\n\u003e \u003e have to double-check that before implementing it.\r\n\u003e \u003e The monitoring code for our level meters uses dB SPL, which is 20log10(avg) and\r\n\u003e \u003e corresponds to the power ratio of the signal, so the value to use here would be\r\n\u003e \u003e more around 2. See also: https://en.wikipedia.org/wiki/Decibel#Acoustics_2\r\n\u003e \r\n\u003e Lets discuss details not now, but when I've written the code.\r\n\r\nWhen you convert to/from dB, simply use db SPL everywhere (described in the above WP page) then there's no need for further discussions.\r\n\r\n\u003e \u003e Also sync is no longer needed\r\n\u003e \u003e then, and should be removed.\r\n\u003e \u003e The sync property is saved im BSE files, so we need compat loading code at\r\n\u003e \u003e least. If sync is to be removed from the UI depends on how that's designed and\r\n\u003e \u003e implemented to cope with panning. Thus my question above.\r\n\u003e \r\n\u003e Right, it should be ignored during read (real compat code is just needed to translate the stored left and right factors =\u003e volume/pan). Sync == true files will have identical values for left and right volume, so sync is not needed to load the file, and sync == false files will have their panning information in left and right volume, so this needs to be translated.\r\n\r\nWe cannot make any assumptions about the values stored in BSE files, about the order or which values are present or not present, e.g. a user can add sync=1 to a bse file that has left=0.5 and right=1. That's why the code currently ensures left + right are synced after parsing if needed.\r\n\r\nFWIW, that's one of the reasons things will get easier when we move to XML based BSE files, we don't have to react to property values coming in in random order, but can query left,right,sync from the XML nodes and attributes and configure the object accordingly."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78#issuecomment-421014075"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78#issuecomment-421014075", "url": "https://github.com/tim-janik/beast/pull/78#issuecomment-421014075", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)", "sections": [ { "text": "", "activityTitle": "**Tim Janik**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@tim-janik", "facts": [ ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 78,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 78\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78#issuecomment-421014075" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 4 commits.

  • 752ee2a BSE: implement Bus::volume_db and Bus::pan properties
  • ea333df BSE: remove Bus::sync property, no longer required due to panning support
  • 39a9064 BEAST-GTK: bstbuseditor: edit volume_db/pan instead of left/right volume
  • 94028c4 RES: add space for editing pan in each mixer bus


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 4 commits in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/e850a0591eb7a301b42426653b15fe9c55e26a67..94028c4b1fc484947a7365c3aa4c5c8130b2a655"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/e850a0591eb7a301b42426653b15fe9c55e26a67..94028c4b1fc484947a7365c3aa4c5c8130b2a655", "url": "https://github.com/tim-janik/beast/pull/78/files/e850a0591eb7a301b42426653b15fe9c55e26a67..94028c4b1fc484947a7365c3aa4c5c8130b2a655", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 4 commits in #78", "sections": [ { "text": "4 new commits pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "752ee2a", "value": "BSE: implement Bus::volume_db and Bus::pan properties" }, { "name": "ea333df", "value": "BSE: remove Bus::sync property, no longer required due to panning support" }, { "name": "39a9064", "value": "BEAST-GTK: bstbuseditor: edit volume_db/pan instead of left/right volume" }, { "name": "94028c4", "value": "RES: add space for editing pan in each mixer bus" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/e850a0591eb7a301b42426653b15fe9c55e26a67..94028c4b1fc484947a7365c3aa4c5c8130b2a655" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • ee9a2df BSE: allow disabling undo for Bus::master_output property setter


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/94028c4b1fc484947a7365c3aa4c5c8130b2a655..ee9a2dfc61e9387221fb3a5a24036f116ce75c0e"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/94028c4b1fc484947a7365c3aa4c5c8130b2a655..ee9a2dfc61e9387221fb3a5a24036f116ce75c0e", "url": "https://github.com/tim-janik/beast/pull/78/files/94028c4b1fc484947a7365c3aa4c5c8130b2a655..ee9a2dfc61e9387221fb3a5a24036f116ce75c0e", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "ee9a2df", "value": "BSE: allow disabling undo for Bus::master_output property setter" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/94028c4b1fc484947a7365c3aa4c5c8130b2a655..ee9a2dfc61e9387221fb3a5a24036f116ce75c0e" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • 1fab351 BSE: Song: port remaining Bus::master-output set calls


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/ee9a2dfc61e9387221fb3a5a24036f116ce75c0e..1fab3519857af77ccaa9b40e59eae94077a93edb"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/ee9a2dfc61e9387221fb3a5a24036f116ce75c0e..1fab3519857af77ccaa9b40e59eae94077a93edb", "url": "https://github.com/tim-janik/beast/pull/78/files/ee9a2dfc61e9387221fb3a5a24036f116ce75c0e..1fab3519857af77ccaa9b40e59eae94077a93edb", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "1fab351", "value": "BSE: Song: port remaining Bus::master-output set calls" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/ee9a2dfc61e9387221fb3a5a24036f116ce75c0e..1fab3519857af77ccaa9b40e59eae94077a93edb" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • 5f86778 BSE: bsebus: add a few extra checks to volume/pan conversions


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #78"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78/files/1fab3519857af77ccaa9b40e59eae94077a93edb..5f86778db247d22161fd7c6aa10ea5c4c2361736"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78/files/1fab3519857af77ccaa9b40e59eae94077a93edb..5f86778db247d22161fd7c6aa10ea5c4c2361736", "url": "https://github.com/tim-janik/beast/pull/78/files/1fab3519857af77ccaa9b40e59eae94077a93edb..5f86778db247d22161fd7c6aa10ea5c4c2361736", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "@swesterfeld pushed 1 commit in #78", "sections": [ { "text": "1 new commit pushed to tim-janik/beast #78:", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ { "name": "5f86778", "value": "BSE: bsebus: add a few extra checks to volume/pan conversions" } ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78/files/1fab3519857af77ccaa9b40e59eae94077a93edb..5f86778db247d22161fd7c6aa10ea5c4c2361736" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

Ok, so let me comment on anything that doesn't have to do with the new volume_db/pan properties first.

Thanks Stefan, great to see steady progress here.

But note that travis-ci shows, that your changes broke the syndrum audio test in all build variants, please take a look at that.

Right. This is a really nasty problem, which is caused by loading/reading code for float64 properties in aida. It turns out that even simple floating point numbers like "1.03" are written out correctly, but restore breaks in bsestorage.cc, because the scanner_parse_paren_rest does return this as "1. 3" and this is restored as a value of 1.0. I've fixed this by using the existing sfi_scanner_parse_real_num function to parse float64 values (see also commit comments). This seems to work, tests pass again. Not sure if this is what you think we should do to fix the problem, though.

About volume clamping, taking a look at other DAWs suggests that this should be done at the UI and doesn't need to be based on consts from the IDL at all.
Also the property descriptios read "Volume adjustment in decibel of left/right bus channel" and that's what we should make them. I.e. we should pass actual dB values, e.g. 96 as property value through the IDL layer. (*1)

Ok. You'll notice that the magic factors are gone, and the clamping looks sane (CLAMP (v, -96, 12)). In particular I don't try to enforce restrictive CLAMP() boundaries for left_factor and right_factor, they are now an implementation detail and should not be used by the UI.

About GParamSpec containing minus, that's because GLib enforces '-' instead of '_' in the pspec names, so we're forced to work around that in other places as well. Please take a look at name_to_identifier in bstparam.cc, seems like it's time to move that to bstutils.hh to avoid gratituous duplicatoin everywhere.

Fixed.

About porting object / sequence properties, I'm actually not sure what's missing there atm. If you want to make an attempt at porting such a property and nag me about missing bits in the IDL layer or similar, that'd be ok as well.

Ok. Maybe I'll try it later.

+      for (auto& c : pname)

pointers and references are written with *& attached to the variable, i.e. write this as "auto &c"

Ok.

+ Const BUS_VOLUME_MIN = 1.58489319246111e-05; /* -96dB */

if at all, this should be using '96' as Const value, and the const be defined in terms of dB. but in other DAWs, the dB range displayed is a function of the UI, e.g. the volume meter of a track display my use different limits than for a related bus channel volume meter.

Fixed.

+        self->right_volume = self->left_volume = center_volume (self->right_volume, self->left_volume);

While you're at it, please make that a two liner.

Fixed. But code was deleted in a later commit.

  •  return (self == master);
    
Please remove extraneous parenthesis like these, they could hide an assignment.

Fixed.

+      auto parent = BSE_ITEM (self)->parent;

Please don't use 'auto' here, but BseItem*, like you did in other places. The reason here is that the variable is the result of a hidden C-style cast (BSE_ITEM()) and used as input for another C-style cast (BSE_SONG), so the actual type used here is completely unclear to the reader and potentially hiding a type bug.

Fixed.

On a side note, it took a while, but I figured again the point of saved_sync. It's clearly trying to preserve the 'sync' state around loading a BSE file (termed 'restore' in the API), and sets that initially to FALSE to allow BSE files to restore different left and right volumes, and syncs thouse only if "sync" is also set or after the restore phase is completed. That heavily depends on the order in which the properties are defined in the BSE file which is somewhat fragile... (*2)

Right. Consider this fixed, as we no longer need sync, and I see absolutely no point in trying to be too perfect here. We read left_volume and right_volume in any order, and that is it. The UI should always use the volume_db/pan properties. If the user used sync before saving, left_volume and right_volume will be the same, and pan will be 0. If not, left_volume and right_volume will be different, and pan will not be 0.

So after commenting on your remarks, let be briefly describe what I did to get volume_db/pan properties. First, I looked at some variations on how to compute factors like our left_volume/right_volume from a total volume_db and panning. A good description of the possibilities is here:

http://www.cs.cmu.edu/~music/icm-online/readings/panlaws/index.html

I choose to implement constant power panning (-3dB), because:

  • some popular DAWs use it as default (for instance Cubase)
  • the mathematical definition is quite straightforward (we want constant power)
  • from this follows that we have all functions we need to implement translation backwards/forwards

However this seems to be one of the things that don't have a globally optimal solution. For instance I tried Bitwig, expecting to see constant power panning, but they appear to use -4.5dB pan law (also confirmed on Forums). Cubase afaik has a project wide setting for -6dB, -4.5dB, -3dB, 0dB, constant power (default).

Long story short: I implemented the volume_db/pan properties as a view on the underlying left_volume/right_volume factors with constant power panning (-3dB). Constant power panning should be a good solution, and it gives the user the ability to control volume and panning individually. We can change the default later on, if we choose to implement other panning strategies.

I adapted the beast-gtk UI. As we have no knob widget, I use a horizontal slider to represent panning. I believe that the new UI is definitely an improvement over the old from a usability point of view, even if it may take some initial effort to understand which widget is doing what.

You'll also find one somewhat clumsy commit that allows setting a Bus::master_output property with and without undo. The old framework had the possibility to suppress undo via g_object_set. I have here implemented one method with and one without undo. However a framework solution would probably be better in the long run, such as
{
auto no_undo = bus->scoped_undo_blocker();
bus->master_output (true); // not undoable
}

The other case would be
{
bus->master_output (true); // undoable
}


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

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld in #78: Ok, so let me comment on anything that doesn't have to do with the new volume_db/pan properties first.\r\n\r\n\u003e Thanks Stefan, great to see steady progress here.\r\n\u003e \r\n\u003e But note that travis-ci shows, that your changes broke the syndrum audio test in all build variants, please take a look at that.\r\n\r\nRight. This is a really nasty problem, which is caused by loading/reading code for float64 properties in aida. It turns out that even simple floating point numbers like \"1.03\" are written out correctly, but restore breaks in bsestorage.cc, because the scanner_parse_paren_rest does return this as \"1. 3\" and this is restored as a value of 1.0. I've fixed this by using the existing sfi_scanner_parse_real_num function to parse float64 values (see also commit comments). This seems to work, tests pass again. Not sure if this is what you think we should do to fix the problem, though.\r\n\r\n\u003e About volume clamping, taking a look at other DAWs suggests that this should be done at the UI and doesn't need to be based on consts from the IDL at all.\r\n\u003e Also the property descriptios read \"Volume adjustment in decibel of left/right bus channel\" and that's what we should make them. I.e. we should pass actual dB values, e.g. 96 as property value through the IDL layer. (*1)\r\n\r\nOk. You'll notice that the magic factors are gone, and the clamping looks sane (CLAMP (v, -96, 12)). In particular I don't try to enforce restrictive CLAMP() boundaries for left_factor and right_factor, they are now an implementation detail and should not be used by the UI.\r\n\r\n\u003e About GParamSpec containing minus, that's because GLib enforces '-' instead of '_' in the pspec names, so we're forced to work around that in other places as well. Please take a look at name_to_identifier in bstparam.cc, seems like it's time to move that to bstutils.hh to avoid gratituous duplicatoin everywhere.\r\n\r\nFixed.\r\n\r\n\u003e About porting object / sequence properties, I'm actually not sure what's missing there atm. If you want to make an attempt at porting such a property and nag me about missing bits in the IDL layer or similar, that'd be ok as well.\r\n\r\nOk. Maybe I'll try it later.\r\n\r\n\u003e ```\r\n\u003e + for (auto\u0026 c : pname)\r\n\u003e ```\r\n\u003e pointers and references are written with *\u0026 attached to the variable, i.e. write this as \"auto \u0026c\"\r\n\r\nOk.\r\n\r\n\u003e ```\r\n\u003e + Const BUS_VOLUME_MIN = 1.58489319246111e-05; /* -96dB */\r\n\u003e ```\r\n\u003e if at all, this should be using '96' as Const value, and the const be defined in terms of dB. but in other DAWs, the dB range displayed is a function of the UI, e.g. the volume meter of a track display my use different limits than for a related bus channel volume meter.\r\n\r\nFixed.\r\n\r\n\u003e ```\r\n\u003e + self-\u003eright_volume = self-\u003eleft_volume = center_volume (self-\u003eright_volume, self-\u003eleft_volume);\r\n\u003e ```\r\n\u003e While you're at it, please make that a two liner.\r\n\r\nFixed. But code was deleted in a later commit.\r\n\r\n\u003e + return (self == master);\r\n\u003e ```\r\n\u003e Please remove extraneous parenthesis like these, they could hide an assignment.\r\n\r\nFixed.\r\n\r\n\u003e ```\r\n\u003e + auto parent = BSE_ITEM (self)-\u003eparent;\r\n\u003e ```\r\n\u003e Please don't use 'auto' here, but BseItem*, like you did in other places. The reason here is that the variable is the result of a hidden C-style cast (BSE_ITEM()) and used as input for another C-style cast (BSE_SONG), so the actual type used here is completely unclear to the reader and potentially hiding a type bug.\r\n\r\nFixed.\r\n\r\n\u003e On a side note, it took a while, but I figured again the point of saved_sync. It's clearly trying to preserve the 'sync' state around loading a BSE file (termed 'restore' in the API), and sets that initially to FALSE to allow BSE files to restore different left and right volumes, and syncs thouse only if \"sync\" is also set or after the restore phase is completed. That heavily depends on the order in which the properties are defined in the BSE file which is somewhat fragile... (*2)\r\n\r\nRight. Consider this fixed, as we no longer need sync, and I see absolutely no point in trying to be too perfect here. We read left_volume and right_volume in any order, and that is it. The UI should always use the volume_db/pan properties. If the user used sync before saving, left_volume and right_volume will be the same, and pan will be 0. If not, left_volume and right_volume will be different, and pan will not be 0.\r\n\r\n\r\nSo after commenting on your remarks, let be briefly describe what I did to get volume_db/pan properties. First, I looked at some variations on how to compute factors like our left_volume/right_volume from a total volume_db and panning. A good description of the possibilities is here:\r\n\r\nhttp://www.cs.cmu.edu/~music/icm-online/readings/panlaws/index.html\r\n\r\nI choose to implement constant power panning (-3dB), because:\r\n- some popular DAWs use it as default (for instance Cubase)\r\n- the mathematical definition is quite straightforward (we want constant power)\r\n- from this follows that we have all functions we need to implement translation backwards/forwards\r\n\r\nHowever this seems to be one of the things that don't have a globally optimal solution. For instance I tried Bitwig, expecting to see constant power panning, but they appear to use -4.5dB pan law (also confirmed on Forums). Cubase afaik has a project wide setting for -6dB, -4.5dB, -3dB, 0dB, constant power (default).\r\n\r\nLong story short: I implemented the volume_db/pan properties as a view on the underlying left_volume/right_volume factors with constant power panning (-3dB). Constant power panning should be a good solution, and it gives the user the ability to control volume and panning individually. We can change the default later on, if we choose to implement other panning strategies.\r\n\r\nI adapted the beast-gtk UI. As we have no knob widget, I use a horizontal slider to represent panning. I believe that the new UI is definitely an improvement over the old from a usability point of view, even if it may take some initial effort to understand which widget is doing what.\r\n\r\n\r\nYou'll also find one somewhat clumsy commit that allows setting a Bus::master_output property with and without undo. The old framework had the possibility to suppress undo via g_object_set. I have here implemented one method with and one without undo. However a framework solution would probably be better in the long run, such as\r\n{\r\n auto no_undo = bus-\u003escoped_undo_blocker();\r\n bus-\u003emaster_output (true); // not undoable\r\n}\r\n\r\nThe other case would be\r\n{\r\n bus-\u003emaster_output (true); // undoable\r\n}"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/78#issuecomment-421138602"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/78#issuecomment-421138602", "url": "https://github.com/tim-janik/beast/pull/78#issuecomment-421138602", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } }, { "@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [tim-janik/beast] Most Bus properties ported to C++ (#78)", "sections": [ { "text": "", "activityTitle": "**Stefan Westerfeld**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@swesterfeld", "facts": [ ] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"issueId\": 78,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close pull request", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"tim-janik/beast\",\n\"pullRequestId\": 78\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/78#issuecomment-421138602" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 375359114\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast