> 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:
https://github.com/tim-janik/beast/pull/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<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 or view it on GitHub:
https://github.com/tim-janik/beast/pull/31#issuecomment-373103450_______________________________________________
beast mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/beast
