[vlc-devel] [PATCH 2/2] vout: update interlacing handling

Hugo Beauzée-Luyssen hugo at beauzee.fr
Tue Apr 25 13:04:12 CEST 2017


Hi,

On Tue, Apr 25, 2017, at 01:38 PM, Victorien Le Couviour--Tuffet wrote:
> Remove deinterlace from 'video-filter' vlc string variable, now handling
> it
> with a boolean.
> This fixes the bug that happened when deinterlacing a video and applying
> a new
> filter: the deinterlace filter was not reapplied when rebuilding the
> filter
> chain.
> We couldn't retrieve this var from the GUI as the presence of this
> filter can change at any time, considering that a video can be partially
> interlaced many times.
> It's also cleaner to handle the presence of the deinterlace filter
> without
> using a VLC variable as we only need to know if it's there in the video
> output
> module.
> ---
>  src/video_output/control.h       |  1 +
>  src/video_output/interlacing.c   | 74
>  +++++-----------------------------------
>  src/video_output/video_output.c  | 27 ++++++++++++---
>  src/video_output/vout_internal.h |  1 +
>  4 files changed, 33 insertions(+), 70 deletions(-)
> 
> diff --git a/src/video_output/control.h b/src/video_output/control.h
> index 14eec3468d..f49208ab6a 100644
> --- a/src/video_output/control.h
> +++ b/src/video_output/control.h
> @@ -42,6 +42,7 @@ enum {
>      VOUT_CONTROL_FLUSH_SUBPICTURE,      /* integer */
>      VOUT_CONTROL_OSD_TITLE,             /* string */
>      VOUT_CONTROL_CHANGE_FILTERS,        /* string */
> +    VOUT_CONTROL_CHANGE_INTERLACE,      /* boolean */

This most likely requires an ABI bump

>      VOUT_CONTROL_CHANGE_SUB_SOURCES,    /* string */
>      VOUT_CONTROL_CHANGE_SUB_FILTERS,    /* string */
>      VOUT_CONTROL_CHANGE_SUB_MARGIN,     /* integer */
> diff --git a/src/video_output/interlacing.c
> b/src/video_output/interlacing.c
> index 308493e6ca..9fa6bedbb8 100644
> --- a/src/video_output/interlacing.c
> +++ b/src/video_output/interlacing.c
> @@ -62,77 +62,21 @@ static bool DeinterlaceIsModeValid(const char *mode)
>      return false;
>  }
>  
> -static char *FilterFind(char *filter_base, const char *module_name)
> -{
> -    const size_t module_length = strlen(module_name);
> -    const char *filter = filter_base;
> -
> -    if (!filter || module_length <= 0)
> -        return NULL;
> -
> -    for (;;) {
> -        char *start = strstr(filter, module_name);
> -        if (!start)
> -            return NULL;
> -        if (start[module_length] == '\0' || start[module_length] == ':')
> -            return start;
> -        filter = &start[module_length];
> -    }
> -}
> -
>  static bool DeinterlaceIsPresent(vout_thread_t *vout)
>  {
> -    char *filter = var_GetNonEmptyString(vout, "video-filter");
> -
> -    bool is_found = FilterFind(filter, "deinterlace") != NULL;
> -
> -    free(filter);
> -
> -    return is_found;
> +    return vout->p->filter.has_deint;
>  }
>  
> -static void DeinterlaceRemove(vout_thread_t *vout)
> +static inline void DeinterlaceRemove(vout_thread_t *vout)

Given that this function is now a one liner, and used once in the file,
I'd inline it completely.

>  {
> -    char *filter = var_GetNonEmptyString(vout, "video-filter");
> -
> -    char *start = FilterFind(filter, "deinterlace");
> -    if (!start) {
> -        free(filter);
> -        return;
> -    }
> -
> -    /* */
> -    strcpy(&start[0], &start[strlen("deinterlace")]);
> -    if (*start == ':')
> -        memmove(start, start + 1, strlen(start) /* + 1 - 1 */);
> -
> -    var_SetString(vout, "video-filter", filter);
> -    free(filter);
> +    vout_control_PushBool(&vout->p->control,
> +                          VOUT_CONTROL_CHANGE_INTERLACE, false);
>  }
> -static void DeinterlaceAdd(vout_thread_t *vout)
> -{
> -    char *filter = var_GetNonEmptyString(vout, "video-filter");
> -
> -    if (FilterFind(filter, "deinterlace")) {
> -        free(filter);
> -        return;
> -    }
> -
> -    /* */
> -    if (filter) {
> -        char *tmp = filter;
> -        if (asprintf(&filter, "%s:%s", tmp, "deinterlace") < 0)
> -            filter = tmp;
> -        else
> -            free(tmp);
> -    } else {
> -        filter = strdup("deinterlace");
> -    }
>  
> -    if (filter) {
> -        var_SetString(vout, "video-filter", filter);
> -        free(filter);
> -    }
> +static inline void DeinterlaceAdd(vout_thread_t *vout)

Same remark here

> +{
> +    vout_control_PushBool(&vout->p->control,
> +                          VOUT_CONTROL_CHANGE_INTERLACE, true);
>  }
>  
>  static int DeinterlaceCallback(vlc_object_t *object, char const *cmd,
> @@ -260,5 +204,3 @@ void vout_SetInterlacingState(vout_thread_t *vout,
> bool is_interlaced)
>      if (is_interlaced)
>          vout->p->interlacing.date = mdate();
>  }
> -
> -
> diff --git a/src/video_output/video_output.c
> b/src/video_output/video_output.c
> index 6ea72ba468..be29feb6f8 100644
> --- a/src/video_output/video_output.c
> +++ b/src/video_output/video_output.c
> @@ -212,6 +212,9 @@ static vout_thread_t *VoutCreate(vlc_object_t
> *object,
>      if (vout->p->input)
>          spu_Attach(vout->p->spu, vout->p->input, true);
>  
> +    /* */
> +    vout->p->filter.has_deint = false;
> +

I don't know the vout code/thread model enough to say for sure, but
shouldn't this be set before the vout thread is created?

>      return vout;
>  }
>  
> @@ -698,6 +701,7 @@ typedef struct {
>  static void ThreadChangeFilters(vout_thread_t *vout,
>                                  const video_format_t *source,
>                                  const char *filters,
> +                                int deinterlace,
>                                  bool is_locked)
>  {
>      ThreadFilterFlush(vout, is_locked);
> @@ -707,6 +711,18 @@ static void ThreadChangeFilters(vout_thread_t *vout,
>  
>      vlc_array_init(&array_static);
>      vlc_array_init(&array_interactive);
> +
> +    if ((vout->p->filter.has_deint =
> +         deinterlace == 1 || (deinterlace == -1 &&
> vout->p->filter.has_deint)))
> +    {
> +        vout_filter_t *e;
> +
> +        if (!(e = malloc(sizeof(*e))))
> +            return ;
> +        config_ChainCreate(&e->name, &e->cfg, "deinterlace");
> +        vlc_array_append(&array_static, e);
> +    }
> +
>      char *current = filters ? strdup(filters) : NULL;
>      while (current) {
>          config_chain_t *cfg;
> @@ -720,8 +736,7 @@ static void ThreadChangeFilters(vout_thread_t *vout,
>                  return ;
>              e->name = name;
>              e->cfg  = cfg;
> -            if (!strcmp(e->name, "deinterlace") ||
> -                !strcmp(e->name, "postproc")) {
> +            if (!strcmp(e->name, "postproc")) {
>                  vlc_array_append(&array_static, e);
>              } else {
>                  vlc_array_append(&array_interactive, e);
> @@ -820,7 +835,7 @@ static int ThreadDisplayPreparePicture(vout_thread_t
> *vout, bool reuse, bool fra
>                      }
>                  }
>                  if (!VideoFormatIsCropArEqual(&decoded->format,
>                  &vout->p->filter.format))
> -                    ThreadChangeFilters(vout, &decoded->format,
> vout->p->filter.configuration, true);
> +                    ThreadChangeFilters(vout, &decoded->format,
> vout->p->filter.configuration, -1, true);
>              }
>          }
>  
> @@ -1559,7 +1574,11 @@ static int ThreadControl(vout_thread_t *vout,
> vout_control_cmd_t cmd)
>          ThreadDisplayOsdTitle(vout, cmd.u.string);
>          break;
>      case VOUT_CONTROL_CHANGE_FILTERS:
> -        ThreadChangeFilters(vout, NULL, cmd.u.string, false);
> +        ThreadChangeFilters(vout, NULL, cmd.u.string, -1, false);
> +        break;
> +    case VOUT_CONTROL_CHANGE_INTERLACE:
> +        ThreadChangeFilters(vout, NULL, vout->p->filter.configuration,
> +                            cmd.u.boolean ? 1 : 0, false);
>          break;
>      case VOUT_CONTROL_CHANGE_SUB_SOURCES:
>          ThreadChangeSubSources(vout, cmd.u.string);
> diff --git a/src/video_output/vout_internal.h
> b/src/video_output/vout_internal.h
> index c7d33bafb6..400bcb8b61 100644
> --- a/src/video_output/vout_internal.h
> +++ b/src/video_output/vout_internal.h
> @@ -124,6 +124,7 @@ struct vout_thread_sys_t
>          video_format_t  format;
>          struct filter_chain_t *chain_static;
>          struct filter_chain_t *chain_interactive;
> +        bool            has_deint;
>      } filter;
>  
>      /* */
> -- 
> 2.12.0
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

Regards,

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr


More information about the vlc-devel mailing list