[FFmpeg-devel] [PATCH 1/2] opt: Add support to query ranges
Michael Niedermayer
michaelni at gmx.at
Sat Dec 8 00:58:48 CET 2012
On Tue, Dec 04, 2012 at 11:31:40PM +0100, Stefano Sabatini wrote:
> On date Tuesday 2012-12-04 20:46:40 +0100, Michael Niedermayer encoded:
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> > libavutil/log.h | 6 +++++
> > libavutil/opt.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > libavutil/opt.h | 50 ++++++++++++++++++++++++++++++++++++
> > 3 files changed, 133 insertions(+)
> >
> > diff --git a/libavutil/log.h b/libavutil/log.h
> > index ba7315f..b5205c5 100644
> > --- a/libavutil/log.h
> > +++ b/libavutil/log.h
> > @@ -114,6 +114,12 @@ typedef struct AVClass {
> > * available since version (51 << 16 | 59 << 8 | 100)
> > */
> > AVClassCategory (*get_category)(void* ctx);
> > +
> > + /**
> > + * Callback to return the supported/allowed ranges.
> > + * available since version (???)
> > + */
> > + struct AVOptionRanges *(*query_ranges)(void *obj, const char *key, int flags);
> > } AVClass;
> >
> > /* av_log API */
> > diff --git a/libavutil/opt.c b/libavutil/opt.c
> > index 0ff45c8..cd26682 100644
> > --- a/libavutil/opt.c
> > +++ b/libavutil/opt.c
> > @@ -1161,6 +1161,83 @@ void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
> > return (uint8_t*)obj + opt->offset;
> > }
> >
> > +AVOptionRanges *av_opt_query_ranges(void *obj, const char *key, int flags) {
> > + const AVClass *c = *(AVClass**)obj;
> > + AVOptionRanges *(*callback)(void *obj, const char *key, int flags) = NULL;
> > +
>
> > + if(c->version > (52 << 16 | 8 << 9))
>
> 8 << 9 => 9 << 8?
>
> also the minor number should be updated.
yes, fixed
>
> > + callback = c->query_ranges;
> > +
> > + if(!callback)
> > + callback = av_opt_query_ranges_default;
> > +
> > + return callback(obj, key, flags);
> > +}
> > +
> > +AVOptionRanges *av_opt_query_ranges_default(void *obj, const char *key, int flags) {
> > + AVOptionRanges *ranges = av_mallocz(sizeof(*ranges));
> > + AVOptionRange **range_array = av_mallocz(sizeof(void*));
> > + AVOptionRange *range = av_mallocz(sizeof(*range));
> > + const AVOption *field = av_opt_find(obj, key, NULL, 0, flags);
> > +
> > + if(!ranges || !range || !range_array || !field)
> > + goto fail;
> > +
> > + ranges->range = range_array;
> > + ranges->range[0] = range;
> > + ranges->nb_ranges = 1;
> > + range->is_range = 1;
> > + range->value_min = field->min;
> > + range->value_max = field->max;
> > +
> > + switch(field->type){
> > + case AV_OPT_TYPE_INT:
> > + case AV_OPT_TYPE_INT64:
> > + case AV_OPT_TYPE_PIXEL_FMT:
> > + case AV_OPT_TYPE_SAMPLE_FMT:
> > + case AV_OPT_TYPE_FLOAT:
> > + case AV_OPT_TYPE_DOUBLE:
> > + break;
> > + case AV_OPT_TYPE_STRING:
> > + range->component_min = 0;
>
> > + range->component_max = 0x10FFFF;
>
> just curious, what this value represents? An inline comment may also help.
added
>
> > + range->value_min = -1;
> > + range->value_max = INT_MAX;
> > + break;
> > + case AV_OPT_TYPE_RATIONAL:
> > + range->component_min = INT_MIN;
> > + range->component_max = INT_MAX;
> > + break;
> > + case AV_OPT_TYPE_IMAGE_SIZE:
> > + range->component_min = 0;
> > + range->component_max = INT_MAX/128/8;
> > + range->value_min = 0;
> > + range->value_max = INT_MAX/8;
> > + break;
> > + default:
> > + goto fail;
> > + }
> > +
> > + return ranges;
> > +fail:
> > + av_free(ranges);
> > + av_free(range);
> > + return NULL;
> > +}
> > +
> > +void av_opt_freep_ranges(AVOptionRanges **rangesp) {
> > + int i;
> > + AVOptionRanges *ranges = *rangesp;
> > +
> > + 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(rangesp);
> > +}
> > +
> > #ifdef TEST
> >
> > #undef printf
> > diff --git a/libavutil/opt.h b/libavutil/opt.h
> > index bf84a9b..742aaec 100644
> > --- a/libavutil/opt.h
> > +++ b/libavutil/opt.h
> > @@ -293,6 +293,25 @@ typedef struct AVOption {
> > const char *unit;
> > } AVOption;
> >
> > +/**
> > + * A single allowed range of values, or a single allowed value.
> > + */
> > +typedef struct AVOptionRange {
> > + const char *str;
>
> > + double value_min, value_max; ///< For string ranges this represents the min/max length, for dimensions this represents the min/max pixel count
> > + double component_min, component_max; ///< For string this represents the unicode range for chars, 0-127 limits to ASCII
>
> I'm not yet convinced that this is actually useful in case of strings,
> checks are usually performed in the setting code so the exposed
> constraints are doomed to stay inconsistent.
we can easily drop this later and declare them undefined for strings
>
> > + int is_range; ///< 1->range, 0->single value
>
> Less compressed (saves some decoding on the user side):
> ///< if set to 1 the struct encodes a range, if set to 0 a single value
changed
>
> > +} AVOptionRange;
> > +
> > +/**
> > + * List of AVOptionRange structs
> > + */
> > +typedef struct AVOptionRanges {
> > + AVOptionRange **range;
> > + int nb_ranges;
> > +} AVOptionRanges;
> > +
> > +
> > #if FF_API_FIND_OPT
> > /**
> > * Look for an option in obj. Look only for the options which
> > @@ -672,6 +691,37 @@ int av_opt_get_sample_fmt(void *obj, const char *name, int search_flags, enum AV
> > * or written to.
> > */
> > void *av_opt_ptr(const AVClass *avclass, void *obj, const char *name);
> > +
> > +/**
>
> > + * Free a AVOptionRanges struct and set it to NULL.
>
> Nit++: Free an AVOptionRanges?
fixed
>
> > + */
> > +void av_opt_freep_ranges(AVOptionRanges **ranges);
> > +
> > +/**
> > + * Get a list of allowed ranges for the given option.
> > + *
> > + * The returned list may depend on other fields in obj like for example profile.
> > + *
> > + * @param flags is a bitmask of flags, undefined flags should not be set and should be ignored
> > + * AV_OPT_SEARCH_FAKE_OBJ indicates that the obj is a double pointer to a AVClass instead of a full instance
> > + *
> > + * The result must be freed with av_opt_freep_ranges
>
> Nit: missing final dot.
added
>
> > + */
> > +AVOptionRanges *av_opt_query_ranges(void *obj, const char *key, int flags);
>
> Regarding the return value: I prefer to handle an int, rather than
> picking and propagating an arbitrary error code, also should improve
> coding consistency.
changed
>
> > +
> > +/**
> > + * Get a default list of allowed ranges for the given option.
> > + *
>
> > + * This list is constructed without using the callback and can be used as
> > + * fallback from within the callback.
>
> This may be clarified, what callback is referenced?
fixed
>
> > + *
> > + * @param flags is a bitmask of flags, undefined flags should not be set and should be ignored
> > + * AV_OPT_SEARCH_FAKE_OBJ indicates that the obj is a double pointer to a AVClass instead of a full instance
> > + *
> > + * The result must be freed with av_opt_free_ranges
>
> Nit+: missing final dot.
added
will push soon
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- 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/20121208/ea84469c/attachment.asc>
More information about the ffmpeg-devel
mailing list