-
Notifications
You must be signed in to change notification settings - Fork 34
Pulse buffer fixes #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
2879c93
to
f5ccb90
Compare
f5ccb90
to
49a7ffa
Compare
The int cast is wrong and unnecessary as internal->buffer_time is a pa_usec_t (which is uint64_t). It does not even make sense to cast it to a uint as it being multiplied by the sample rate (and then divided by 1000000 before it get fitted into battr.tlength, which is a uint32_t). Also ditch the "+7" trick for format->bits as the function would have returned already if it isn't 8/16(/24).
We really just want to set battr.tlength (as its default is 2s). It's better to left the others for pulse to determine.
49a7ffa
to
6872758
Compare
@@ -73,7 +73,7 @@ typedef struct ao_pulse_internal { | |||
struct pa_simple *simple; | |||
char *server, *sink, *client_name; | |||
pa_usec_t static_delay; | |||
pa_usec_t buffer_time; | |||
int buffer_time; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this make more sense as unsigned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ePirat To be frank, my C is really bad, so I don't exactly know. I don't even remember why I made this change because this was so long ago. I think the reason was that atoi
returns int
, and/or there was this (int)
casting for (internal->buffer_time * format->rate)
, so to prevent something goes wrong accidentally (like it being an unsigned resulting in other variables getting set to some unexpected value), I went with int
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, actually it might be because atoi(value) * 1000
was apparently due to buffer_time
being a "usec", and because there is no "pa_msec_t", I went with what atoi
returns when I remove the * 1000
(and didn't care what tlength
is)...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I maybe just ditch this line of change? Like keeping it as pa_usec_t
(uint64_t
)? Or should I change it to unsigned int
(or uint32_t
?) anyway?
(Does using 64-bit for it minimize the chance of overflow as of internal->buffer_time * format->rate
(i.e. before / 1000
)? Or does the "limit" come from what tlength
is instead?)
No description provided.