Re: [tim-janik/beast] Jack driver (#31)

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

Re: [tim-janik/beast] Jack driver (#31)

Stefan Westerfeld-2

@swesterfeld pushed 1 commit.

  • 48895f7 JACK: improve doxygen comments for FrameRingBuffer class


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/1efc568137fc475e59717865f4024fb38e016474..48895f7fa1ca045dcc1d00e8b5128ce83984035a"}}}</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] Jack driver (#31)

Stefan Westerfeld-2

@swesterfeld pushed 1 commit.

  • 6d2c991 JACK: avoid vector<vector<T>> (emacs syntax highlighting)


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/48895f7fa1ca045dcc1d00e8b5128ce83984035a..6d2c991b6fcab0a8ac3f7f68db11a6f7e1066572"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

@swesterfeld pushed 1 commit.

  • f63e535 JACK: document TEST_DROPOUT() macro, useful for debugging the driver


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/6d2c991b6fcab0a8ac3f7f68db11a6f7e1066572..f63e5357a9e79f947e55d416fa04fb2f24c41134"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

@swesterfeld pushed 1 commit.

  • eb25b3c JACK: rename debug macro PDEBUG->JDEBUG


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/f63e5357a9e79f947e55d416fa04fb2f24c41134..eb25b3cf29ee3df7562943e0706efce8de1e48f9"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

@swesterfeld pushed 1 commit.

  • b1a6e22 JACK: add actual values to ringbuffer size warning


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/eb25b3cf29ee3df7562943e0706efce8de1e48f9..b1a6e2226e2bab777df7572c6fe09a4d3312785b"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

@swesterfeld pushed 1 commit.

  • c91a4db JACK: use C++11 in-class initialization


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/b1a6e2226e2bab777df7572c6fe09a4d3312785b..c91a4db5d535e21ef68d20bba4cd381b914bad51"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

@swesterfeld pushed 1 commit.

  • 77d2ac6 JACK: remove trailing whitespaces


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/aaf4de220abe684ff29ebf4f1536f998cd9c8c55..77d2ac6b91f5d3a36d9eeadbbe82dc473e796f5c"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

configure.ac: work the "Output summary message" into a different patch set, that's to be discussed seperately form a jackd driver. And for what's worth, if we do that, I'd want a format similar to what Rapicorn has, and also add a lot more information about other packages there. I.e. that's why it should become a seperate discussion. And, IMHO, software should never command a user the way it's done at the end of the summary, that's can easily feel insulting.

I've suggested a patch for a configure summary message here: #33

Once this is merged, we can rebase this branch.

In the comment on "class FrameRingBuffer", add a brief summary in the first line, our doxygen config extracts one-liner briefs from the first line. Same for the other function comments, Add a brief one-liner, before starting with @returns ;-)

Done. Although the class is in an anonymous namespace, so doxygen will probably ignore it.

Sigh, emacs suckage, please insert a space between angel brackets when writing "vector". C++11 can handle it fine but my emacs highlighter still trips up on << >>.

Done.

guint, gfloat, etc. We've stopped using the useless glib type aliases years ago. The only case this actually helps (arguably) is: gchar string_to_free_via_glib = g_strdup ("..."); So you know to use g_free on the gchar later on. In any case we definitely don't use these in newly written code, so please adjust by using uint and fix/squash your commit into the first one introducing the types.

We've got quite a bit of history here, so it wasn't easy, but I've eliminated guint and friends.

Remove TEST_DROPOUT (unused)

I've documented what it does instead, it is used to create dropouts in the driver, from which it should recover properly, I've used this during development.

Move to top of file: define PDEBUG - and, since this is jack, it should be JDEBUG ;-)

Done.

Using '%' is notoriously slow (it's actually a DIV for the CPU), is there a way the ring buffer is likely to be useful with power-of-2 values only? We could use a bit mask instead of '%' then.

Well, its not a huge problem, as % it is only called once per block size, so I believe if you were to profile the jack driver, practically no time would be spend here, but hopefully somewhere in the synthesis.

We do have non-power-of-two values, because bse itself has non-power-of-two engine blocks. In fact for jack we could do better on that side, i.e. if the jack callback is 64 values, then our engine block size should probably be 64 values as well. At least I believe that is what everybody else is doing.

However, the ring buffer requires one extra sample space to distinguish between a full and empty buffer. So what we could do is allocate the next power of two as ringbuffer size. As we're only speaking of a speculative optimization that I believe would not make any difference at all here, I'm not particularily keen on changing things, though. It works as it is, is tested, and I don't think it really needs to be changed, or would make a measurable difference anyway.

Add the actual values to a warning like this: Bse::warning ("JACK driver: ring buffer size not correct: (jack->buffer_frames != buffer_frames)\n"); I.e. add ": (%u != %u)", so if we get a user report, we can see what went wrong, not just where.

Done.

ctors: DeviceDetails() : ports (0), input_ports (0), output_ports (0), etc - Please use direct field initializers in newly written code, i.e. uint ports = 0;

Done.

While you're at it, also always rebase to latest master and do a non-linear push.

Done.

Finally, you have a bunch of trailing whitespaces, please remove. Emacs has a command here, there's probably something in vim as well that can display/kill trailing whitespaces.

Fixed.


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld in #31: \u003e configure.ac: work the \"Output summary message\" into a different patch set, that's to be discussed seperately form a jackd driver. And for what's worth, if we do that, I'd want a format similar to what Rapicorn has, and also add a lot more information about other packages there. I.e. that's why it should become a seperate discussion. And, IMHO, software should never command a user the way it's done at the end of the summary, that's can easily feel insulting.\r\n\r\nI've suggested a patch for a configure summary message here: https://github.com/tim-janik/beast/pull/33\r\n\r\nOnce this is merged, we can rebase this branch.\r\n\r\n\u003e In the comment on \"class FrameRingBuffer\", add a brief summary in the first line, our doxygen config extracts one-liner briefs from the first line. Same for the other function comments, Add a brief one-liner, before starting with @returns ;-)\r\n\r\nDone. Although the class is in an anonymous namespace, so doxygen will probably ignore it.\r\n\r\n\u003e Sigh, emacs suckage, please insert a space between angel brackets when writing \"vector\u003cvector\u003e\". C++11 can handle it fine but my emacs highlighter still trips up on \u003c\u003c \u003e\u003e.\r\n\r\nDone.\r\n\r\n\u003e guint, gfloat, etc. We've stopped using the useless glib type aliases years ago. The only case this actually helps (arguably) is: gchar string_to_free_via_glib = g_strdup (\"...\"); So you know to use g_free on the gchar later on. In any case we definitely don't use these in newly written code, so please adjust by using uint and fix/squash your commit into the first one introducing the types.\r\n\r\nWe've got quite a bit of history here, so it wasn't easy, but I've eliminated guint and friends.\r\n\r\n\u003e Remove TEST_DROPOUT (unused)\r\n\r\nI've documented what it does instead, it is used to create dropouts in the driver, from which it should recover properly, I've used this during development.\r\n\r\n\u003e Move to top of file: define PDEBUG - and, since this is jack, it should be JDEBUG ;-)\r\n\r\nDone.\r\n\r\n\u003e Using '%' is notoriously slow (it's actually a DIV for the CPU), is there a way the ring buffer is likely to be useful with power-of-2 values only? We could use a bit mask instead of '%' then.\r\n\r\nWell, its not a huge problem, as % it is only called once per block size, so I believe if you were to profile the jack driver, practically no time would be spend here, but hopefully somewhere in the synthesis.\r\n\r\nWe do have non-power-of-two values, because bse itself has non-power-of-two engine blocks. In fact for jack we could do better on that side, i.e. if the jack callback is 64 values, then our engine block size should probably be 64 values as well. At least I believe that is what everybody else is doing.\r\n\r\nHowever, the ring buffer requires one extra sample space to distinguish between a full and empty buffer. So what we could do is allocate the next power of two as ringbuffer size. As we're only speaking of a speculative optimization that I believe would not make any difference at all here, I'm not particularily keen on changing things, though. It works as it is, is tested, and I don't think it really needs to be changed, or would make a measurable difference anyway.\r\n\r\n\u003e Add the actual values to a warning like this: Bse::warning (\"JACK driver: ring buffer size not correct: (jack-\u003ebuffer_frames != buffer_frames)\\n\"); I.e. add \": (%u != %u)\", so if we get a user report, we can see what went wrong, not just where.\r\n\r\nDone.\r\n\r\n\u003e ctors: DeviceDetails() : ports (0), input_ports (0), output_ports (0), etc - Please use direct field initializers in newly written code, i.e. uint ports = 0;\r\n\r\nDone.\r\n\r\n\u003e While you're at it, also always rebase to latest master and do a non-linear push.\r\n\r\nDone.\r\n\r\n\u003e Finally, you have a bunch of trailing whitespaces, please remove. Emacs has a command here, there's probably something in vim as well that can display/kill trailing whitespaces.\r\n\r\nFixed."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31#issuecomment-373103450"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

@swesterfeld pushed 1 commit.


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #31"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31/files/77d2ac6b91f5d3a36d9eeadbbe82dc473e796f5c..af6f5c856f4485fec2bc3104b3f4b86f43abbca6"}}}</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] Jack driver (#31)

Stefan Westerfeld-2
In reply to this post by Stefan Westerfeld-2

Ok, as discussed previously, I created a minimal description of how to start jackd and then beast so that we use jackd. I'm not really convinced that this documentation is useful to the average user, but it could help starting jackd if you normally don't use it.

https://github.com/swesterfeld/beast/blob/jack-driver/docs/howto-jack.md


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://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld in #31: Ok, as discussed previously, I created a minimal description of how to start jackd and then beast so that we use jackd. I'm not really convinced that this documentation is useful to the average user, but it could help starting jackd if you normally don't use it.\r\n\r\nhttps://github.com/swesterfeld/beast/blob/jack-driver/docs/howto-jack.md"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/31#issuecomment-373150580"}}}</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast