[FFmpeg-devel] [PATCH] lavu/opt: add const to av_opt_copy arg

[Lukasz Marek]

On 27.11.2014 04:06, Michael Niedermayer wrote:
> On Thu, Nov 27, 2014 at 12:37:38AM +0100, Lukasz Marek wrote:
>> On 24.11.2014 02:06, Michael Niedermayer wrote:
>>> On Sun, Nov 23, 2014 at 10:25:39PM +0100, Lukasz Marek wrote:
>>>> On 23.11.2014 02:29, Michael Niedermayer wrote:
>>>>> On Sun, Nov 23, 2014 at 01:01:19AM +0100, Lukasz Marek wrote:
>>>>>> On 23.11.2014 00:58, Lukasz Marek wrote:
>>>>>>> Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
>>>>>>> ---
>>>>>>>   libavutil/opt.c | 2 +-
>>>>>>>   libavutil/opt.h | 2 +-
>>>>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/libavutil/opt.c b/libavutil/opt.c
>>>>>>> index 0546a37..47b1f0c 100644
>>>>>>> --- a/libavutil/opt.c
>>>>>>> +++ b/libavutil/opt.c
>>>>>>> @@ -1570,7 +1570,7 @@ static int opt_size(enum AVOptionType type)
>>>>>>>       return 0;
>>>>>>>   }
>>>>>>>
>>>>>>> -int av_opt_copy(void *dst, void *src)
>>>>>>> +int av_opt_copy(void *dst, FF_CONST_AVUTIL53 void *src)
>>>>>>>   {
>>>>>>>       const AVOption *o = NULL;
>>>>>>>       const AVClass *c;
>>>>>>> diff --git a/libavutil/opt.h b/libavutil/opt.h
>>>>>>> index 7338e78..6b6c996 100644
>>>>>>> --- a/libavutil/opt.h
>>>>>>> +++ b/libavutil/opt.h
>>>>>>> @@ -825,7 +825,7 @@ int av_opt_query_ranges(AVOptionRanges **, void *obj, const char *key, int flags
>>>>>>>    * @param src  Object to copy into
>>>>>>>    * @return 0 on success, negative on error
>>>>>>>    */
>>>>>>> -int av_opt_copy(void *dest, void *src);
>>>>>>> +int av_opt_copy(void *dest, FF_CONST_AVUTIL53 void *src);
>>>>>>>
>>>>>>>   /**
>>>>>>>    * Get a default list of allowed ranges for the given option.
>>>>>>>
>>>>>>
>>>>>> I added FF_CONST_AVUTILS53 macro, but is this really needed? Cannot
>>>>>> be just const?
>>>>>> I'm asking because I think it doesn't fix anything.
>>>>>> I guess is it API/ABI thing, but why?
>>>>>
>>>>> a user application could have a function pointer like
>>>>>
>>>>> all_ffmpeg_functions->opt_copy = av_opt_copy;
>>>>>
>>>>> and if we add const the prototype changes and this can fail to build
>>>>> with some compiler flags or C++ or whatever
>>>>>
>>>>> and yes i have seen an application that had function pointers to
>>>>> ffmpeg functions
>>>>
>>>> Thx for explanation. It would be good to have option to disable this
>>>> compability mode too. Sometimes it is not helping but annoying for
>>>> most users.
>>>>
>>>> I attached updated patch. I add const to av_next_option and
>>>> av_opt_next to avoid warnings. It have to be applied on top of
>>>> [PATCH 1/2] lavu/opt: handle NULL obj in av_opt_next
>>>>
>>>> There is much more places it could be added in opt.c, but this would
>>>> again trigger adding in other files (like log.h) to avoid warnings.
>>>>
>>>
>>>>   opt.c |    6 +++---
>>>>   opt.h |    6 +++---
>>>>   2 files changed, 6 insertions(+), 6 deletions(-)
>>>> 8a3df1768aaaef53aa632d5515b5041e7ba7c8f0  0001-lavu-opt-add-const-to-av_opt_copy-arg-and-dependenci.patch
>>>>  From a8989702029f8c536b2153d3e6b52b1c3a9cc20f Mon Sep 17 00:00:00 2001
>>>> From: Lukasz Marek <lukasz.m.luki2 at gmail.com>
>>>> Date: Sat, 22 Nov 2014 20:41:21 +0100
>>>> Subject: [PATCH] lavu/opt: add const to av_opt_copy arg and dependencies
>>>>
>>>> Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
>>>> ---
>>>>   libavutil/opt.c | 6 +++---
>>>>   libavutil/opt.h | 6 +++---
>>>>   2 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/libavutil/opt.c b/libavutil/opt.c
>>>> index 5b26a00..5b305a4 100644
>>>> --- a/libavutil/opt.c
>>>> +++ b/libavutil/opt.c
>>>> @@ -42,13 +42,13 @@
>>>>   #include <float.h>
>>>>
>>>>   #if FF_API_OLD_AVOPTIONS
>>>> -const AVOption *av_next_option(void *obj, const AVOption *last)
>>>> +const AVOption *av_next_option(FF_CONST_AVUTIL53 void *obj, const AVOption *last)
>>>
>>> FF_CONST_AVUTIL53 is already set to const, a new
>>> FF_CONST_AVUTIL55 would be needed
>>
>> Updated version attached.
>> const could be added almost everywhere, but av_opt_find2 is blocking a lot.
>>
>>   opt.c     |    8 ++++----
>>   opt.h     |    7 ++++---
>>   version.h |    7 +++++++
>>   3 files changed, 15 insertions(+), 7 deletions(-)
>> dcd1e5a4a60083437e69810821b1b856f0a328dd  0001-lavu-opt-add-consts-where-possible.patch
>>  From 49c6d2bd65ee36bc7c6a5947be442f1c49b0d4f7 Mon Sep 17 00:00:00 2001
>> From: Lukasz Marek <lukasz.m.luki2 at gmail.com>
>> Date: Thu, 27 Nov 2014 00:11:01 +0100
>> Subject: [PATCH] lavu/opt: add consts where possible
>
> LGTM

pushed