[vlc-devel] [PATCH 4/4] filters: add an operations structure to set the callbacks

Steve Lhomme robux4 at ycbcr.xyz
Tue Oct 6 09:46:33 CEST 2020


On 2020-10-06 9:14, Thomas Guillem wrote:
> This patch should be named: "VLC filters : A New Ops"
> 
> OK, sorrry, real review below...
> 
> On Mon, Oct 5, 2020, at 17:03, Steve Lhomme wrote:
>> From: Alexandre Janniaux <ajanni at videolabs.io>
>>
>> Generate a simple operations structure for filters generating their own filter
>> callback via VIDEO_FILTER_WRAPPER().
>>
>> The filter chain sets a mouse handler on video filters that don't have one,
>> just as before, by using a local version of the ops structure of the filter and
>> adding the local mouse callback.
>>
>> Co-authored-by: Steve Lhomme <robux4 at ycbcr.xyz>
>> ---
>>   include/vlc_filter.h                          | 124 ++++++++++--------
>>   modules/access/screen/screen.c                |   2 +-
>>   modules/arm_neon/chroma_yuv.c                 |  44 +++----
>>   modules/arm_neon/yuv_rgb.c                    |  10 +-
>>   modules/audio_filter/audiobargraph_a.c        |   7 +-
>>   modules/audio_filter/center.c                 |   7 +-
>>   modules/audio_filter/channel_mixer/dolby.c    |   6 +-
>>   .../audio_filter/channel_mixer/headphone.c    |   7 +-
>>   modules/audio_filter/channel_mixer/mono.c     |   7 +-
>>   modules/audio_filter/channel_mixer/remap.c    |   6 +-
>>   modules/audio_filter/channel_mixer/simple.c   |   8 +-
>>   .../channel_mixer/spatialaudio.cpp            |  18 ++-
>>   modules/audio_filter/channel_mixer/trivial.c  |  24 +++-
>>   modules/audio_filter/chorus_flanger.c         |  11 +-
>>   modules/audio_filter/compressor.c             |   7 +-
>>   modules/audio_filter/converter/format.c       |  14 +-
>>   modules/audio_filter/converter/tospdif.c      |   8 +-
>>   modules/audio_filter/equalizer.c              |   7 +-
>>   modules/audio_filter/gain.c                   |   6 +-
>>   modules/audio_filter/karaoke.c                |   7 +-
>>   modules/audio_filter/libebur128.c             |   7 +-
>>   modules/audio_filter/normvol.c                |   6 +-
>>   modules/audio_filter/param_eq.c               |  20 +--
>>   modules/audio_filter/resampler/bandlimited.c  |   6 +-
>>   modules/audio_filter/resampler/soxr.c         |  10 +-
>>   modules/audio_filter/resampler/speex.c        |   6 +-
>>   modules/audio_filter/resampler/src.c          |   7 +-
>>   modules/audio_filter/resampler/ugly.c         |   6 +-
>>   modules/audio_filter/scaletempo.c             |  14 +-
>>   .../audio_filter/spatializer/spatializer.cpp  |  12 +-
>>   modules/audio_filter/stereo_widen.c           |   6 +-
>>   modules/hw/d3d11/d3d11_deinterlace.c          |   7 +-
>>   modules/hw/d3d11/d3d11_filters.c              |   6 +-
>>   modules/hw/d3d11/d3d11_surface.c              |  20 +--
>>   modules/hw/d3d9/d3d9_filters.c                |   6 +-
>>   modules/hw/d3d9/dxa9.c                        |  16 ++-
>>   modules/hw/d3d9/dxva2_deinterlace.c           |   7 +-
>>   modules/hw/mmal/converter.c                   |   7 +-
>>   modules/hw/mmal/deinterlace.c                 |  14 +-
>>   modules/hw/nvdec/chroma.c                     |   6 +-
>>   modules/hw/vaapi/chroma.c                     |  10 +-
>>   modules/hw/vaapi/filters.c                    |  24 +++-
>>   modules/hw/vdpau/adjust.c                     |   6 +-
>>   modules/hw/vdpau/chroma.c                     |  23 +++-
>>   modules/hw/vdpau/deinterlace.c                |   6 +-
>>   modules/hw/vdpau/sharpen.c                    |   6 +-
>>   modules/spu/audiobargraph_v.c                 |  12 +-
>>   modules/spu/dynamicoverlay/dynamicoverlay.c   |   6 +-
>>   modules/spu/logo.c                            |  18 ++-
>>   modules/spu/marq.c                            |   6 +-
>>   modules/spu/mosaic.c                          |   6 +-
>>   modules/spu/rss.c                             |   6 +-
>>   modules/spu/subsdelay.c                       |   6 +-
>>   modules/text_renderer/freetype/freetype.c     |   7 +-
>>   modules/text_renderer/nsspeechsynthesizer.m   |   6 +-
>>   modules/text_renderer/sapi.cpp                |  10 +-
>>   modules/text_renderer/svg.c                   |   6 +-
>>   modules/text_renderer/tdummy.c                |   6 +-
>>   modules/video_chroma/chain.c                  |  15 ++-
>>   modules/video_chroma/cvpx.c                   |  28 +++-
>>   modules/video_chroma/grey_yuv.c               |  29 ++--
>>   modules/video_chroma/i420_nv12.c              |  12 +-
>>   modules/video_chroma/i420_rgb.c               |  18 +--
>>   modules/video_chroma/i420_yuy2.c              |  59 +++++----
>>   modules/video_chroma/i422_i420.c              |   6 +-
>>   modules/video_chroma/i422_yuy2.c              |  69 +++++-----
>>   modules/video_chroma/omxdl.c                  |  52 ++++----
>>   modules/video_chroma/rv32.c                   |   6 +-
>>   modules/video_chroma/swscale.c                |   6 +-
>>   modules/video_chroma/yuvp.c                   |   2 +-
>>   modules/video_chroma/yuy2_i420.c              |   6 +-
>>   modules/video_chroma/yuy2_i422.c              |   6 +-
>>   modules/video_filter/adjust.c                 |  16 ++-
>>   modules/video_filter/alphamask.c              |   7 +-
>>   modules/video_filter/anaglyph.c               |   8 +-
>>   modules/video_filter/antiflicker.c            |   7 +-
>>   modules/video_filter/ball.c                   |   6 +-
>>   modules/video_filter/blend.cpp                |  13 +-
>>   modules/video_filter/blendbench.c             |  12 +-
>>   modules/video_filter/bluescreen.c             |   7 +-
>>   modules/video_filter/canvas.c                 |   7 +-
>>   modules/video_filter/ci_filters.m             |  14 +-
>>   modules/video_filter/colorthres.c             |  14 +-
>>   modules/video_filter/croppadd.c               |   7 +-
>>   .../video_filter/deinterlace/deinterlace.c    |  12 +-
>>   modules/video_filter/edgedetection.c          |   7 +-
>>   modules/video_filter/erase.c                  |   7 +-
>>   modules/video_filter/extract.c                |   7 +-
>>   modules/video_filter/fps.c                    |   7 +-
>>   modules/video_filter/freeze.c                 |   9 +-
>>   modules/video_filter/gaussianblur.c           |   7 +-
>>   modules/video_filter/gradfun.c                |  10 +-
>>   modules/video_filter/gradient.c               |   7 +-
>>   modules/video_filter/grain.c                  |  10 +-
>>   modules/video_filter/hqdn3d.c                 |   7 +-
>>   modules/video_filter/invert.c                 |   6 +-
>>   modules/video_filter/magnify.c                |   9 +-
>>   modules/video_filter/mirror.c                 |   6 +-
>>   modules/video_filter/motionblur.c             |   6 +-
>>   modules/video_filter/motiondetect.c           |   6 +-
>>   modules/video_filter/oldmovie.c               |   6 +-
>>   modules/video_filter/opencv_example.cpp       |  12 +-
>>   modules/video_filter/opencv_wrapper.c         |   9 +-
>>   modules/video_filter/posterize.c              |   6 +-
>>   modules/video_filter/postproc.c               |   6 +-
>>   modules/video_filter/psychedelic.c            |   6 +-
>>   modules/video_filter/puzzle.c                 |   8 +-
>>   modules/video_filter/ripple.c                 |   7 +-
>>   modules/video_filter/rotate.c                 |  14 +-
>>   modules/video_filter/scale.c                  |   7 +-
>>   modules/video_filter/scene.c                  |   8 +-
>>   modules/video_filter/sepia.c                  |   6 +-
>>   modules/video_filter/sharpen.c                |   6 +-
>>   modules/video_filter/transform.c              |   8 +-
>>   modules/video_filter/vhs.c                    |   6 +-
>>   modules/video_filter/wave.c                   |   6 +-
>>   modules/visualization/glspectrum.c            |   6 +-
>>   modules/visualization/goom.c                  |   8 +-
>>   modules/visualization/projectm.cpp            |  10 +-
>>   modules/visualization/visual/visual.c         |   8 +-
>>   modules/visualization/vsxu.cpp                |  10 +-
>>   src/audio_output/filters.c                    |   6 +-
>>   src/audio_output/meter.c                      |   8 +-
>>   src/misc/filter.c                             |   2 +-
>>   src/misc/filter_chain.c                       |  10 +-
>>   src/misc/image.c                              |   6 +-
>>   src/video_output/vout_subpictures.c           |   6 +-
>>   127 files changed, 1007 insertions(+), 438 deletions(-)
>>
>> diff --git a/include/vlc_filter.h b/include/vlc_filter.h
>> index d9c274a645a..0ced8ea03d6 100644
>> --- a/include/vlc_filter.h
>> +++ b/include/vlc_filter.h
>> @@ -79,68 +79,44 @@ typedef struct filter_owner_t
>>   
>>   struct vlc_mouse_t;
>>   
>> -/** Structure describing a filter
>> - * @warning BIG FAT WARNING : the code relies on the first 4 members of
>> - * filter_t and decoder_t to be the same, so if you have anything to add,
>> - * do it at the end of the structure.
>> - */
>> -struct filter_t
>> +struct vlc_filter_operations
>>   {
>> -    struct vlc_object_t obj;
>> -
>> -    /* Module properties */
>> -    module_t *          p_module;
>> -    void               *p_sys;
>> -
>> -    /* Input format */
>> -    es_format_t         fmt_in;
>> -    vlc_video_context   *vctx_in;  // video filter, set by owner
>> -
>> -    /* Output format of filter */
>> -    es_format_t         fmt_out;
>> -    vlc_video_context   *vctx_out; // video filter, handled by the filter
>> -    bool                b_allow_fmt_out_change;
>> -
>> -    /* Name of the "video filter" shortcut that is requested, can be NULL */
>> -    const char *        psz_name;
>> -    /* Filter configuration */
>> -    config_chain_t *    p_cfg;
>> -
>> +    /* Operation depending on the type of filter. */
>>       union
>>       {
>>           /** Filter a picture (video filter) */
>> -        picture_t * (*pf_video_filter)( filter_t *, picture_t * );
>> +        picture_t * (*filter_video)(filter_t *, picture_t *);
>>   
>>           /** Filter an audio block (audio filter) */
>> -        block_t * (*pf_audio_filter)( filter_t *, block_t * );
>> +        block_t * (*filter_audio)(filter_t *, block_t *);
>>   
>>           /** Blend a subpicture onto a picture (blend) */
>> -        void (*pf_video_blend)( filter_t *,  picture_t *, const picture_t *,
>> -                                 int, int, int );
>> +        void (*blend_video)(filter_t *,  picture_t *, const picture_t *,
>> +                            int, int, int);
>>   
>>           /** Generate a subpicture (sub source) */
>> -        subpicture_t *(*pf_sub_source)( filter_t *, vlc_tick_t );
>> +        subpicture_t *(*source_sub)(filter_t *, vlc_tick_t);
>>   
>>           /** Filter a subpicture (sub filter) */
>> -        subpicture_t *(*pf_sub_filter)( filter_t *, subpicture_t * );
>> +        subpicture_t *(*filter_sub)(filter_t *, subpicture_t *);
>>   
>>           /** Render text (text render) */
>> -        int (*pf_render)( filter_t *, subpicture_region_t *,
>> -                          subpicture_region_t *, const vlc_fourcc_t * );
>> +        int (*render)(filter_t *, subpicture_region_t *,
>> +                      subpicture_region_t *, const vlc_fourcc_t *);
>>       };
>>   
>>       union
>>       {
>>           /* TODO: video filter drain */
>>           /** Drain (audio filter) */
>> -        block_t *(*pf_audio_drain) ( filter_t * );
>> +        block_t *(*drain_audio)(filter_t *);
>>       };
>>   
>>       /** Flush
>>        *
>>        * Flush (i.e. discard) any internal buffer in a video or audio filter.
>>        */
>> -    void (*pf_flush)( filter_t * );
>> +    void (*flush)(filter_t *);
>>   
>>       /** Change viewpoint
>>        *
>> @@ -148,20 +124,49 @@ struct filter_t
>>        * used for Ambisonics rendering will change its output according to this
>>        * viewpoint.
>>        */
>> -    void (*pf_change_viewpoint)( filter_t *, const vlc_viewpoint_t * );
>> +    void (*change_viewpoint)(filter_t *, const vlc_viewpoint_t *);
>>   
>> -    union
>> -    {
>> -        /** Filter mouse state (video filter).
>> -         *
>> -         * If non-NULL, you must convert from output to input formats:
>> -         * - If VLC_SUCCESS is returned, the mouse state is propagated.
>> -         * - Otherwise, the mouse change is not propagated.
>> -         * If NULL, the mouse state is considered unchanged and will be
>> -         * propagated. */
>> -        int (*pf_video_mouse)( filter_t *, struct vlc_mouse_t *,
>> -                               const struct vlc_mouse_t *p_old);
>> -    };
>> +    /** Filter mouse state (video filter).
>> +     *
>> +     * If non-NULL, you must convert from output to input formats:
>> +     * - If VLC_SUCCESS is returned, the mouse state is propagated.
>> +     * - Otherwise, the mouse change is not propagated.
>> +     * If NULL, the mouse state is considered unchanged and will be
>> +     * propagated. */
>> +    int (*video_mouse)(filter_t *, struct vlc_mouse_t *,
>> +                       const struct vlc_mouse_t *p_old);
>> +
>> +};
>> +
>> +/** Structure describing a filter
>> + * @warning BIG FAT WARNING : the code relies on the first 4 members of
>> + * filter_t and decoder_t to be the same, so if you have anything to add,
>> + * do it at the end of the structure.
>> + */
>> +struct filter_t
>> +{
>> +    struct vlc_object_t obj;
>> +
>> +    /* Module properties */
>> +    module_t *          p_module;
>> +    void               *p_sys;
>> +
>> +    /* Input format */
>> +    es_format_t         fmt_in;
>> +    vlc_video_context   *vctx_in;  // video filter, set by owner
>> +
>> +    /* Output format of filter */
>> +    es_format_t         fmt_out;
>> +    vlc_video_context   *vctx_out; // video filter, handled by the filter
>> +    bool                b_allow_fmt_out_change;
>> +
>> +    /* Name of the "video filter" shortcut that is requested, can be NULL */
>> +    const char *        psz_name;
>> +    /* Filter configuration */
>> +    config_chain_t *    p_cfg;
>> +
>> +    /* Implementation of filter API */
>> +    const struct vlc_filter_operations *ops;
>>   
>>       /** Private structure for the owner of the filter */
>>       filter_owner_t      owner;
>> @@ -170,7 +175,7 @@ struct filter_t
>>   /**
>>    * This function will return a new picture usable by p_filter as an
>> output
>>    * buffer. You have to release it using picture_Release or by returning
>> - * it to the caller as a pf_video_filter return value.
>> + * it to the caller as a ops->filter_video return value.
>>    * Provided for convenience.
>>    *
>>    * \param p_filter filter_t object
>> @@ -198,15 +203,15 @@ static inline picture_t *filter_NewPicture(
>> filter_t *p_filter )
>>    */
>>   static inline void filter_Flush( filter_t *p_filter )
>>   {
>> -    if( p_filter->pf_flush != NULL )
>> -        p_filter->pf_flush( p_filter );
>> +    if( p_filter->ops && p_filter->ops->flush != NULL )
> 
> filters->ops should be mandatory no ?

I think that's the case for other operations structures. So I say yes.


More information about the vlc-devel mailing list