[vlc-devel] [PATCH] Enable/Disable/Auto deinterlace functions for libvlc

Thomas Guillem thomas at gllm.fr
Fri May 4 17:34:43 CEST 2018


On Fri, May 4, 2018, at 16:53, Luís Fernandes wrote:
> Hi,
> 
> Here’s the patch without fetching the config.
> 
> @Thomas as a side note, I did only add the "fetching of the default
> deinterlace value" on your suggestion in the VLCKit mailing list ;)
Yes sorry about that, but since you can now set the "auto" mode with
your patches, I don't think it's useful anymore.
> 
> @Caro as soon as this is accepted I’ll update the VLCKit patch!
> 
> Cheers,
> Luís
> 
> 
> From effa894f8231379841f7c387a6931064fcca929d Mon Sep 17 00:00:00 2001> From: Luis Fernandes <zipleen at gmail.com>
> Date: Mon, 30 Apr 2018 14:33:08 +0100
> Subject: [PATCH] add auto deinterlacer-mode which is also valid
> 
> ---
>  lib/video.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/video.c b/lib/video.c
> index 035cc0ebf1..a3e5b248e9 100644
> --- a/lib/video.c
> +++ b/lib/video.c
> @@ -675,7 +675,8 @@ void libvlc_video_set_deinterlace(
> libvlc_media_player_t *p_mi,>       && strcmp (psz_mode, "discard")  && strcmp (psz_mode, "linear")>       && strcmp (psz_mode, "mean")     && strcmp (psz_mode, "x")
>       && strcmp (psz_mode, "yadif")    && strcmp (psz_mode, "yadif2x")> -     && strcmp (psz_mode, "phosphor") && strcmp (psz_mode, "ivtc"))
> +     && strcmp (psz_mode, "phosphor") && strcmp (psz_mode, "ivtc")
> +     && strcmp (psz_mode, "auto"))
>          return;
> 
>      if (*psz_mode)
> --
> 2.14.3 (Apple Git-98)
> 
> 
> From 71ea2190622c64e4adfe03a4131a17861f0ffe8f Mon Sep 17 00:00:00 2001> From: Luis Fernandes <zipleen at gmail.com>
> Date: Fri, 4 May 2018 15:42:56 +0100
> Subject: [PATCH] change function of deinterlace to specify which
> deinterlace>  mode and filter we want to use
> 
> ---
>  include/vlc/libvlc_media_player.h |  5 ++++-
>  lib/video.c                       | 29 +++++++++++++++-------------->  2 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/include/vlc/libvlc_media_player.h
> b/include/vlc/libvlc_media_player.h> index 20b220448b..a82f70f5e7 100644
> --- a/include/vlc/libvlc_media_player.h
> +++ b/include/vlc/libvlc_media_player.h
> @@ -1419,9 +1419,12 @@ int libvlc_video_take_snapshot(
> libvlc_media_player_t *p_mi, unsigned num,>   * Enable or disable deinterlace filter
>   *
>   * \param p_mi libvlc media player
> - * \param psz_mode type of deinterlace filter, NULL to disable
> + * \param deinterlace deinterlace mode -1:auto (default), 0:
>   disabled, 1: enabled> + * \param psz_mode type of deinterlace filter, NULL to default
> + * \version LibVLC 4.0.0 and later
>   */
>  LIBVLC_API void libvlc_video_set_deinterlace(
>  libvlc_media_player_t *p_mi,> +                                                  int deinterlace,
>                                                    const char
>                                                    *psz_mode );> 
>  /**
> diff --git a/lib/video.c b/lib/video.c
> index a3e5b248e9..f23320a528 100644
> --- a/lib/video.c
> +++ b/lib/video.c
> @@ -663,13 +663,20 @@ end:
>  }
> 
>  /*******************************************************************-
>  ***********> - * libvlc_video_set_deinterlace : enable deinterlace
> + * libvlc_video_set_deinterlace : enable/disable/auto deinterlace and
>   filter>   *******************************************************************-
>   **********/> -void libvlc_video_set_deinterlace( libvlc_media_player_t *p_mi,
> +void libvlc_video_set_deinterlace( libvlc_media_player_t *p_mi, int
> deinterlace,>                                     const char *psz_mode )
>  {
> +    if (deinterlace != 0 && deinterlace != 1)
> +        deinterlace = -1;
> +
>      if (psz_mode == NULL)
> +    {
>          psz_mode = "";
> +        deinterlace = 0;

Why forcing deinterlace to zero in that case ?

> +    }
> +
>      if (*psz_mode
>       && strcmp (psz_mode, "blend")    && strcmp (psz_mode, "bob")
>       && strcmp (psz_mode, "discard")  && strcmp (psz_mode, "linear")> @@ -679,13 +686,10 @@ void libvlc_video_set_deinterlace(
> libvlc_media_player_t *p_mi,>       && strcmp (psz_mode, "auto"))
>          return;
> 
> -    if (*psz_mode)
> -    {
> +    if (*psz_mode && deinterlace != 0)
>          var_SetString (p_mi, "deinterlace-mode", psz_mode);
> -        var_SetInteger (p_mi, "deinterlace", 1);
> -    }
> -    else
> -        var_SetInteger (p_mi, "deinterlace", 0);
> +
> +    var_SetInteger (p_mi, "deinterlace", deinterlace);
> 
>      size_t n;
>      vout_thread_t **pp_vouts = GetVouts (p_mi, &n);
> @@ -693,13 +697,10 @@ void libvlc_video_set_deinterlace(
> libvlc_media_player_t *p_mi,>      {
>          vout_thread_t *p_vout = pp_vouts[i];
> 
> -        if (*psz_mode)
> -        {
> +        if (*psz_mode && deinterlace != 0)
>              var_SetString (p_vout, "deinterlace-mode", psz_mode);
> -            var_SetInteger (p_vout, "deinterlace", 1);
> -        }
> -        else
> -            var_SetInteger (p_vout, "deinterlace", 0);
> +
> +        var_SetInteger (p_vout, "deinterlace", deinterlace);
>          vlc_object_release (p_vout);
>      }
>      free (pp_vouts);
> --
> 2.15.1 (Apple Git-101)
> 
> 
> 
> 
>> On 4 May 2018, at 09:58, Carola Nitz via vlc-devel <vlc-
>> devel at videolan.org> wrote:>> 
>> 
>> *From: *Carola Nitz <nitz.carola at googlemail.com>
>> *Subject: **Re: [vlc-devel] [PATCH] Enable/Disable/Auto deinterlace
>> functions for libvlc*>> *Date: *4 May 2018 at 09:58:49 GMT+1
>> *To: *Mailing list for VLC media player developers <vlc-
>> devel at videolan.org>>> 
>> 
>> Hey Luis,
>> 
>> I just wanted to follow up on this. 
>> Did you have time to adjust this patch and address the leak ?
>> I’d love for this to get merged into master :) 
>> 
>> Best,
>> Caro
>> 
>>> On May 3, 2018, at 11:26 AM, Thomas Guillem <thomas at gllm.fr> wrote:>>> 
>>> 
>>> 
>>> On Mon, Apr 30, 2018, at 18:29, Luís Miguel Fernandes wrote:
>>>> Hi Steve,
>>>> 
>>>> Thanks for the comments, here's the revised patches.
>>>> 
>>>> From effa894f8231379841f7c387a6931064fcca929d Mon Sep 17
>>>> 00:00:00 2001>>>> From: Luis Fernandes <zipleen at gmail.com>
>>>> Date: Mon, 30 Apr 2018 14:33:08 +0100
>>>> Subject: [PATCH] add auto deinterlacer-mode which is also valid
>>>> 
>>>> ---
>>>>  lib/video.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/lib/video.c b/lib/video.c
>>>> index 035cc0ebf1..a3e5b248e9 100644
>>>> --- a/lib/video.c
>>>> +++ b/lib/video.c
>>>> @@ -675,7 +675,8 @@ void libvlc_video_set_deinterlace(
>>>> libvlc_media_player_t *p_mi,>>>>       && strcmp (psz_mode, "discard")  && strcmp (psz_mode,
>>>>       "linear")>>>>       && strcmp (psz_mode, "mean")     && strcmp (psz_mode, "x")
>>>>       && strcmp (psz_mode, "yadif")    && strcmp (psz_mode,
>>>>       "yadif2x")>>>> -     && strcmp (psz_mode, "phosphor") && strcmp (psz_mode,
>>>>       "ivtc"))>>>> +     && strcmp (psz_mode, "phosphor") && strcmp (psz_mode, "ivtc")>>>> +     && strcmp (psz_mode, "auto"))
>>>>          return;
>>>> 
>>>>      if (*psz_mode)
>>>> --
>>>> 2.14.3 (Apple Git-98)
>>>> 
>>>> From 2b656a5fca5c69e626ac1727d7bc9f3d980f37e6 Mon Sep 17
>>>> 00:00:00 2001>>>> From: Luis Fernandes <zipleen at gmail.com>
>>>> Date: Mon, 30 Apr 2018 14:43:00 +0100
>>>> Subject: [PATCH] change function of deinterlace to specify which
>>>> deinterlace>>>>  mode and filter we want to use
>>>> 
>>>> ---
>>>>  include/vlc/libvlc_media_player.h |  5 ++++-
>>>>  lib/video.c                       | 34 ++++++++++++++++++++-------------->>>>  2 files changed, 24 insertions(+), 15 deletions(-)
>>>> 
>>>> diff --git a/include/vlc/libvlc_media_player.h
>>>> b/include/vlc/libvlc_media_player.h>>>> index 20b220448b..a82f70f5e7 100644
>>>> --- a/include/vlc/libvlc_media_player.h
>>>> +++ b/include/vlc/libvlc_media_player.h
>>>> @@ -1419,9 +1419,12 @@ int libvlc_video_take_snapshot(
>>>> libvlc_media_player_t *p_mi, unsigned num,>>>>   * Enable or disable deinterlace filter
>>>>   *
>>>>   * \param p_mi libvlc media player
>>>> - * \param psz_mode type of deinterlace filter, NULL to disable
>>>> + * \param deinterlace deinterlace mode -1:auto (default), 0:
>>>>   disabled, 1: enabled>>>> + * \param psz_mode type of deinterlace filter, NULL to default
>>>> + * \version LibVLC 4.0.0 and later
>>>>   */
>>>>  LIBVLC_API void libvlc_video_set_deinterlace(
>>>>  libvlc_media_player_t *p_mi,>>>> +                                                  int deinterlace,>>>>                                                    const char
>>>>                                                    *psz_mode );>>>> 
>>>>  /**
>>>> diff --git a/lib/video.c b/lib/video.c
>>>> index a3e5b248e9..f96fec7a79 100644
>>>> --- a/lib/video.c
>>>> +++ b/lib/video.c
>>>> @@ -663,13 +663,25 @@ end:
>>>>  }
>>>> 
>>>>  /****************************************************************-
>>>>  **************>>>> - * libvlc_video_set_deinterlace : enable deinterlace
>>>> + * libvlc_video_set_deinterlace : enable/disable/auto deinterlace
>>>>   and filter>>>>   ****************************************************************-
>>>>   *************/>>>> -void libvlc_video_set_deinterlace( libvlc_media_player_t *p_mi,
>>>> +void libvlc_video_set_deinterlace( libvlc_media_player_t *p_mi,
>>>> int deinterlace,>>>>                                     const char *psz_mode )
>>>>  {
>>>> +    if (deinterlace != 0 && deinterlace != 1)
>>>> +        deinterlace = -1;
>>>> +
>>>> +    if (psz_mode == NULL)
>>>> +    {
>>>> +        psz_mode = var_GetString(p_mi, "deinterlace-mode");
>>>> +    }
>>> 
>>> You leak psz_mode here.
>>> But I think it should stay NULL. Just don't set "deinterlace-mode"
>>> in that case but only the "deinterlace" int.>>> 
>>>> +
>>>>      if (psz_mode == NULL)
>>>> +    {
>>>>          psz_mode = "";
>>>> +        deinterlace = 0;
>>>> +    }
>>>> +
>>>>      if (*psz_mode
>>>>       && strcmp (psz_mode, "blend")    && strcmp (psz_mode, "bob")>>>>       && strcmp (psz_mode, "discard")  && strcmp (psz_mode,
>>>>       "linear")>>>> @@ -679,13 +691,10 @@ void libvlc_video_set_deinterlace(
>>>> libvlc_media_player_t *p_mi,>>>>       && strcmp (psz_mode, "auto"))
>>>>          return;
>>>> 
>>>> -    if (*psz_mode)
>>>> -    {
>>>> +    if (*psz_mode && deinterlace != 0)
>>>>          var_SetString (p_mi, "deinterlace-mode", psz_mode);
>>>> -        var_SetInteger (p_mi, "deinterlace", 1);
>>>> -    }
>>>> -    else
>>>> -        var_SetInteger (p_mi, "deinterlace", 0);
>>>> +
>>>> +    var_SetInteger (p_mi, "deinterlace", deinterlace);
>>>> 
>>>>      size_t n;
>>>>      vout_thread_t **pp_vouts = GetVouts (p_mi, &n);
>>>> @@ -693,13 +702,10 @@ void libvlc_video_set_deinterlace(
>>>> libvlc_media_player_t *p_mi,>>>>      {
>>>>          vout_thread_t *p_vout = pp_vouts[i];
>>>> 
>>>> -        if (*psz_mode)
>>>> -        {
>>>> +        if (*psz_mode && deinterlace != 0)
>>>>              var_SetString (p_vout, "deinterlace-mode", psz_mode);>>>> -            var_SetInteger (p_vout, "deinterlace", 1);
>>>> -        }
>>>> -        else
>>>> -            var_SetInteger (p_vout, "deinterlace", 0);
>>>> +
>>>> +        var_SetInteger (p_vout, "deinterlace", deinterlace);
>>>>          vlc_object_release (p_vout);
>>>>      }
>>>>      free (pp_vouts);
>>>> --
>>>> 2.14.3 (Apple Git-98)
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> Regards,
>>>> Luís
>>>> 
>>>>> On 30 Apr 2018, at 07:55, Steve Lhomme <robux4 at ycbcr.xyz> wrote:
>>>>> 
>>>>> Please post your patches inline, it's easier to comment


>>>>> 


>>>>> 
>>>>> +    if (deinterlace < -1 || deinterlace > 1)
>>>>> +        deinterlace = -1;
>>>>> To make it more clear what you're doing maybe check if the value
>>>>> is neither 0 nor 1.>>>>> 
>>>>> 


>>>>> 
>>>>> +        module_config_t *config = config_FindConfig("deinterlace-
>>>>>          mode");>>>>> This should be var_GetString
>>>>> 
>>>>> 


>>>>> 
>>>>> +     && strcmp (psz_mode, "auto"))
>>>>> It seems that should go in another patch before this one.
>>>>> 
>>>>> 


>>>>> 
>>>>> -        var_SetInteger (p_mi, "deinterlace", 1);
>>>>> +        var_SetInteger (p_mi, "deinterlace", deinterlace);
>>>>>      }
>>>>>      else
>>>>> -        var_SetInteger (p_mi, "deinterlace", 0);
>>>>> +        var_SetInteger (p_mi, "deinterlace", deinterlace);
>>>>> 
>>>>> In both cases of the else you do the same call. So don't do it in
>>>>> the else.>>>>> 
>>>>> Le 28/04/2018 à 20:13, Luís Fernandes a écrit :
>>>>>> Hi all,  I’ve reworked the patch to change the method signature,
>>>>>> and thus break compatibility with 3.0. I’ve added a \version
>>>>>> annotation to the .h of the function, but I have no idea if I
>>>>>> need to do anything else to signal that this is for libvlc4.
>>>>>> Please advice on what I need to add more for the patch to be
>>>>>> accepted.  Some notes on the patch: - I had to add "auto" because
>>>>>> the default deinterlace-mode for VLCKit is normally "auto", so
>>>>>> without it the mode would never get triggered - default mode is
>>>>>> "auto" (-1) - which seems to be the default mode for vlc as well
>>>>>> Regards, Luís

>>>>>>>>>>>> 
>>>>>>
>>>>>>
>>>>>>> On 22 Apr 2018, at 11:13, Jean-Baptiste Kempf <jb at videolan.org>
>>>>>>> wrote:  Hello,  On Sun, 22 Apr 2018, at 09:37, Rémi Denis-
>>>>>>> Courmont wrote:
>>>>>>>
>>>>>>>> IMO, an API clean-up and thus an ABI break is well overdue in
>>>>>>>> general, and with that, I'd do 1. But that's just my own
>>>>>>>> personal opinion.
>>>>>>>>
>>>>>>> Yes. Break API and ABI for 4.0. libvlc6. Go ahead.  But if
>>>>>>> anyone breaks ABI for 3.0, I'll break his neck. (in a soft
>>>>>>> fashion) :D  -- Jean-Baptiste Kempf -  President +33 672 704 734
>>>>>>> _______________________________________________ vlc-devel
>>>>>>> mailing list To unsubscribe or modify your subscription options:
>>>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>>>>
>>>>>>>>>>>> 
>>>>>>
>>>>>> _______________________________________________ vlc-devel mailing
>>>>>> list To unsubscribe or modify your subscription options:
>>>>>> https://mailman.videolan.org/listinfo/vlc-devel>>>>> 
>>>>> _______________________________________________
>>>>> vlc-devel mailing list
>>>>> To unsubscribe or modify your subscription options:
>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>> _________________________________________________
>>>> vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>> Email had 2 attachments:


>>>>  * auto_deinterlace.patch 1k (text/plain)
>>>>  * set_filter_auto_deinterlace.patch 5k (text/plain)>>> 
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> 
>> 
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> 
> _________________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> Email had 2 attachments:


>  * auto_deinterlace_mode.patch 1k (text/plain)
>  * set_auto_deinterlace.patch 5k (text/plain)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180504/669e9ccd/attachment.html>


More information about the vlc-devel mailing list