Hallo,
> >>>> implement inlineable functions PA_CVOLUME_VALID(),
> >>>> PA_CVOLUME_CHANNELS_EQUALS_TO(), PA_CVOLUME_IS_MUTED(),
> >>>> PA_CVOLUME_IS_NORM() that assume data is valid
> >>>
> >>> Why are these uppercase? AFAIK, we don't usually uppercase inline
> functions.
> >>
> >> I've modelled after e.g. PA_ALIGN_PTR() in pulsecore/macro.h or
> >> PA_SOURCE_IS_LINKED() in source.h
> >>
> >> I think there is no clear rule; I'd prefer PA_CVOLUME_IS_NORM() to
> >> pa_cvolume_is_norm_internal() or pa_cvolume_is_norm_unchecked()
> >>
> >> thanks, p.
> >
> > My preference is lower case for functions (I suspect the exceptions
> > began as macros).
>
> My preference is to avoid uppercase too. I think it's okay to move the
> pa_assert to pa_assert_fp in general if the asserts end up being CPU
> intensive - rather than making one pa_cvolume_is_norm() and one
> pa_cvolume_is_norm_unchecked() function.
there is the issue of
* inlineability (function under pulse/ are API/ABI)
* avoiding checks with NDEBUG
* naming (I'm fine with lowercase)
I suggest to add new functions under pulsecore/ that mirror those under
pulse/ but are inlineable and do less checking
let's take pa_cvolume_channels_equal_to() in pulse/volume.c as an example:
int pa_cvolume_channels_equal_to(const pa_cvolume *a, pa_volume_t v) {
unsigned c;
pa_assert(a);
pa_return_val_if_fail(pa_cvolume_valid(a), 0);
pa_return_val_if_fail(PA_VOLUME_IS_VALID(v), 0);
for (c = 0; c < a->channels; c++)
if (a->values[c] != v)
return 0;
return 1;
}
the function is called often in critial paths, non-inlineable and the
checks don't go away with NDEBUG since they use pa_return_val_if_fail()
and are mandated by the API
so I'd suggest to add a function pa_cvolume_channels_equal_to_internal()
in pulsecore/volume-util.h:
static inline int pa_cvolume_channels_equal_to_internal(const pa_cvolume
*a, pa_volume_t v) {
unsigned c;
pa_assert_fp(a);
pa_assert_fp(pa_cvolume_valid(a), 0);
pa_assert_fp(PA_VOLUME_IS_VALID(v), 0);
for (c = 0; c < a->channels; c++)
if (a->values[c] != v)
return 0;
return 1;
}
this results in
(1) the name is rather long and ugly, yet lowercase
(2) the function is inlineable and not part of the API/ABI
(3) checks will be go away with OPTIMIZE or NDEBUG, the behaviour is
DIFFERENT compared to pa_cvolume_channels_equal_to() -- the
assumption is that cvolumes and volumes are always valid internally
regards, p.
--
Peter Meerwald
+43-664-2444418 (mobile)
