Message ID | 20210607230414.612-25-dcnieho@gmail.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | avdevice (mostly dshow) enhancements | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
Actually it is commonly understood all over the world that limited range is the default when not present. All video in the world except Dolby Vision profile 5 (if using IPTPQc2) is limited range.
Valerii Zapodovnikov: > Actually it is commonly understood all over the world that limited range is > the default when not present. All video in the world except Dolby Vision > profile 5 (if using IPTPQc2) is limited range. This has absolutely nothing to do with full/limited range, but rather whether the AVOptionRange contains an interval or a single value. (Not that I know why this needs to be set explicitly and not implicitly via is_range = value_min < value_max.) - Andreas
Ah, yes, that is AVColorRange, sorry. :( Haha.
On Tue, Jun 8, 2021 at 1:45 AM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > This has absolutely nothing to do with full/limited range, but rather > whether the AVOptionRange contains an interval or a single value. > (Not that I know why this needs to be set explicitly and not implicitly > via is_range = value_min < value_max.) > Yes, the is_range field seems superfluous, but its already there. What are your thoughts on the is_set field? For usage example, see patch device_get_capabilities.c in patch 32/35 in this series.
On Tue, Jun 8, 2021 at 1:45 AM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > This has absolutely nothing to do with full/limited range, but rather > whether the AVOptionRange contains an interval or a single value. > (Not that I know why this needs to be set explicitly and not implicitly > via is_range = value_min < value_max.) Hmm, instead of adding a new field, would a bit of documentation be a good idea? There are three cases: 1. output is a range. Then value_min < value_max. 2. output is set but not a range (single value): Then value_min==value_max. (We probably shouldn't remove the is_range flag? OTOH, it is not used in the ffmpeg code base at all...) 3. output is not set: then value_min>value_max (and perhaps special values value_min=0, value_max=-1. A pair of special values does not have the problem that a single special value has: how to distinguish it from an actual value) Then users can easily determine themselves: is_set = value_min > value_max (or preferrably value_min==0 && value_max==-1) is_range = value_min < value_max So two questions: 1. What do you think of documenting case 3 instead of adding a new field to API? 2. What do you think of removing is_range? Cheers, Dee
diff --git a/libavutil/opt.c b/libavutil/opt.c index 4124efd9b6..ab127b30fa 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -1865,6 +1865,7 @@ int av_opt_query_ranges_default(AVOptionRanges **ranges_arg, void *obj, const ch ranges->nb_ranges = 1; ranges->nb_components = 1; range->is_range = 1; + range->is_set = 1; range->value_min = field->min; range->value_max = field->max; diff --git a/libavutil/opt.h b/libavutil/opt.h index c2329e5589..ac0b4567a6 100644 --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -325,6 +325,11 @@ typedef struct AVOptionRange { * If set to 1 the struct encodes a range, if set to 0 a single value. */ int is_range; + /** + * Is set flag. + * If set to 1 the struct contains a value, if set to 0, the range is empty. + */ + int is_set; } AVOptionRange; /** diff --git a/libavutil/version.h b/libavutil/version.h index e11eaa20d0..34b83112de 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 57 -#define LIBAVUTIL_VERSION_MINOR 0 +#define LIBAVUTIL_VERSION_MINOR 1 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
When querying a range of formats of an avdevice, sometimes for a given format the queried option is not available. This is not an error as the user is asking for a valid capability, it just doesn't always apply to all the matching formats of the device. This is now communicated through a special value (like 0 or -1), but that is a problem (like always with special values). The is_set field alleviates the use of special values. This will be used when implementing the avdevice capabilities API for avdevice/dshow in a later commit Signed-off-by: Diederick Niehorster <dcnieho@gmail.com> --- libavutil/opt.c | 1 + libavutil/opt.h | 5 +++++ libavutil/version.h | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-)