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

Carola Nitz nitz.carola at googlemail.com
Fri May 4 10:58:49 CEST 2018


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 <mailto: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 <mailto: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 <mailto: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> <mailto: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 <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 <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 <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 <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 <https://mailman.videolan.org/listinfo/vlc-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180504/e58d62d4/attachment.html>


More information about the vlc-devel mailing list