Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
On Sun, 3 Mar 2024, James Almer wrote: On 3/3/2024 12:53 PM, Diederick C. Niehorster wrote: On Sun, Mar 3, 2024 at 3:55 PM Anton Khirnov wrote: Quoting Marton Balint (2024-02-26 20:38:46) The more I think about it, it is actually a broader problem. You are changing the semantics of existing AV_OPT_TYPE_xxx types. So previously an option with AV_OPT_TYPE_STRING used to have default value in default_val.str. After your patch, it will be either default_val.str, or default_val.str[1], based on if it is an array or not. I think the API user safely assumed that if the option type is known to him, he will always find the default value in the relevant default_val field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT, because previously to get the default value AVOption->default_val.i64 was used, and now .str[1] should be instead... In my view the semantics of default_val (and offset) are only defined when declaring options on your own object, not when accessing those fields when declared by some other code. I also see no good reason for any user to read these fields. I disagree. Here an example: for a GUI using some part of ffmpeg and wanting to give the user some control over the ffmpeg operation, it would not be strange to be able to set options and either indicate which values are default, or have a "reset to defaults" button. I have written such a thing (not yet opensourced). There's av_opt_set_defaults() to set default values, then av_opt_is_set_to_default() and av_opt_is_set_to_default_by_name() to check if an option is already set to the default value. What Anton said is that the user has no need to access the fields directly when there are helpers explicitly documented for this purpose. The AVOption struct is public, and the default_val field is public too. There is nothing that warns the user not to access it. The fact that there are helper functions does not change this. But it is not the default values that is a problem. The semantic change is the problem. I could have had a code which iterates through every AVOption of an object and prints the values with type AV_OPT_TYPE_INT. Your change will suddently break that, because arrays will also have the type of AV_OPT_TYPE_INT. So can you please introduce a new option type for arrays? Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
On 3/3/2024 12:53 PM, Diederick C. Niehorster wrote: On Sun, Mar 3, 2024 at 3:55 PM Anton Khirnov wrote: Quoting Marton Balint (2024-02-26 20:38:46) The more I think about it, it is actually a broader problem. You are changing the semantics of existing AV_OPT_TYPE_xxx types. So previously an option with AV_OPT_TYPE_STRING used to have default value in default_val.str. After your patch, it will be either default_val.str, or default_val.str[1], based on if it is an array or not. I think the API user safely assumed that if the option type is known to him, he will always find the default value in the relevant default_val field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT, because previously to get the default value AVOption->default_val.i64 was used, and now .str[1] should be instead... In my view the semantics of default_val (and offset) are only defined when declaring options on your own object, not when accessing those fields when declared by some other code. I also see no good reason for any user to read these fields. I disagree. Here an example: for a GUI using some part of ffmpeg and wanting to give the user some control over the ffmpeg operation, it would not be strange to be able to set options and either indicate which values are default, or have a "reset to defaults" button. I have written such a thing (not yet opensourced). There's av_opt_set_defaults() to set default values, then av_opt_is_set_to_default() and av_opt_is_set_to_default_by_name() to check if an option is already set to the default value. What Anton said is that the user has no need to access the fields directly when there are helpers explicitly documented for this purpose. Also the ffmpeg CLI has the ability to print the options and their defaults for a component. Can this still be done? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
On Sun, Mar 3, 2024 at 3:55 PM Anton Khirnov wrote: > Quoting Marton Balint (2024-02-26 20:38:46) > > The more I think about it, it is actually a broader problem. > > > > You are changing the semantics of existing AV_OPT_TYPE_xxx types. So > > previously an option with AV_OPT_TYPE_STRING used to have default value > in > > default_val.str. After your patch, it will be either default_val.str, or > > default_val.str[1], based on if it is an array or not. > > > > I think the API user safely assumed that if the option type is known to > > him, he will always find the default value in the relevant default_val > > field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT, > > because previously to get the default value AVOption->default_val.i64 > was > > used, and now .str[1] should be instead... > > In my view the semantics of default_val (and offset) are only defined > when declaring options on your own object, not when accessing those > fields when declared by some other code. I also see no good reason for > any user to read these fields. > I disagree. Here an example: for a GUI using some part of ffmpeg and wanting to give the user some control over the ffmpeg operation, it would not be strange to be able to set options and either indicate which values are default, or have a "reset to defaults" button. I have written such a thing (not yet opensourced). Also the ffmpeg CLI has the ability to print the options and their defaults for a component. Can this still be done? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
Quoting Marton Balint (2024-02-26 20:38:46) > The more I think about it, it is actually a broader problem. > > You are changing the semantics of existing AV_OPT_TYPE_xxx types. So > previously an option with AV_OPT_TYPE_STRING used to have default value in > default_val.str. After your patch, it will be either default_val.str, or > default_val.str[1], based on if it is an array or not. > > I think the API user safely assumed that if the option type is known to > him, he will always find the default value in the relevant default_val > field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT, > because previously to get the default value AVOption->default_val.i64 was > used, and now .str[1] should be instead... In my view the semantics of default_val (and offset) are only defined when declaring options on your own object, not when accessing those fields when declared by some other code. I also see no good reason for any user to read these fields. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
On Mon, 26 Feb 2024, Anton Khirnov wrote: Quoting Marton Balint (2024-02-23 20:05:06) On Fri, 23 Feb 2024, Anton Khirnov wrote: AVOption.array_max_size is added before AVOption.unit to avoid increasing sizeof(AVOption). --- doc/APIchanges| 3 + libavutil/opt.c | 344 -- libavutil/opt.h | 26 libavutil/tests/opt.c | 34 + tests/ref/fate/opt| 23 ++- 5 files changed, 385 insertions(+), 45 deletions(-) [...] --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -288,6 +288,16 @@ enum AVOptionType{ */ #define AV_OPT_FLAG_CHILD_CONSTS(1 << 18) +/** + * The option is an array. + * + * When adding array options to an object, @ref AVOption.offset should refer to + * a pointer corresponding to the option type. The pointer should be immediately + * followed by an unsigned int that will store the number of elements in the + * array. + */ +#define AV_OPT_FLAG_ARRAY (1 << 19) + /** * AVOption */ @@ -313,6 +323,16 @@ typedef struct AVOption { union { int64_t i64; double dbl; + +/** + * This member is always used for AV_OPT_FLAG_ARRAY options. When + * non-NULL, the first character of the string must be the separator to + * be used for (de)serializing lists to/from strings with av_opt_get(), This is quite ugly. Also it breaks the assumption that if the user sets an option value to the default value of the option, than it will work. I don't follow, what assumption are you talking about? The more I think about it, it is actually a broader problem. You are changing the semantics of existing AV_OPT_TYPE_xxx types. So previously an option with AV_OPT_TYPE_STRING used to have default value in default_val.str. After your patch, it will be either default_val.str, or default_val.str[1], based on if it is an array or not. I think the API user safely assumed that if the option type is known to him, he will always find the default value in the relevant default_val field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT, because previously to get the default value AVOption->default_val.i64 was used, and now .str[1] should be instead... The way I see it, the ARRAY modifier should either be a flag/mask of the option type (not AVOption->flags), or it should be a entierly new type, AV_OPT_TYPE_ARRAY, and its base type should be stored elsewhere. For new opt types, we can define the semantics of default_val as we want, so Andreas's suggestion is good to make default_val point to an AVArrayOptionSettings struct or something like that. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
On 2/26/2024 2:14 PM, Anton Khirnov wrote: Quoting Marton Balint (2024-02-23 20:05:06) On Fri, 23 Feb 2024, Anton Khirnov wrote: AVOption.array_max_size is added before AVOption.unit to avoid increasing sizeof(AVOption). --- doc/APIchanges| 3 + libavutil/opt.c | 344 -- libavutil/opt.h | 26 libavutil/tests/opt.c | 34 + tests/ref/fate/opt| 23 ++- 5 files changed, 385 insertions(+), 45 deletions(-) [...] --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -288,6 +288,16 @@ enum AVOptionType{ */ #define AV_OPT_FLAG_CHILD_CONSTS(1 << 18) +/** + * The option is an array. + * + * When adding array options to an object, @ref AVOption.offset should refer to + * a pointer corresponding to the option type. The pointer should be immediately + * followed by an unsigned int that will store the number of elements in the + * array. + */ +#define AV_OPT_FLAG_ARRAY (1 << 19) + /** * AVOption */ @@ -313,6 +323,16 @@ typedef struct AVOption { union { int64_t i64; double dbl; + +/** + * This member is always used for AV_OPT_FLAG_ARRAY options. When + * non-NULL, the first character of the string must be the separator to + * be used for (de)serializing lists to/from strings with av_opt_get(), This is quite ugly. Also it breaks the assumption that if the user sets an option value to the default value of the option, than it will work. I don't follow, what assumption are you talking about? So let's just remove this feature for now. Eventually I think some new struct should be introduced, e.g. AVOptionExtension, which can be used to specify additional option settings, such as array min/max size, and maybe separator. It would be a lot more clean and future proof than filling the holes in AVOption. I've actually considered that, but don't see a clean way of linking such an extension with its option. We only have an int-sized hole, so can't add a new pointer field to AVOption. If there's no other solution, then adding a new field is ok. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
Anton Khirnov: > Quoting Marton Balint (2024-02-23 20:05:06) >> >> >> On Fri, 23 Feb 2024, Anton Khirnov wrote: >> >>> AVOption.array_max_size is added before AVOption.unit to avoid >>> increasing sizeof(AVOption). >>> --- >>> doc/APIchanges| 3 + >>> libavutil/opt.c | 344 -- >>> libavutil/opt.h | 26 >>> libavutil/tests/opt.c | 34 + >>> tests/ref/fate/opt| 23 ++- >>> 5 files changed, 385 insertions(+), 45 deletions(-) >>> >> [...] >> >>> --- a/libavutil/opt.h >>> +++ b/libavutil/opt.h >>> @@ -288,6 +288,16 @@ enum AVOptionType{ >>> */ >>> #define AV_OPT_FLAG_CHILD_CONSTS(1 << 18) >>> >>> +/** >>> + * The option is an array. >>> + * >>> + * When adding array options to an object, @ref AVOption.offset should >>> refer to >>> + * a pointer corresponding to the option type. The pointer should be >>> immediately >>> + * followed by an unsigned int that will store the number of elements in >>> the >>> + * array. >>> + */ >>> +#define AV_OPT_FLAG_ARRAY (1 << 19) >>> + >>> /** >>> * AVOption >>> */ >>> @@ -313,6 +323,16 @@ typedef struct AVOption { >>> union { >>> int64_t i64; >>> double dbl; >>> + >>> +/** >>> + * This member is always used for AV_OPT_FLAG_ARRAY options. When >>> + * non-NULL, the first character of the string must be the >>> separator to >>> + * be used for (de)serializing lists to/from strings with >>> av_opt_get(), >> >> This is quite ugly. Also it breaks the assumption that if the user sets an >> option value to the default value of the option, than it will work. > > I don't follow, what assumption are you talking about? > >> So let's just remove this feature for now. >> >> Eventually I think some new struct should be introduced, e.g. >> AVOptionExtension, which can be used to specify additional option >> settings, such as array min/max size, and maybe separator. It would be a >> lot more clean and future proof than filling the holes in AVOption. > > I've actually considered that, but don't see a clean way of linking such > an extension with its option. We only have an int-sized hole, so can't > add a new pointer field to AVOption. I suppose there could be a new > array of extensions in AVClass, linked to options by name, but that > seems even more ugly and inefficient. > You could add an AVArrayOption struct and use a pointer to such a struct as default value for an array option. Then you would not be constrained by an int-sized hole. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
Quoting Marton Balint (2024-02-23 20:05:06) > > > On Fri, 23 Feb 2024, Anton Khirnov wrote: > > > AVOption.array_max_size is added before AVOption.unit to avoid > > increasing sizeof(AVOption). > > --- > > doc/APIchanges| 3 + > > libavutil/opt.c | 344 -- > > libavutil/opt.h | 26 > > libavutil/tests/opt.c | 34 + > > tests/ref/fate/opt| 23 ++- > > 5 files changed, 385 insertions(+), 45 deletions(-) > > > [...] > > > --- a/libavutil/opt.h > > +++ b/libavutil/opt.h > > @@ -288,6 +288,16 @@ enum AVOptionType{ > > */ > > #define AV_OPT_FLAG_CHILD_CONSTS(1 << 18) > > > > +/** > > + * The option is an array. > > + * > > + * When adding array options to an object, @ref AVOption.offset should > > refer to > > + * a pointer corresponding to the option type. The pointer should be > > immediately > > + * followed by an unsigned int that will store the number of elements in > > the > > + * array. > > + */ > > +#define AV_OPT_FLAG_ARRAY (1 << 19) > > + > > /** > > * AVOption > > */ > > @@ -313,6 +323,16 @@ typedef struct AVOption { > > union { > > int64_t i64; > > double dbl; > > + > > +/** > > + * This member is always used for AV_OPT_FLAG_ARRAY options. When > > + * non-NULL, the first character of the string must be the > > separator to > > + * be used for (de)serializing lists to/from strings with > > av_opt_get(), > > This is quite ugly. Also it breaks the assumption that if the user sets an > option value to the default value of the option, than it will work. I don't follow, what assumption are you talking about? > So let's just remove this feature for now. > > Eventually I think some new struct should be introduced, e.g. > AVOptionExtension, which can be used to specify additional option > settings, such as array min/max size, and maybe separator. It would be a > lot more clean and future proof than filling the holes in AVOption. I've actually considered that, but don't see a clean way of linking such an extension with its option. We only have an int-sized hole, so can't add a new pointer field to AVOption. I suppose there could be a new array of extensions in AVClass, linked to options by name, but that seems even more ugly and inefficient. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
On Fri, 23 Feb 2024, Anton Khirnov wrote: AVOption.array_max_size is added before AVOption.unit to avoid increasing sizeof(AVOption). --- doc/APIchanges| 3 + libavutil/opt.c | 344 -- libavutil/opt.h | 26 libavutil/tests/opt.c | 34 + tests/ref/fate/opt| 23 ++- 5 files changed, 385 insertions(+), 45 deletions(-) [...] --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -288,6 +288,16 @@ enum AVOptionType{ */ #define AV_OPT_FLAG_CHILD_CONSTS(1 << 18) +/** + * The option is an array. + * + * When adding array options to an object, @ref AVOption.offset should refer to + * a pointer corresponding to the option type. The pointer should be immediately + * followed by an unsigned int that will store the number of elements in the + * array. + */ +#define AV_OPT_FLAG_ARRAY (1 << 19) + /** * AVOption */ @@ -313,6 +323,16 @@ typedef struct AVOption { union { int64_t i64; double dbl; + +/** + * This member is always used for AV_OPT_FLAG_ARRAY options. When + * non-NULL, the first character of the string must be the separator to + * be used for (de)serializing lists to/from strings with av_opt_get(), This is quite ugly. Also it breaks the assumption that if the user sets an option value to the default value of the option, than it will work. So let's just remove this feature for now. Eventually I think some new struct should be introduced, e.g. AVOptionExtension, which can be used to specify additional option settings, such as array min/max size, and maybe separator. It would be a lot more clean and future proof than filling the holes in AVOption. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 15/38] lavu/opt: add array options
AVOption.array_max_size is added before AVOption.unit to avoid increasing sizeof(AVOption). --- doc/APIchanges| 3 + libavutil/opt.c | 344 -- libavutil/opt.h | 26 libavutil/tests/opt.c | 34 + tests/ref/fate/opt| 23 ++- 5 files changed, 385 insertions(+), 45 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index d26110b285..371fd2f465 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09 API changes, most recent first: +2024-02-xx - xx - lavu 58.xx.100 - opt.h + Add AV_OPT_FLAG_ARRAY and AVOption.array_max_size. + 2024-02-21 - xx - lavc 60.40.100 - avcodec.h Deprecate AV_INPUT_BUFFER_MIN_SIZE without replacement. diff --git a/libavutil/opt.c b/libavutil/opt.c index ebc8063dc6..88236660a1 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -56,6 +56,77 @@ const AVOption *av_opt_next(const void *obj, const AVOption *last) return NULL; } +static const size_t opt_elem_size[] = { +[AV_OPT_TYPE_FLAGS] = sizeof(unsigned), +[AV_OPT_TYPE_INT] = sizeof(int), +[AV_OPT_TYPE_INT64] = sizeof(int64_t), +[AV_OPT_TYPE_UINT64]= sizeof(uint64_t), +[AV_OPT_TYPE_DOUBLE]= sizeof(double), +[AV_OPT_TYPE_FLOAT] = sizeof(float), +[AV_OPT_TYPE_STRING]= sizeof(char *), +[AV_OPT_TYPE_RATIONAL] = sizeof(AVRational), +[AV_OPT_TYPE_BINARY]= sizeof(uint8_t *), +[AV_OPT_TYPE_DICT] = sizeof(AVDictionary *), +[AV_OPT_TYPE_IMAGE_SIZE]= sizeof(int[2]), +[AV_OPT_TYPE_VIDEO_RATE]= sizeof(AVRational), +[AV_OPT_TYPE_PIXEL_FMT] = sizeof(int), +[AV_OPT_TYPE_SAMPLE_FMT]= sizeof(int), +[AV_OPT_TYPE_DURATION] = sizeof(int64_t), +[AV_OPT_TYPE_COLOR] = sizeof(uint8_t[4]), +#if FF_API_OLD_CHANNEL_LAYOUT +[AV_OPT_TYPE_CHANNEL_LAYOUT]= sizeof(uint64_t), +#endif +[AV_OPT_TYPE_CHLAYOUT] = sizeof(AVChannelLayout), +[AV_OPT_TYPE_BOOL] = sizeof(int), +}; + +static uint8_t opt_array_sep(const AVOption *o) +{ +av_assert1(o->flags & AV_OPT_FLAG_ARRAY); +return o->default_val.str ? o->default_val.str[0] : ','; +} + +static void *opt_array_pelem(const AVOption *o, void *array, unsigned idx) +{ +av_assert1(o->flags & AV_OPT_FLAG_ARRAY); +return (uint8_t *)array + idx * opt_elem_size[o->type]; +} + +static unsigned *opt_array_pcount(void *parray) +{ +return (unsigned *)((void **)parray + 1); +} + +static void opt_free_elem(const AVOption *o, void *ptr) +{ +switch (o->type) { +case AV_OPT_TYPE_STRING: +case AV_OPT_TYPE_BINARY: +av_freep(ptr); +break; + +case AV_OPT_TYPE_DICT: +av_dict_free((AVDictionary **)ptr); +break; + +case AV_OPT_TYPE_CHLAYOUT: +av_channel_layout_uninit((AVChannelLayout *)ptr); +break; + +default: +break; +} +} + +static void opt_free_array(const AVOption *o, void *parray, unsigned *count) +{ +for (unsigned i = 0; i < *count; i++) +opt_free_elem(o, opt_array_pelem(o, *(void **)parray, i)); + +av_freep(parray); +*count = 0; +} + static int read_number(const AVOption *o, const void *dst, double *num, int *den, int64_t *intnum) { switch (o->type) { @@ -580,6 +651,76 @@ FF_ENABLE_DEPRECATION_WARNINGS return AVERROR(EINVAL); } +static int opt_set_array(void *obj, void *target_obj, const AVOption *o, + const char *val, void *dst) +{ +const size_t elem_size = opt_elem_size[o->type]; +const uint8_t sep = opt_array_sep(o); +uint8_t *str = NULL; + +void *elems = NULL; +unsigned nb_elems = 0; +int ret; + +if (val && *val) { +str = av_malloc(strlen(val) + 1); +if (!str) +return AVERROR(ENOMEM); +} + +// split and unescape the string +while (val && *val) { +uint8_t *p = str; +void *tmp; + +if (o->array_max_size && nb_elems >= o->array_max_size) { +av_log(obj, AV_LOG_ERROR, + "Cannot assign more than %u elements to array option %s\n", + o->array_max_size, o->name); +ret = AVERROR(EINVAL); +goto fail; +} + +for (; *val; val++, p++) { +if (*val == '\\' && val[1]) +val++; +else if (*val == sep) { +val++; +break; +} +*p = *val; +} +*p = 0; + +tmp = av_realloc_array(elems, nb_elems + 1, elem_size); +if (!tmp) { +ret = AVERROR(ENOMEM); +goto fail; +} +elems = tmp; + +tmp = opt_array_pelem(o, elems, nb_elems); +memset(tmp, 0, elem_size); + +ret = opt_set_elem(obj, target_obj, o, str, tmp); +if (ret < 0) +