On Sat, 19 Mar 2016, Anton Khirnov wrote:

Quoting Martin Storsjö (2016-03-18 13:01:39)
From: Michael Niedermayer <michae...@gmx.at>

This includes documentation and other modifications by
Lukasz Marek.
---
 doc/APIchanges      |  3 +++
 libavutil/opt.c     | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 libavutil/opt.h     | 13 ++++++++++++
 libavutil/version.h |  2 +-
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 20fecb9..aa6f004 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@ libavutil:     2015-08-28

 API changes, most recent first:

+2016-xx-xx - xxxxxxx - lavu 55.7.0 - opt.h
+  Add av_opt_copy().
+
 2016-02-23 - 9200514 - lavf 57.5.0 - avformat.h
   Add AVStream.codecpar, deprecate AVStream.codec.

diff --git a/libavutil/opt.c b/libavutil/opt.c
index 5825a72..75e8970 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -757,6 +757,64 @@ const AVClass *av_opt_child_class_next(const AVClass 
*parent, const AVClass *pre
     return NULL;
 }

+static int opt_size(enum AVOptionType type)
+{
+    switch(type) {
+    case AV_OPT_TYPE_INT:
+    case AV_OPT_TYPE_FLAGS:     return sizeof(int);
+    case AV_OPT_TYPE_INT64:     return sizeof(int64_t);
+    case AV_OPT_TYPE_DOUBLE:    return sizeof(double);
+    case AV_OPT_TYPE_FLOAT:     return sizeof(float);
+    case AV_OPT_TYPE_STRING:    return sizeof(uint8_t*);
+    case AV_OPT_TYPE_RATIONAL:  return sizeof(AVRational);
+    case AV_OPT_TYPE_BINARY:    return sizeof(uint8_t*) + sizeof(int);
+    }
+    return 0;

This should return an error that should be checked by the caller.

Sure

+}
+
+int av_opt_copy(void *dst, const void *src)
+{
+    const AVOption *o = NULL;
+    const AVClass *c;
+    int ret = 0;
+
+    if (!src)
+        return 0;

I'm not sure about this, it looks invalid.

Invalid in which sense? That this function shouldn't ever have to care about that case, that it should be the caller's duty to never try that?

+
+    c = *(AVClass**)src;
+    if (*(AVClass**)dst && c != *(AVClass**)dst)

Why the first check? dst is assumed to be non-NULL anyway, and if it is
NULL, the code lower down will explode.

It's not about whether dst is null or not, but whether the AVClass pointer in dst has been set or not. If it hasn't been set, we don't need to see if it matches the source class.

+        return AVERROR(EINVAL);
+
+    while ((o = av_opt_next(src, o))) {
+        void *field_dst = ((uint8_t*)dst) + o->offset;
+        void *field_src = ((uint8_t*)src) + o->offset;
+        uint8_t **field_dst8 = (uint8_t**)field_dst;
+        uint8_t **field_src8 = (uint8_t**)field_src;
+
+        if (o->type == AV_OPT_TYPE_STRING) {
+            set_string(dst, o, *field_src8, field_dst8);
+            if (*field_src8 && !*field_dst8)
+                ret = AVERROR(ENOMEM);
+        } else if (o->type == AV_OPT_TYPE_BINARY) {
+            int len = *(int*)(field_src8 + 1);
+            if (*field_dst8 != *field_src8)
+                av_freep(field_dst8);
+            *field_dst8 = av_malloc(len);

Didn't we have some objections to zero-size mallocs?

They're kinda icky, yes. In this case, the code is ok (we only check it for being non-null if len as nonzero), but we could avoid it altogether.

// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to