Sunday, July 14, 2013

Vol 1. Refactoring

Hi. Time for a quick update about my Summer of Code project.
I'm going to talk a little about my first checklist item, refactoring the resampling code. While refactoring isn't quite exciting as implementing new features it is a necessity to assure code maintainability.

So let's take a look at the resampler interface and see what we can do. The interface consists of a pa_resampler structure which we create if we want to do some resampling. This structure holds all of our settings the resampler cares about, (sample specifications, different buffers) but also some specific data for different resampling implementations.
The interesting bits are shown bellow.

struct pa_resampler {
...
    void (*impl_free)(pa_resampler *r);
    void (*impl_update_rates)(pa_resampler *r);
    void (*impl_resample)(pa_resampler *r, const pa_memchunk *in, 
                          unsigned in_samples, pa_memchunk *out, 
                          unsigned *out_samples);
    void (*impl_reset)(pa_resampler *r);

    struct { /* data specific to the trivial resampler */
        unsigned o_counter;
        unsigned i_counter;
    } trivial;

    struct { /* data specific to the peak finder pseudo resampler */
        unsigned o_counter;
        unsigned i_counter;

        float max_f[PA_CHANNELS_MAX];
        int16_t max_i[PA_CHANNELS_MAX];

    } peaks;
...
};
After the implementation specific function pointers we can see multiple structures holding implementation specific data. Since the resampler can't switch implementations on the fly (without destroying and recreating a resampler) only one of these structures is used at a time.
There are six of those structures contained inside of pa_resampler and some of them have only a single member which is quite pointless.

Further bellow of the file we see a big init_table containing the mapping between a resampling method (an enumeration) and its initialization function.

static int (* const init_table[])(pa_resampler*r) = {
    [PA_RESAMPLER_SRC_SINC_BEST_QUALITY]   = libsamplerate_init,
    [PA_RESAMPLER_SRC_SINC_MEDIUM_QUALITY] = libsamplerate_init,
    [PA_RESAMPLER_SRC_SINC_FASTEST]        = libsamplerate_init,
    [PA_RESAMPLER_SRC_ZERO_ORDER_HOLD]     = libsamplerate_init,
    [PA_RESAMPLER_SRC_LINEAR]              = libsamplerate_init,
    [PA_RESAMPLER_TRIVIAL]                 = trivial_init,
    [PA_RESAMPLER_SPEEX_FLOAT_BASE+0]      = speex_init,
    [PA_RESAMPLER_SPEEX_FLOAT_BASE+1]      = speex_init,
    ...
    [PA_RESAMPLER_SPEEX_FLOAT_BASE+10]     = speex_init,
    [PA_RESAMPLER_SPEEX_FIXED_BASE+0]      = speex_init,
    ...
    [PA_RESAMPLER_SPEEX_FIXED_BASE+10]     = speex_init,
    ...
As we can see there are quite some duplicates here. There are a total of 32 entries while only having 6 distinctive init functions. There is another big table like this containing the implementation names, this table doesn't contain any duplicates but it would be nice if we could group the implementation names into smaller implementation specific tables.

So without further ado here is the first code snippet with the appropriate changes.

struct pa_resampler {
    ...
    pa_resampler_implementation implementation;
    ...
};
All the implementation specific data is now contained inside a single structure.
And here is how the equivalent second code snippet looks like.
static pa_resampler_implementation *impl_table[] = {
    [PA_RESAMPLER_SRC_SINC_FASTEST] = &libsamplerate_impl,
    [PA_RESAMPLER_TRIVIAL] = &trivial_impl,
    [PA_RESAMPLER_SPEEX_FLOAT_BASE] = &speex_impl,
    [PA_RESAMPLER_FFMPEG] = &ffmpeg_impl,
    [PA_RESAMPLER_AUTO] = &auto_impl,
    [PA_RESAMPLER_COPY] = ©_impl,
    [PA_RESAMPLER_PEAKS] = &peaks_impl,
};
No more duplicate entries here.
And at last here is how the pa_resampler_implementation structure is defined.
struct pa_resampler_implementation {
    int (*init)(pa_resampler *r);
    void (*free)(pa_resampler *r);
    void (*update_rates)(pa_resampler *r);
    void (*resample)(pa_resampler *r, const pa_memchunk *in,
                     unsigned in_samples, pa_memchunk *out,
                     unsigned *out_samples);
    void (*reset)(pa_resampler *r);
    void *data;
    const char *names[PA_RESAMPLER_MAX_VARIANTS];
};
The implementation specific structures are replaced by a simple opaque data entry and the implementations init function takes care of the allocating. The big names table is also now split up and contained inside this structure.

These changes aren't yet merged upstream and some of it may change if needed. Further details are on my github page.
Thats it for now, next time there should be a more interesting topic. Thanks for your attention and bye.

No comments:

Post a Comment

Post a Comment