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

[Lukasz Marek]

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavu-opt-add-consts-where-possible.patch
Type: text/x-patch
Size: 3547 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141127/abaf675e/attachment.bin>