On date Thursday 2011-06-09 21:03:53 +0200, Anton Khirnov wrote: > On Thu, 9 Jun 2011 16:13:07 +0200, Stefano Sabatini > <[email protected]> wrote: > > On date Thursday 2011-06-09 14:24:11 +0200, Anton Khirnov encoded: [...] > > > > > +/* > > > > > + * A convenience function that applies all the options from options > > > > > to obj.
Nit: Apply all the options... (for functions the doxy tells what the function *does* rather than what the function *is*). > > > > > + * > > > > > + * @param obj A struct whose first element is a pointer to AVClass. > > > > > + * @param options Options to process. Will be replaced with an > > > > > AVDictionary > > > > > + * containing all options not found in obj. > > > > > > > > Would it make sense to keep not found options as a separate param? I > > > > mean in the case you want to keep the passed dictionary (seems safer > > > > to me, less side-effects). That would be: > > > > > > > > av_opt_...(void *obj, AVDictionary *options, AVDictionary > > > > **not_found_options, int flags?); > > > > > > > > In case not_found_options is NULL then the dictionary is not filled. > > > > > > > > > Again, i don't see the advantage in this. If the caller wants to > > > preserve the dict, he can do it with a single av_dict_copy. Adding a new > > > parameter seems messy to me. > > > > Re-using options is prone to leaks if you don't remember to explicitely > > free it, having: > > > > int av_opt_set_dict(void *obj, const AVDictionary *options, AVDictionary > > **not_found_options); > > > > is more explicit and you don't have to worry about the resulting > > not-found options dictionary if you don't care about it (just set > > not_found_options to NULL). The only problem is that you need to > > explicitely free options, but this looks like a feature to me, the > > default assumption is that a function won't destroy a param which is > > passed to it (safer, avoids side-effects). > > I don't think it's more explicit. With my API, the caller always manages > the dicts he allocated himself. With what you're suggesting, lavu allocates > a dict for him and he has to remember to free it. > +it's yet another parameter creep. Well I don't like the idea of destroying the passed dictionary and using the same param as input and output, I can see at least one case when this is inconvenient (setting the same options in different contexts), but then since this doesn't look like a common scenario I won't insist if you insist (in these cases hearing a third opinion helps). -- Give me a sleeping pill and tell me your troubles. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
