[vlc-devel] [PATCH 1/2] opencv_wrapper: Port to video filter2

Rémi Denis-Courmont remi at remlab.net
Tue Aug 7 22:30:56 CEST 2012


Le mardi 7 août 2012 19:32:38 Edward Wang, vous avez écrit :
>  static int Create( vlc_object_t *p_this )
>  {
> -    vout_thread_t *p_vout = (vout_thread_t *)p_this;
> +    filter_t* p_filter = (filter_t*)p_this;
>      char *psz_chroma, *psz_output, *psz_verbosity;
>      int i = 0;
> 
>      /* Allocate structure */
> -    p_vout->p_sys = malloc( sizeof( vout_sys_t ) );
> -    if( p_vout->p_sys == NULL )
> +    p_filter->p_sys = malloc( sizeof( filter_sys_t ) );
> +    if( p_filter->p_sys == NULL )
>          return VLC_ENOMEM;
> 
>      /* Init structure */
> -    p_vout->p_sys->p_image = image_HandlerCreate( p_vout );
> +    p_filter->p_sys->p_image = image_HandlerCreate( p_filter );
>      for (i = 0; i < VOUT_MAX_PLANES; i++)
> -        p_vout->p_sys->p_cv_image[i] = NULL;
> -    p_vout->p_sys->p_proc_image = NULL;
> -    p_vout->p_sys->p_to_be_freed = NULL;
> -    p_vout->p_sys->i_cv_image_size = 0;
> -
> -    p_vout->pf_init = Init;
> -    p_vout->pf_end = End;
> -    p_vout->pf_manage = NULL;
> -    p_vout->pf_render = Render;
> -    p_vout->pf_display = NULL;
> -    p_vout->pf_control = Control;
> +        p_filter->p_sys->p_cv_image[i] = NULL;

Cosmetic bug.

> +    p_filter->p_sys->p_proc_image = NULL;
> +    p_filter->p_sys->p_to_be_freed = NULL;
> +    p_filter->p_sys->i_cv_image_size = 0;
> +
> +    p_filter->pf_video_filter = Filter;
> 
>      /* Retrieve and apply config */
> -    psz_chroma = var_InheritString( p_vout, "opencv-chroma" );
> +    psz_chroma = var_InheritString( p_filter, "opencv-chroma" );
>      if( psz_chroma == NULL )
>      {
> -        msg_Err( p_vout, "configuration variable %s empty, using 'grey'",
> +        msg_Err( p_filter, "configuration variable %s empty, using
> 'grey'", "opencv-chroma" );
> -        p_vout->p_sys->i_internal_chroma = GREY;
> +        p_filter->p_sys->i_internal_chroma = GREY;
>      }
>      else
>      {

No objections but the old code could be cascaded:

if( psz_chroma == NULL ) { ... } else if( !strcmp( ... ) ) ... else ...

>          if( !strcmp( psz_chroma, "input" ) )
> -            p_vout->p_sys->i_internal_chroma = CINPUT;
> +            p_filter->p_sys->i_internal_chroma = CINPUT;
>          else if( !strcmp( psz_chroma, "I420" ) )
> -            p_vout->p_sys->i_internal_chroma = GREY;
> +            p_filter->p_sys->i_internal_chroma = GREY;
>          else if( !strcmp( psz_chroma, "RGB32" ) )
> -            p_vout->p_sys->i_internal_chroma = RGB;
> +            p_filter->p_sys->i_internal_chroma = RGB;
>          else
>          {
> -            msg_Err( p_vout, "no valid opencv-chroma provided, using
> 'grey'" ); -            p_vout->p_sys->i_internal_chroma = GREY;
> +            msg_Err( p_filter, "no valid opencv-chroma provided, using
> 'grey'" ); +            p_filter->p_sys->i_internal_chroma = GREY;
>          }
>      }
>      free( psz_chroma);
> 
> -    psz_output = var_InheritString( p_vout, "opencv-output" );
> +    psz_output = var_InheritString( p_filter, "opencv-output" );
>      if( psz_output == NULL )
>      {
> -        msg_Err( p_vout, "configuration variable %s empty, using 'input'",
> +        msg_Err( p_filter, "configuration variable %s empty, using
> 'input'", "opencv-output" );
> -        p_vout->p_sys->i_wrapper_output = VINPUT;
> +        p_filter->p_sys->i_wrapper_output = VINPUT;
>      }
>      else
>      {
>          if( !strcmp( psz_output, "none" ) )
> -            p_vout->p_sys->i_wrapper_output = NONE;
> +            p_filter->p_sys->i_wrapper_output = NONE;
>          else if( !strcmp( psz_output, "input" ) )
> -            p_vout->p_sys->i_wrapper_output = VINPUT;
> +            p_filter->p_sys->i_wrapper_output = VINPUT;
>          else if( !strcmp( psz_output, "processed" ) )
> -            p_vout->p_sys->i_wrapper_output = PROCESSED;
> +            p_filter->p_sys->i_wrapper_output = PROCESSED;
>          else
>          {
> -            msg_Err( p_vout, "no valid opencv-output provided, using
> 'input'" ); -            p_vout->p_sys->i_wrapper_output = VINPUT;
> +            msg_Err( p_filter, "no valid opencv-output provided, using
> 'input'" ); +            p_filter->p_sys->i_wrapper_output = VINPUT;
>          }
>      }
>      free( psz_output);
> 
> -    psz_verbosity = var_InheritString( p_vout, "opencv-verbosity" );
> +    psz_verbosity = var_InheritString( p_filter, "opencv-verbosity" );
>      if( psz_verbosity == NULL )
>      {
> -        msg_Err( p_vout, "configuration variable %s empty, using 'input'",
> +        msg_Err( p_filter, "configuration variable %s empty, using
> 'input'", "opencv-verbosity" );
> -        p_vout->p_sys->i_verbosity = VERB_ERROR;
> +        p_filter->p_sys->i_verbosity = VERB_ERROR;
>      }
>      else
>      {
>          if( !strcmp( psz_verbosity, "error" ) )
> -            p_vout->p_sys->i_verbosity = VERB_ERROR;
> +            p_filter->p_sys->i_verbosity = VERB_ERROR;
>          else if( !strcmp( psz_verbosity, "warning" ) )
> -            p_vout->p_sys->i_verbosity = VERB_WARN;
> +            p_filter->p_sys->i_verbosity = VERB_WARN;
>          else if( !strcmp( psz_verbosity, "debug" ) )
> -            p_vout->p_sys->i_verbosity = VERB_DEBUG;
> +            p_filter->p_sys->i_verbosity = VERB_DEBUG;
>          else
>          {
> -            msg_Err( p_vout, "no valid opencv-verbosity provided, using
> 'error'" ); -            p_vout->p_sys->i_verbosity = VERB_ERROR;
> +            msg_Err( p_filter, "no valid opencv-verbosity provided, using
> 'error'" ); +            p_filter->p_sys->i_verbosity = VERB_ERROR;
>          }
>      }
>      free( psz_verbosity);

Same here.

> -    p_vout->p_sys->psz_inner_name =
> -        var_InheritString( p_vout, "opencv-filter-name" );
> -    p_vout->p_sys->f_scale =
> -        var_InheritFloat( p_vout, "opencv-scale" );
> +    p_filter->p_sys->psz_inner_name =
> +        var_InheritString( p_filter, "opencv-filter-name" );
> +    p_filter->p_sys->f_scale =
> +        var_InheritFloat( p_filter, "opencv-scale" );
> 
> -    if (p_vout->p_sys->i_verbosity > VERB_WARN)
> -        msg_Info(p_vout, "Configuration: opencv-scale: %f, opencv-chroma:
> %d, " +    if (p_filter->p_sys->i_verbosity > VERB_WARN)
> +        msg_Info(p_filter, "Configuration: opencv-scale: %f,
> opencv-chroma: %d, " "opencv-output: %d, opencv-verbosity %d,
> opencv-filter %s", -            p_vout->p_sys->f_scale,
> -            p_vout->p_sys->i_internal_chroma,
> -            p_vout->p_sys->i_wrapper_output,
> -            p_vout->p_sys->i_verbosity,
> -            p_vout->p_sys->psz_inner_name);
> -
> -    return VLC_SUCCESS;
> -}
> -
> -/*************************************************************************
> **** - * Init: initialize opencv_wrapper video thread output method
> -
> **************************************************************************
> ***/ -static int Init( vout_thread_t *p_vout )
> -{
> -    video_format_t fmt;
> -    vout_sys_t *p_sys = p_vout->p_sys;
> -    I_OUTPUTPICTURES = 0;
> -
> -    /* Initialize the output video format */
> -    memset( &fmt, 0, sizeof(video_format_t) );
> -    p_vout->output.i_chroma = p_vout->render.i_chroma;
> -    p_vout->output.i_width  = p_vout->render.i_width;
> -    p_vout->output.i_height = p_vout->render.i_height;
> -    p_vout->output.i_aspect = p_vout->render.i_aspect;
> -    p_vout->fmt_out = p_vout->fmt_in;           //set to input video
> format -
> -    fmt = p_vout->fmt_out;
> -    if (p_sys->i_wrapper_output == PROCESSED)   //set to processed video
> format -    {
> -        fmt.i_width = fmt.i_width * p_sys->f_scale;
> -        fmt.i_height = fmt.i_height * p_sys->f_scale;
> -        fmt.i_visible_width = fmt.i_visible_width * p_sys->f_scale;
> -        fmt.i_visible_height = fmt.i_visible_height * p_sys->f_scale;
> -        fmt.i_x_offset = fmt.i_x_offset * p_sys->f_scale;
> -        fmt.i_y_offset = fmt.i_y_offset * p_sys->f_scale;
> -
> -        if (p_sys->i_internal_chroma == GREY)
> -            fmt.i_chroma = VLC_CODEC_I420;
> -        else if (p_sys->i_internal_chroma == RGB)
> -            fmt.i_chroma = VLC_CODEC_RGB32;
> -    }
> +            p_filter->p_sys->f_scale,
> +            p_filter->p_sys->i_internal_chroma,
> +            p_filter->p_sys->i_wrapper_output,
> +            p_filter->p_sys->i_verbosity,
> +            p_filter->p_sys->psz_inner_name);
> 
>      /* Load the internal opencv filter */
>      /* We don't need to set up video formats for this filter as it not
> actually using a picture_t */
> -    p_sys->p_opencv = vlc_object_create(
> p_vout, sizeof(filter_t) );
> +    p_filter->p_sys->p_opencv =
> vlc_object_create( p_filter, sizeof(filter_t) );

I don´t see why a VLC filter object would be needed with another VLC filter 
object.

> 
> -    if (p_vout->p_sys->psz_inner_name)
> -        p_sys->p_opencv->p_module =
> -            module_need( p_sys->p_opencv, p_sys->psz_inner_name, NULL,
> false );
> +    if (p_filter->p_sys->psz_inner_name)
> +        p_filter->p_sys->p_opencv->p_module =
> +            module_need( p_filter->p_sys->p_opencv,
> p_filter->p_sys->psz_inner_name, NULL, false );
> 
> -    if( !p_sys->p_opencv->p_module )
> +    if( !p_filter->p_sys->p_opencv->p_module )
>      {
> -        msg_Err( p_vout, "can't open internal opencv filter: %s",
> p_vout->p_sys->psz_inner_name ); -        p_vout->p_sys->psz_inner_name =
> NULL;
> -        vlc_object_release( p_sys->p_opencv );
> -        p_sys->p_opencv = NULL;
> +        msg_Err( p_filter, "can't open internal opencv filter: %s",
> p_filter->p_sys->psz_inner_name ); +       
> p_filter->p_sys->psz_inner_name = NULL;
> +        vlc_object_release( p_filter->p_sys->p_opencv );
> +        p_filter->p_sys->p_opencv = NULL;

It looks like the error handling is missing here.

>      }
> 
>      /* Try to open the real video output */
> -    if (p_sys->i_verbosity > VERB_WARN)
> -        msg_Dbg( p_vout, "spawning the real video output" );
> -
> -    p_vout->p_sys->p_vout = vout_Create( p_vout, &fmt );
> -
> -    /* Everything failed */
> -    if( p_vout->p_sys->p_vout == NULL )
> -    {
> -        msg_Err( p_vout, "can't open vout, aborting" );
> -        return VLC_EGENERIC;
> -    }
> -
> -    vout_filter_AllocateDirectBuffers( p_vout, VOUT_MAX_PICTURES );
> -
> -    vout_filter_AddChild( p_vout, p_vout->p_sys->p_vout, NULL );
> +    if (p_filter->p_sys->i_verbosity > VERB_WARN)
> +        msg_Dbg( p_filter, "spawning the real video output" );
> 
>      return VLC_SUCCESS;
>  }
(...)
>  static void Destroy( vlc_object_t *p_this )
>  {
> -    vout_thread_t *p_vout = (vout_thread_t *)p_this;
> +    filter_t* p_filter = (filter_t*)p_this;
> +    ReleaseImages( p_filter );
> 
> -    ReleaseImages(p_vout);
> -
> -    if( p_vout->p_sys->p_image )
> -        image_HandlerDelete( p_vout->p_sys->p_image );
> +    if( p_filter->p_sys->p_opencv )

I don't see the point of dealing with this failure case so late.

> +    {
> +        // Release the internal OpenCV filter.
> +        if( p_filter->p_sys->p_opencv->p_module )
> +            module_unneed( p_filter->p_sys->p_opencv,
> p_filter->p_sys->p_opencv->p_module ); +        vlc_object_release(
> p_filter->p_sys->p_opencv );
> +        p_filter->p_sys->p_opencv = NULL;
> +    }
> 
> -    free( p_vout->p_sys );
> +    free( p_filter->p_sys );
>  }
(...)


-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis



More information about the vlc-devel mailing list