[gstreamer-bugs] [Bug 609801] [volume] Use sample accurate property values if a controller is used

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=609801
  GStreamer | gst-plugins-base | git

--- Comment #14 from Sebastian Dröge <slomo at circular-chaos.org> 2010-02-18 08:13:22 UTC ---
(In reply to comment #12)
> Now more comments on the actual patch - it is nicely done. But honestly I don't
> think we want to do it like this for audio. For video we can sync on each
> frame. For audio we want to sync at a similar interval.
> 
> process_controlled_*
> ...
>   for (i = 0; i < num_samples; i++) {
>     for (j = 0; j < channels; j++) {
>       if (*mute) {
>         *data++ = 0;
>       } else {
>         gdouble val = *data * (*volume) + 0.5;
> 
>         *data++ = (gint8) CLAMP (val, VOLUME_MIN_INT8, VOLUME_MAX_INT8);
>       }
>     }
>     volume++;
>     mute++;
>   }
> 
> lets at least move the "if (*mute)" check out of the channel loop. Otherwise
> this is okay. Compared to the uncontrolled loops we have the overhead for
> nested loops and interating over 3 arrays. In addition we have the overhead for
> filling the 2 control value arrays all the time. SO the controller overhead is
> larger than the actual operation.

Yes, that can be optimized of course. We could even get rid of the mute array
by first combining it with the volume array.

> I see the control-value array functionality more suitable for fetching the
> control curse for display purposes in the UI.
> 
> in volume_before_transform:
> 
> * shouldn't the GstControlSource object be taken in e.g. volume_setup() - this
> doesn't change while running.

It doesn't? It can change at every point in time unfortunately, it could even
change while we're processing a buffer. Nothing is preventing the application
from doing that.

> * also filling the self->mutes/volumes with default values needs to be done
> when the size has changes and not on every buffer.

Because the control source can change for every buffer it is necessary
unfortunately. For one buffer both could be there, for the next only the volume
csource could be there.

(In reply to comment #13)
> In reply to comment #11. I agree that if the inner loop is multiplying the
> samples with a const value or with one vector, then the speed difference might
> be acceptable. To get there we would need:
> * process_controlled() to take only one volume array
>   * thus if mute is also controlled, merge both: volume=(1-mute)*volume;

Yes, that would probably be a good idea, see above ;) If the generic approach
here is accepted I can optimize this kind of things

> * some means to avoid updating control value array unless there is a change

the GstController API definitely needs some improvements and cleanups, yes ;)
Most of that can only be done in 0.11 though...


Another way of lowering the overhead would be to generate a property value for
every second sample or for every n-samples. That could be combined with the
control-rate of the GstController somehow.

But it's definitely better to do this way than to sync the properties
(including notify signals and stuff) multiple times for every buffer, that has
a bigger overhead ;)

(In reply to comment #11)

> It can't imagine that rendering a controller curve for N samples should be
> such an expensive operation. Even less so when the algorithm is allowed to
> take shortcuts based on the configured granularity. I might be wrong, though...

Well, it's quite expensive. Let's take the GstInterpolationControlSource in
linear interpolation mode for example (cubic is even more expensive of course,
same goes for the GstLFOControlSource). For every position it currently
searches inside a binary tree (GSequence to be exact), gets the current and
next value, interpolates, returns it. Doing that for every sample is probably
not the best idea :) Yes, for generating a row of values there could be a
single binary tree search and incremental searches for the next value but
that's still quite expensive.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.