[vlc-devel] [PATCH] Added Central Channel Filter module
Thomas Guillem
thomas at gllm.fr
Mon Apr 6 11:27:59 CEST 2020
On Fri, Apr 3, 2020, at 13:27, Vedanta Nayak wrote:
> Hello, I am an applicant for GSoc and this is my first time sending a
> patch on the vlc-devel list, so please excuse any errors.
> I have implemented an upmixing filter for stereo audio which makes a
> central channel by averaging the left and the right channels and
> dividing by a factor of sqrt(2).
Hello, thanks for this patch, cf. my review inline:
> ---
> modules/audio_filter/Makefile.am | 6 +-
> modules/audio_filter/center.c | 139 +++++++++++++++++++++++++++++++
> 2 files changed, 143 insertions(+), 2 deletions(-)
> create mode 100644 modules/audio_filter/center.c
>
> diff --git a/modules/audio_filter/Makefile.am b/modules/audio_filter/Makefile.am
> index 309074c75b..f17630cc2a 100644
> --- a/modules/audio_filter/Makefile.am
> +++ b/modules/audio_filter/Makefile.am
> @@ -33,7 +33,8 @@ libspatializer_plugin_la_SOURCES = \
> audio_filter/spatializer/revmodel.hpp \
> audio_filter/spatializer/spatializer.cpp
> libspatializer_plugin_la_LIBADD = $(LIBM)
> -
> +libcenter_plugin_la_SOURCES = audio_filter/center.c
> +libcenter_plugin_la_LIBADD = $(LIBM)
> audio_filter_LTLIBRARIES = \
> libaudiobargraph_a_plugin.la \
> libchorus_flanger_plugin.la \
> @@ -46,7 +47,8 @@ audio_filter_LTLIBRARIES = \
> libscaletempo_plugin.la \
> libscaletempo_pitch_plugin.la \
> libspatializer_plugin.la \
> - libstereo_widen_plugin.la
> + libstereo_widen_plugin.la \
> + libcenter_plugin.la
>
> # Channel mixers
> libdolby_surround_decoder_plugin_la_SOURCES = \
> diff --git a/modules/audio_filter/center.c
> b/modules/audio_filter/center.c
> new file mode 100644
> index 0000000000..07df0e18dc
> --- /dev/null
> +++ b/modules/audio_filter/center.c
> @@ -0,0 +1,139 @@
> +/*****************************************************************************
> + * center.c : Central channel filter
> +
> *****************************************************************************
> + * Copyright © 2020 VLC authors and VideoLAN
> + *
> + * Authors: Vedanta Nayak <vedantnayak2 at gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or
> modify it
> + * under the terms of the GNU Lesser General Public License as
> published by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> License
> + * along with this program; if not, write to the Free Software
> Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> +
> *****************************************************************************/
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <vlc_common.h>
> +#include <vlc_aout.h>
> +#include <vlc_filter.h>
> +#include <vlc_plugin.h>
> +
> +
> +/*------------ Declare local functions -----------*/
Nit: such comments are not really useful, specially for such a small code.
> +static int Open (vlc_object_t *);
> +static block_t *Process (filter_t *, block_t *);
> +static void Close (vlc_object_t *);
> +typedef struct
> +{
> + int i_left;
> + int i_right;
> + int i_center;
> +}sys_t;
> +/*------------ Module description ----------------*/
> +
> +vlc_module_begin()
> + set_shortname (N_("Center"))
> + set_description (N_("Create a central channel"))
> + set_category (CAT_AUDIO)
> + set_subcategory (SUBCAT_AUDIO_AFILTER)
> + set_capability ("audio filter",0)
> + set_callbacks (Open,Close)
> +vlc_module_end ()
If you put the module description at the end of the file, you won't need to declare local functions.
> +
> +/*------------ Open function ---------------------*/
> +
> +static int Open (vlc_object_t *in)
> +{
> + int i = 0;
> + int i_offset = 0;
> + filter_t *filter = (filter_t *)in;
> + sys_t *p_sys;
> + if (filter->fmt_in.audio.i_channels != 2)
> + {
> + return VLC_EGENERIC;
> + }
> + p_sys = filter->p_sys = malloc (sizeof(sys_t));
> + if (p_sys == NULL)
> + return VLC_ENOMEM;
> + p_sys->i_left = -1;
> + p_sys->i_right = -1;
> +
> + while (pi_vlc_chan_order_wg4[i])
> + {
> + if ( filter->fmt_out.audio.i_physical_channels &&
> pi_vlc_chan_order_wg4[i] )
> + {
> + switch( pi_vlc_chan_order_wg4[i] )
> + {
> + case AOUT_CHAN_LEFT:
> + p_sys->i_left = i_offset;
> + break;
> + case AOUT_CHAN_RIGHT:
> + p_sys->i_right = i_offset;
> + break;
> + case AOUT_CHAN_CENTER:
> + p_sys->i_center = i_offset;
> + }
> + ++i_offset;
> + }
> + ++i;
> + }
In VLC, audio channels are always expected to be in the pi_vlc_chan_order_wg4 order.
Therefore, it will be always LEFT, RIGHT, CENTER in that order and you could simplify this code a lot.
To make sure this module won't be broken if the VLC order change in the future, you should add a static_assert that will check that AOUT_CHAN_LEFT < AOUT_CHAN_RIGHT < AOUT_CHAN_CENTER
> + filter->fmt_out.audio.i_format = VLC_CODEC_FL32;
> + filter->fmt_in.audio.i_format = VLC_CODEC_FL32;
> + filter->fmt_out.audio.i_physical_channels = AOUT_CHAN_LEFT |
> AOUT_CHAN_RIGHT | AOUT_CHAN_CENTER;
> + filter->fmt_out.audio.i_rate = filter->fmt_in.audio.i_rate;
> + aout_FormatPrepare(&filter->fmt_in.audio);
> + aout_FormatPrepare(&filter->fmt_out.audio);
> + filter->pf_audio_filter = Process;
> + return VLC_SUCCESS;
> +}
> +
> +static void Close (vlc_object_t *this)
> +{
> + filter_t *temp = (filter_t *)this;
> + free(temp->p_sys);
> +}
> +
> +
> +/*---------- Process function --------------------*/
> +static block_t *Process ( filter_t *filter, block_t *in_buf )
> +{
> + float *in = (float*)in_buf->p_buffer;
> + sys_t * p_sys = filter->p_sys;
> + size_t i_nb_samples = in_buf->i_nb_samples;
> + size_t i_nb_channels =
> aout_FormatNbChannels(&filter->fmt_out.audio);
It will be always 3 since you force it in the Open function. You could use a define instead.
> + size_t i;
> + block_t *out_buf = block_Alloc(sizeof(float) * i_nb_samples *
> i_nb_channels );
> + if ( !out_buf )
> + {
> + block_Release(in_buf);
> + return out_buf;
> + }
> + float * p_out = (float*)out_buf->p_buffer;
> + out_buf->i_nb_samples = i_nb_samples;
> + out_buf->i_dts = in_buf->i_dts;
> + out_buf->i_pts = in_buf->i_pts;
> + out_buf->i_length = sizeof(float) * i_nb_samples;
> + memset(out,0,out_buf->i_buffer);
This doesn't compile. What is this out variable ? Beside, you don't need to set the out buffer to 0 since you will override it just after.
> + const float factor = .70710678;
> + for ( i = 0 ; i < i_nb_samples ; ++i)
> + {
> + float left = in[i*2];
> + float right = in[i*2+1];
> + float center = ( left + right ) / 2;
You should multiply it by factor here directly
> + p_out[i * i_nb_channels + p_sys->i_left ] = left;
> + p_out[i * i_nb_channels + p_sys->i_right ] = right;
> + p_out[i * i_nb_channels + p_sys->i_center] = center * factor;
> + }
> + block_Release(in_buf);
> + return out_buf;
> +}
> --
> 2.26.0
Best regards,
Thomas Guillem
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list