[FFmpeg-devel] [PATCH 4/8] lavu/opt: extend AVOptionRange by second value

[Michael Niedermayer]

On Sun, Mar 23, 2014 at 03:52:49PM +0100, Lukasz Marek wrote:
> On 08.03.2014 22:00, Nicolas George wrote:
> >Le tridi 13 ventôse, an CCXXII, Lukasz Marek a écrit :
> >>OK, I tried to make it different, we have following function:
> >>
> >>int av_opt_query_ranges(AVOptionRanges **, void *obj, const char
> >>*key, int flags);
> >>
> >>AVOptionRanges is **. We can return more than one struct without any
> >>changes to API.
> >>
> >>So querying option size will return 2 x AVOptionRanges, querying
> >>color may return 3/4 x AVOptionRanges.
> >
> >I believe this is a very good idea.
> >
> >>AVOptionRanges may be extended by one filed "option_name" for example
> >>
> >>typedef struct AVOptionRanges {
> >>     AVOptionRange **range;
> >>     int nb_ranges;
> >>     char *option_name
> >>} AVOptionRanges;
> >>
> >>So, when querying for example "window_size" it would return one
> >>struct with option_name set to "window_size.width" and struct with
> >>option_name set to "window_size.height".
> >>
> >>in general option_name param would return option name with suffix
> >>that would be defined per option type. all AVoptionRanges will
> >>require to return the same number of AVOptionRange structs.
> >
> >I do not think it is necessary: the name of the option is known by the
> >caller, and the order of the components is determined by the option type.
> >Parsing a string is not very convenient anyway.
> >
> >>av_opt_query_ranges so far returns >=0 for success, it can be
> >>redefined to return number of AVOptionRanges.
> >
> >That seems like an obvious change.
> 
> 
> I attached a patch that implements this.
> The problem I noticed later is how to free it.
> I added nb_components to AVOptionRanges, but this field is
> duplicated for each component which is probably not the best.
> If you have any other solution to solve freeing then give a hint.
> 
> I pushed whole working branch to my repo github.com/lukaszmluki/ffmpeg
> where implementation for opengl is also available and can be tested with
> tools/device_capabilities
> 
> 

>  opt.c |   33 ++++++++++++++++++++++++---------
>  opt.h |   13 +++++++++++--
>  2 files changed, 35 insertions(+), 11 deletions(-)
> e560f163c6f61b5819e9f382615877e0bdfe12fe  0001-lavu-opt-extend-AVOptionRange-by-extra-values.patch
> From 702508c0c97812ed72deaa133140291fe3637196 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki at gmail.com>
> Date: Sat, 22 Feb 2014 23:32:57 +0100
> Subject: [PATCH] lavu/opt: extend AVOptionRange by extra values
> 
> TODO: micro bump
> 
> AVOptionRange is not flexible enough to store AV_OPT_TYPE_IMAGE_SIZE
> ranges. Current implementation can only store pixel count.
> This patch aims to keep backward compatibility and extend
> AVOptionRange with possibility to store width/height ranges.
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavutil/opt.c | 33 ++++++++++++++++++++++++---------
>  libavutil/opt.h | 13 +++++++++++--
>  2 files changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 652a2dd..77d20b9 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -1511,6 +1511,7 @@ void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
>  
>  int av_opt_query_ranges(AVOptionRanges **ranges_arg, void *obj, const char *key, int flags)
>  {
> +    int ret, i;
>      const AVClass *c = *(AVClass**)obj;
>      int (*callback)(AVOptionRanges **, void *obj, const char *key, int flags) = NULL;
>  
> @@ -1520,7 +1521,14 @@ int av_opt_query_ranges(AVOptionRanges **ranges_arg, void *obj, const char *key,
>      if (!callback)
>          callback = av_opt_query_ranges_default;
>  
> -    return callback(ranges_arg, obj, key, flags);
> +    ret = callback(ranges_arg, obj, key, flags);
> +    if (ret >= 0) {
> +        if (!(flags & AV_OPT_MULTI_COMPINENT_RANGE))
> +            ret = 1;
> +        for (i = 0; i < ret; i++)
> +            (*ranges_arg)[i].nb_components = ret;
> +    }
> +    return ret;
>  }
>  
>  int av_opt_query_ranges_default(AVOptionRanges **ranges_arg, void *obj, const char *key, int flags)
> @@ -1583,8 +1591,9 @@ int av_opt_query_ranges_default(AVOptionRanges **ranges_arg, void *obj, const ch
>          goto fail;
>      }
>  
> +    ranges->nb_components = 1;
>      *ranges_arg = ranges;
> -    return 0;
> +    return 1;
>  fail:
>      av_free(ranges);
>      av_free(range);
> @@ -1594,15 +1603,21 @@ fail:
>  
>  void av_opt_freep_ranges(AVOptionRanges **rangesp)
>  {
> -    int i;
> -    AVOptionRanges *ranges = *rangesp;
> +    int i, r;
> +    AVOptionRanges *ranges;
> +
> +    if (!rangesp || !rangesp[0])
> +        return;
>  
> -    for (i = 0; i < ranges->nb_ranges; i++) {
> -        AVOptionRange *range = ranges->range[i];
> -        av_freep(&range->str);
> -        av_freep(&ranges->range[i]);
> +    for (r = 0; r < rangesp[0]->nb_components; r++) {
> +        ranges = &(*rangesp)[r];
> +        for (i = 0; i < ranges->nb_ranges; i++) {
> +            AVOptionRange *range = ranges->range[i];
> +            av_freep(&range->str);
> +            av_freep(&ranges->range[i]);
> +        }
> +        av_freep(&ranges->range);
>      }
> -    av_freep(&ranges->range);
>      av_freep(rangesp);
>  }
>  
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index cd1b18e..049c49e 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -324,6 +324,7 @@ typedef struct AVOptionRange {
>  typedef struct AVOptionRanges {
>      AVOptionRange **range;
>      int nb_ranges;
> +    int nb_components;
>  } AVOptionRanges;
>  
>  

> @@ -559,6 +560,12 @@ int av_opt_eval_q     (void *obj, const AVOption *o, const char *val, AVRational
>  #define AV_OPT_SEARCH_FAKE_OBJ   0x0002
>  
>  /**
> + *  Allows av_opt_query_ranges and av_opt_query_ranges_default to return more than
> + *  one instance of AVOptionRanges.
> + */
> +#define AV_OPT_MULTI_COMPINENT_RANGE 0x0004

this should be a larger value to reduce chances of compatibility
issues with libav (in case they use 4 for something else)

probably ok otherwise

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140325/983e8ff4/attachment.asc>