[tim-janik/beast] More Song properties C++ified (#69)

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

[tim-janik/beast] More Song properties C++ified (#69)

Gnome - Beast mailing list

Here is a C++ port for Song: tpqn / numerator / denominator

I renamed tpqn so that it says "TPQN" and not "Ticks" at the UI, because I think "Ticks" is a terrible name for that property, as it doesn't say what kind of ticks we have here.

I added STANDARD_RDONLY - we could also call this STANDARD RDONLY or STANDARD READONLY - I choose the name that SFI_* originally used.

I had to remove the computed defaults, since idl doesn't allow me to compute the default of a property. Previously the default values for properties were set in the Song implementation, so that the property defaults for bpm/numerator/... would always match the defaults from bse_song_timing_get_default().

However I think for Song, duplicating the defaults in bse_song_timing_get_default() and bseapi.idl is okay, because these probably will not change (often). One alternative would be to somehow allow the Song implementation to read the defaults that bseapi.idl defines, and so get rid of the duplication and make the idl file the canonic place.

The only Song property that remains is the post processing network, but since this has the type object, I am not able to port it now. If you have any preference for the next object properties to port, let me know.


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

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

Commit Summary

  • BSE: Song::tick_pointer: use C++ false rather than FALSE macro
  • BSE: Song::numerator: port property to C++
  • BSE: Song::denominator: port property to C++
  • BSE: Song::tpqn: 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":"More Song properties C++ified (#69)"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/69"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/69", "url": "https://github.com/tim-janik/beast/pull/69", "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": "More Song properties C++ified (#69)", "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": "fe160ac", "value": "BSE: Song::tick_pointer: use C++ false rather than FALSE macro" }, { "name": "2460bb9", "value": "BSE: Song::numerator: port property to C++" }, { "name": "fde484d", "value": "BSE: Song::denominator: port property to C++" }, { "name": "87f267c", "value": "BSE: Song::tpqn: port property to C++" } ] }, { "title": "File Changes", "facts": [ { "name": "Modified", "value": "[bse/bseapi.idl](https://github.com/tim-janik/beast/pull/69/files#diff-0) (58 changes)" }, { "name": "Modified", "value": "[bse/bsesong.cc](https://github.com/tim-janik/beast/pull/69/files#diff-1) (112 changes)" }, { "name": "Modified", "value": "[bse/bsesong.hh](https://github.com/tim-janik/beast/pull/69/files#diff-2) (6 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\": 69,\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\": 69\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/69" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/69.patch" } ], "@type": "OpenUri", "name": "View patch" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/69.diff" } ], "@type": "OpenUri", "name": "View diff" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 373245018\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] More Song properties C++ified (#69)

Gnome - Beast mailing list

Thanks a lot Stefan.
I'm not sure that "TPQN" is any better than "Ticks" for users at the UI, at least I'm not aware of another sequencer that has TPQN labels... I think in either case users will need the tooltip to decipher what the property is about.

There's one other thing I noticed during review. With the old GParamSpec + GObject based property API, the GObject machinery ensures that the GValue passed in to property setters complies with the GParamSpec value range. With our IDL API, nothing like that is enforced anymore. That means, an int32 property between 1-256 can now be set to 0 or -2^31 through the API. For a denominator to become 0 that could affect stability.
Please keep that in mind for ported properties and watch out if we shouldn't add extra guards, like:

denominator = CLAMP (input, 0, 256)


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 #69: Thanks a lot Stefan.\r\nI'm not sure that \"TPQN\" is any better than \"Ticks\" for users at the UI, at least I'm not aware of another sequencer that has TPQN labels... I think in either case users will need the tooltip to decipher what the property is about.\r\n\r\nThere's one other thing I noticed during review. With the old GParamSpec + GObject based property API, the GObject machinery ensures that the GValue passed in to property setters complies with the GParamSpec value range. With our IDL API, nothing like that is enforced anymore. That means, an int32 property between 1-256 can now be set to 0 or -2^31 through the API. For a denominator to become 0 that could affect stability.\r\nPlease keep that in mind for ported properties and watch out if we shouldn't add extra guards, like:\r\n\r\n denominator = CLAMP (input, 0, 256)"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/69#issuecomment-417492287"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/69#issuecomment-417492287", "url": "https://github.com/tim-janik/beast/pull/69#issuecomment-417492287", "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] More Song properties C++ified (#69)", "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\": 69,\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\": 69\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/69#issuecomment-417492287" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 373245018\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] More Song properties C++ified (#69)

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

Closed #69 via dc84f2c.


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":"Closed #69 via dc84f2c563775472ac985cf8bdad40c75423e49d."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/69#event-1819415121"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/69#event-1819415121", "url": "https://github.com/tim-janik/beast/pull/69#event-1819415121", "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] More Song properties C++ified (#69)", "sections": [ { "text": "", "activityTitle": "**Tim Janik**", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@tim-janik", "facts": [ ] } ], "potentialAction": [ { "targets": [ { "os": "default", "uri": "https://github.com/tim-janik/beast/pull/69#event-1819415121" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 373245018\n}" } ], "themeColor": "26292E" } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast