[vlc-devel] [PATCH] Enable/Disable/Auto deinterlace functions for libvlc
Luis Fernandes
zipleen at gmail.com
Fri May 4 18:12:33 CEST 2018
Hi Thomas,
No worries!
The forcing of that is about the comment in the function "if mode is NULL, deinterlace will be disabled".
If you think this is not useful, I'll remove it right away - I'm not too attached to the idea, it was used in the previous implementation to "disable" deinterlace.
Cheers,
Luís
> On 4 May 2018, at 16:34, Thomas Guillem <thomas at gllm.fr> wrote:
>
>
>> 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)
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180504/81f12d54/attachment-0001.html>
More information about the vlc-devel
mailing list