On Wed, Jul 30, 2014 at 09:22:27PM +0200, wm4 wrote:
> > /**
> > @@ -123,6 +125,12 @@ int av_dict_count(const AVDictionary *m);
> > int av_dict_set(AVDictionary **pm, const char *key, const char *value, int
> > flags);
> >
> > /**
> > + * Convenience wrapper for av_dict_set that converts the value to a string
> > + * and stores it.
> > + */
> > +int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value, int
> > flags);
> > +
> > +/**
>
> I think that's a pretty good idea, but you could make it more general
> by adding something like av_dict_set_fmt(), which adds a
> snprintf-formatted value. av_dict_set_int could then call this too.
It seemed to me that there just wouldn't be enough places to really
justify it.
A generic function will still allow the various variations of
using %d vs. %u vs. %PRId64 etc. for doing essentially the same thing.
Also since I wouldn't know the length beforehand, I'd have to use
something like our asprintf.
However to do that efficiently, no "no strdup" flag should be used.
But then you run into the memleak issue my other patch is about, and
that would have to be resolved first.
And at that point I concluded that maybe this is a solution with fairly good
complexity/benefit ratio, something I am not convinced of for a more
generic one.
Especially if the difference would likely be between (more or less)
av_dict_set(d, "KEY", flags, "%xyz", somevar);
and
av_dict_set(d, "KEY", av_asprintf("%xyz", somevar), flags | DONT_STRDUP_VAL);
Note that I think we have code that does things much more messily than
the latter one for no good reason, so that might be worth looking at.
More or less that was my thought process, though I might have missed
some things.
> Also, maybe it would be better to split the libavutil and the other
> changes into 2 commits.
I guess I just wanted to have the proof of usefulness and the API
together, but I admit just mailing them out together is probably good
enough.
I can do that later once there aren't further comments if it's preferred.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
