[vlc-devel] [PATCH 2/2] video_filter: add a module using converters around a filter when it can't be used as-is

Filip Roséen filip at atch.se
Wed May 10 15:49:37 CEST 2017


Hi Steve,

I am not sure what the status for this patch is after *Remi's*
comments, I however wrote my reply prior to noticing that there was an
on-going discussion, so I will send it to `vlc-devel` in either case.

On 2017-05-05 10:14, Steve Lhomme wrote:

> Fixes #18078
> ---
>  modules/MODULES_LIST             |   1 +
>  modules/video_filter/Makefile.am |   5 +-
>  modules/video_filter/chain.c     | 216 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 221 insertions(+), 1 deletion(-)
>  create mode 100644 modules/video_filter/chain.c
> 
> diff --git a/modules/MODULES_LIST b/modules/MODULES_LIST
> index 3205a0850b..b1d17da830 100644
> --- a/modules/MODULES_LIST
> +++ b/modules/MODULES_LIST
> @@ -144,6 +144,7 @@ $Id$
>   * file_keystore: store secrets on a file, may use a submodule to crypt secrets
>   * file_logger: file logger plugin
>   * filesystem: Filesystem access module
> + * filterchain: Video filtering using intermediate converter modules
>   * fingerprinter: AcoustID audio fingerprinter using chromaprint
>   * flac: Flac decoder using libflac
>   * flacsys: FLAC demuxer
> diff --git a/modules/video_filter/Makefile.am b/modules/video_filter/Makefile.am
> index 8fcc93c4db..bf4b9ce9ad 100644
> --- a/modules/video_filter/Makefile.am
> +++ b/modules/video_filter/Makefile.am
> @@ -67,6 +67,8 @@ libvhs_plugin_la_SOURCES = video_filter/vhs.c
>  libwave_plugin_la_SOURCES = video_filter/wave.c
>  libwave_plugin_la_LIBADD = $(LIBM)
>  
> +libfilterchain_plugin_la_SOURCES = video_filter/chain.c
> +
>  video_filter_LTLIBRARIES = \
>  	libadjust_plugin.la \
>  	libalphamask_plugin.la \
> @@ -105,7 +107,8 @@ video_filter_LTLIBRARIES = \
>  	libfps_plugin.la \
>  	libfreeze_plugin.la \
>  	libpuzzle_plugin.la \
> -	librotate_plugin.la
> +	librotate_plugin.la \
> +	libfilterchain_plugin.la
>  
>  libdeinterlace_plugin_la_SOURCES = \
>  	video_filter/deinterlace/deinterlace.c video_filter/deinterlace/deinterlace.h \
> diff --git a/modules/video_filter/chain.c b/modules/video_filter/chain.c
> new file mode 100644
> index 0000000000..b9ab7083e8
> --- /dev/null
> +++ b/modules/video_filter/chain.c
> @@ -0,0 +1,216 @@
> +/*****************************************************************************
> + * chain.c : chain multiple video filter modules as a last resort solution
> + *****************************************************************************
> + * Copyright (C) 2017 VLC authors, VideoLAN and VideoLabs
> + * $Id$
> + *
> + * Authors: Steve Lhomme <robux4 at videolabs.io>
> + *
> + * 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.
> + *****************************************************************************/
> +
> +/*****************************************************************************
> + * Preamble
> + *****************************************************************************/
> +
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +#include <vlc_filter.h>
> +#include <vlc_picture.h>
> +
> +/*****************************************************************************
> + * Local prototypes.
> + *****************************************************************************/
> +static const vlc_fourcc_t pi_allowed_chromas[] = {
> +    VLC_CODEC_I420,
> +    VLC_CODEC_I422,
> +    VLC_CODEC_I420_10L,
> +    VLC_CODEC_I420_16L,
> +    VLC_CODEC_RGB32,
> +    VLC_CODEC_RGB24,
> +    0
> +};
> +
> +struct filter_sys_t
> +{
> +    filter_chain_t *p_chain;
> +};
> +
> +/*****************************************************************************
> + * Buffer management
> + *****************************************************************************/
> +static picture_t *BufferNew( filter_t *p_filter )
> +{
> +    filter_t *p_parent = p_filter->owner.sys;
> +
> +    return filter_NewPicture( p_parent );
> +}
> +
> +#define CHAIN_LEVEL_MAX 1
> +#define RECURSIVE_STRING (MODULE_STRING "-flevel")
> +
> +/*****************************************************************************
> + * Builders
> + *****************************************************************************/
> +
> +static int BuildShortcutChain( filter_t *p_filter )
> +{
> +    es_format_t fmt_mid;
> +    int i_ret = VLC_EGENERIC;
> +
> +    /* Now try chroma format list */
> +    for( int i = 0; pi_allowed_chromas[i]; i++ )
> +    {
> +        filter_chain_Reset( p_filter->p_sys->p_chain, &p_filter->fmt_in, &p_filter->fmt_out );
> +
> +        const vlc_fourcc_t i_chroma = pi_allowed_chromas[i];
> +        if( i_chroma == p_filter->fmt_in.i_codec ||
> +            i_chroma == p_filter->fmt_out.i_codec )
> +            continue;
> +
> +        msg_Dbg( p_filter, "Trying to use chroma %4.4s as middle man",
> +                 (char*)&i_chroma );
> +
> +        es_format_Copy( &fmt_mid, &p_filter->fmt_in );
> +        fmt_mid.i_codec        =
> +        fmt_mid.video.i_chroma = i_chroma;
> +        fmt_mid.video.i_rmask  = 0;
> +        fmt_mid.video.i_gmask  = 0;
> +        fmt_mid.video.i_bmask  = 0;
> +        video_format_FixRgb(&fmt_mid.video);
> +
> +        if( filter_chain_AppendConverter( p_filter->p_sys->p_chain,
> +                                          NULL, &fmt_mid ) == VLC_SUCCESS )
> +        {
> +            filter_t *found = filter_chain_AppendFilter( p_filter->p_sys->p_chain,
> +                                                         p_filter->shortcut,
> +                                                         p_filter->p_cfg, &fmt_mid,
> +                                                         &fmt_mid, true );
> +            if (found != NULL)
> +            {
> +                if ( p_filter->b_allow_fmt_out_change ||
> +                     filter_chain_AppendConverter(p_filter->p_sys->p_chain, &fmt_mid,
> +                                                  &p_filter->fmt_out) == VLC_SUCCESS)
> +                {
> +                    es_format_Clean( &fmt_mid );
> +                    i_ret = VLC_SUCCESS;
> +                    break;
> +                }
> +            }
> +        }
> +        es_format_Clean( &fmt_mid );
> +    }
> +    if (i_ret != VLC_SUCCESS)
> +        filter_chain_Reset( p_filter->p_sys->p_chain, &p_filter->fmt_in, &p_filter->fmt_out );
> +
> +    var_Destroy( p_filter, RECURSIVE_STRING );
> +    return i_ret;
> +}
> +
> +/*****************************************************************************
> + * Chain
> + *****************************************************************************/
> +static picture_t *Chain( filter_t *p_filter, picture_t *p_pic )
> +{
> +    return filter_chain_VideoFilter( p_filter->p_sys->p_chain, p_pic );
> +}
> +
> +/*****************************************************************************
> + * Activate: allocate a chroma function
> + *****************************************************************************
> + * This function allocates and initializes a chroma function
> + *****************************************************************************/

Doxygen comments?

> +static int Activate( vlc_object_t *p_this )
> +{
> +    filter_t *p_filter = (filter_t *)p_this;
> +    filter_sys_t *p_sys;
> +    int i_ret;
> +
> +    if (p_filter->shortcut == NULL)
> +        return VLC_EGENERIC;
> +
> +    p_sys = p_filter->p_sys = calloc( 1, sizeof( *p_sys ) );
> +    if( !p_sys )

As long as we have `unlikely` and `likely` in the codebase, I
personally prefer us to use it where applicable.

> +        return VLC_ENOMEM;
> +
> +    filter_owner_t owner = {
> +        .sys = p_filter,
> +        .video = {
> +            .buffer_new = BufferNew,
> +        },
> +    };
> +
> +    p_sys->p_chain = filter_chain_NewVideo( p_filter, false, &owner );
> +    if( !p_sys->p_chain )
> +    {
> +        free( p_sys );
> +        return VLC_EGENERIC;
> +    }
> +
> +    int type = VLC_VAR_INTEGER;
> +    if( var_Type( p_filter->obj.parent, RECURSIVE_STRING ) != 0 )
> +        type |= VLC_VAR_DOINHERIT;
> +
> +    var_Create( p_filter, RECURSIVE_STRING, type );
> +    /* Note: atomicity is not actually needed here. */
> +    var_IncInteger( p_filter, RECURSIVE_STRING );
> +
> +    int level = var_GetInteger( p_filter, RECURSIVE_STRING );
> +    if( level < 0 || level > CHAIN_LEVEL_MAX )
> +        msg_Err( p_filter, "Too high level of recursion (%d)", level );
> +    else
> +        i_ret = BuildShortcutChain( p_filter );
> +
> +    if( i_ret )

`i_ret` is potentially uninitialized (on error).

> +    {
> +        /* Hum ... looks like this really isn't going to work. Too bad. */
> +        var_Destroy( p_filter, RECURSIVE_STRING );
> +        filter_chain_Delete( p_sys->p_chain );
> +        free( p_sys );
> +        return VLC_EGENERIC;
> +    }
> +
> +    if ( p_filter->b_allow_fmt_out_change )
> +    {
> +        es_format_Clean( &p_filter->fmt_out );
> +        es_format_Copy( &p_filter->fmt_out, filter_chain_GetFmtOut(p_sys->p_chain) );
> +    }
> +
> +    /* */
> +    p_filter->pf_video_filter = Chain;
> +    return VLC_SUCCESS;
> +}
> +
> +static void Destroy( vlc_object_t *p_this )
> +{
> +    filter_t *p_filter = (filter_t *)p_this;
> +
> +    var_Destroy( p_filter, RECURSIVE_STRING );
> +    filter_chain_Delete( p_filter->p_sys->p_chain );
> +    free( p_filter->p_sys );
> +}
> +
> +/*****************************************************************************
> + * Module descriptor
> + *****************************************************************************/
> +vlc_module_begin ()
> +    set_description( N_("Video filter using a chain of video converter modules") )
> +    set_capability( "video filter", 1 )
> +    set_callbacks( Activate, Destroy )
> +vlc_module_end ()
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170510/502a0683/attachment.html>


More information about the vlc-devel mailing list