[2/3] opt: Add flag to return NULL when applicable in av_opt_get

Message ID 1449560533-29152-2-git-send-email-martin@martin.st
State New
Headers show

Commit Message

Martin Storsjö Dec. 8, 2015, 7:42 a.m.
From: Rodger Combs <rodger.combs@gmail.com>

---
 doc/APIchanges      |  3 +++
 libavutil/opt.c     | 12 ++++++++++--
 libavutil/opt.h     | 10 ++++++++++
 libavutil/version.h |  2 +-
 4 files changed, 24 insertions(+), 3 deletions(-)

Comments

Luca Barbato Dec. 8, 2015, 10:03 a.m. | #1
On 08/12/15 08:42, Martin Storsjö wrote:
> From: Rodger Combs <rodger.combs@gmail.com>
> 
> ---
>  doc/APIchanges      |  3 +++
>  libavutil/opt.c     | 12 ++++++++++--
>  libavutil/opt.h     | 10 ++++++++++
>  libavutil/version.h |  2 +-
>  4 files changed, 24 insertions(+), 3 deletions(-)
> 

Should be Ok.
Luca Barbato Dec. 8, 2015, 10:14 a.m. | #2
On 08/12/15 08:42, Martin Storsjö wrote:
> From: Rodger Combs <rodger.combs@gmail.com>
> 
> ---
>  doc/APIchanges      |  3 +++
>  libavutil/opt.c     | 12 ++++++++++--
>  libavutil/opt.h     | 10 ++++++++++
>  libavutil/version.h |  2 +-
>  4 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 4c5e32e..bbffbb4 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil:     2015-08-28
>  
>  API changes, most recent first:
>  
> +2015-xx-xx - xxxxxxx - lavu 55.4.0 - opt.h
> +  Add AV_OPT_ALLOW_NULL

Actually, wouldn't be simpler to return a not-found error?

lu
Hendrik Leppkes Dec. 8, 2015, 10:22 a.m. | #3
On Tue, Dec 8, 2015 at 11:14 AM, Luca Barbato <lu_zero@gentoo.org> wrote:
> On 08/12/15 08:42, Martin Storsjö wrote:
>> From: Rodger Combs <rodger.combs@gmail.com>
>>
>> ---
>>  doc/APIchanges      |  3 +++
>>  libavutil/opt.c     | 12 ++++++++++--
>>  libavutil/opt.h     | 10 ++++++++++
>>  libavutil/version.h |  2 +-
>>  4 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 4c5e32e..bbffbb4 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,9 @@ libavutil:     2015-08-28
>>
>>  API changes, most recent first:
>>
>> +2015-xx-xx - xxxxxxx - lavu 55.4.0 - opt.h
>> +  Add AV_OPT_ALLOW_NULL
>
> Actually, wouldn't be simpler to return a not-found error?
>

But the option exists, its just set to NULL. Returning an not-found
would be rather confusing, as you couldn't differentiate.
Luca Barbato Dec. 8, 2015, 1:22 p.m. | #4
On 08/12/15 11:22, Hendrik Leppkes wrote:
> On Tue, Dec 8, 2015 at 11:14 AM, Luca Barbato <lu_zero@gentoo.org> wrote:
>> On 08/12/15 08:42, Martin Storsjö wrote:
>>> From: Rodger Combs <rodger.combs@gmail.com>
>>>
>>> ---
>>>  doc/APIchanges      |  3 +++
>>>  libavutil/opt.c     | 12 ++++++++++--
>>>  libavutil/opt.h     | 10 ++++++++++
>>>  libavutil/version.h |  2 +-
>>>  4 files changed, 24 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>> index 4c5e32e..bbffbb4 100644
>>> --- a/doc/APIchanges
>>> +++ b/doc/APIchanges
>>> @@ -13,6 +13,9 @@ libavutil:     2015-08-28
>>>
>>>  API changes, most recent first:
>>>
>>> +2015-xx-xx - xxxxxxx - lavu 55.4.0 - opt.h
>>> +  Add AV_OPT_ALLOW_NULL
>>
>> Actually, wouldn't be simpler to return a not-found error?
>>
> 
> But the option exists, its just set to NULL. Returning an not-found
> would be rather confusing, as you couldn't differentiate.

My problem is that it gets another special case, while you'd like to
query if the option exists or the option is set.

So I'd use a flag to differentiate if we do not want to have two
different errors for not found, but I'd rather not have something
special just for 2 types and not the others.

lu

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 4c5e32e..bbffbb4 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@  libavutil:     2015-08-28
 
 API changes, most recent first:
 
+2015-xx-xx - xxxxxxx - lavu 55.4.0 - opt.h
+  Add AV_OPT_ALLOW_NULL
+
 2015-xx-xx - xxxxxxx - lavc 57.11.0 - avcodec.h dirac.h
   xxxxxxx - Add av_packet_add_side_data().
   xxxxxxx - Add AVCodecContext.coded_side_data.
diff --git a/libavutil/opt.c b/libavutil/opt.c
index b3435e0..eb37c60 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -348,12 +348,20 @@  int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val)
     case AV_OPT_TYPE_DOUBLE:    ret = snprintf(buf, sizeof(buf), "%f" ,     *(double *)dst);break;
     case AV_OPT_TYPE_RATIONAL:  ret = snprintf(buf, sizeof(buf), "%d/%d",   ((AVRational*)dst)->num, ((AVRational*)dst)->den);break;
     case AV_OPT_TYPE_STRING:
-        if (*(uint8_t**)dst)
+        if (*(uint8_t**)dst) {
             *out_val = av_strdup(*(uint8_t**)dst);
-        else
+        } else if (search_flags & AV_OPT_ALLOW_NULL) {
+            *out_val = NULL;
+            return 0;
+        } else {
             *out_val = av_strdup("");
+        }
         return *out_val ? 0 : AVERROR(ENOMEM);
     case AV_OPT_TYPE_BINARY:
+        if (!*(uint8_t**)dst && (search_flags & AV_OPT_ALLOW_NULL)) {
+            *out_val = NULL;
+            return 0;
+        }
         len = *(int*)(((uint8_t *)dst) + sizeof(uint8_t *));
         if ((uint64_t)len*2 + 1 > INT_MAX)
             return AVERROR(EINVAL);
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 6d3f1fe..7214e68 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -391,6 +391,12 @@  int av_opt_eval_q     (void *obj, const AVOption *o, const char *val, AVRational
 #define AV_OPT_SEARCH_FAKE_OBJ   0x0002
 
 /**
+ *  In av_opt_get, return NULL if the option has a pointer type and is set to
+ *  NULL, rather than returning an empty string.
+ */
+#define AV_OPT_ALLOW_NULL        0x0004
+
+/**
  * Look for an option in an object. Consider only options which
  * have all the specified flags set.
  *
@@ -520,6 +526,10 @@  int av_opt_set_dict_val(void *obj, const char *name, const AVDictionary *val, in
  */
 /**
  * @note the returned string will be av_malloc()ed and must be av_free()ed by the caller
+ *
+ * @note if AV_OPT_ALLOW_NULL is set in search_flags in av_opt_get, and the option has
+ * AV_OPT_TYPE_STRING or AV_OPT_TYPE_BINARY and is set to NULL, *out_val will be set
+ * to NULL instead of an allocated empty string.
  */
 int av_opt_get         (void *obj, const char *name, int search_flags, uint8_t   **out_val);
 int av_opt_get_int     (void *obj, const char *name, int search_flags, int64_t    *out_val);
diff --git a/libavutil/version.h b/libavutil/version.h
index 7131122..802a445 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -54,7 +54,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR 55
-#define LIBAVUTIL_VERSION_MINOR  3
+#define LIBAVUTIL_VERSION_MINOR  4
 #define LIBAVUTIL_VERSION_MICRO  0
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \