On Fri, 6 Jun 2014 11:03:46 -0400, Andrew Stone <[email protected]> wrote: > On Fri, Jun 6, 2014 at 7:28 AM, Anton Khirnov <[email protected]> wrote: > > > > On Thu, 5 Jun 2014 13:00:12 -0400, Andrew Stone <[email protected]> wrote: > >> av_metadata_updated checks all metadata dictionaries and checks to see > >> if any option named "metadata" was set inside the context; if it was, > >> the value is merged into current context metadata. > >> > >> Rather than maintaining a list of metadata changes and presenting them to > >> applications, applications must poll the changes to show updates. > >> Typically, > >> this data is only used for display, so updating displayed information with > >> identical information should be acceptable. > >> > >> References: > >> https://lists.libav.org/pipermail/libav-devel/2014-May/059933.html > >> --- > > > > After some thinking about this, I can't help thinking that this versioning > > scheme is an overkill for what you're doing. > > > > Adding version to dicts looks highly tailored to this specific use case and > > not > > likely to be useful for anything else. > > > > And on the demuxer level what you're interested in is not the actual > > numerical > > difference between versions (since it does you no good to know that the > > metadata > > changed multiple times), but simply whether metadata changed or not. For > > that, a > > single-bit flag (per each metadata dict) is enough. The demuxer (or the > > protocol) can always set or clear it, since it knows whether metadata > > changed or > > not. > > While this is certainly doable, I'm not sure what you really gain by > making it a single bit: (1) the compiler will most likely pad it > anyway
My main concern here is certainly not performance or size optimisations, trying to save a couple bytes here would indeed be rather silly. What I'm after is avoiding the spread of single-use APIs in generic structures. Or can you envision some other use case for AVDictitonary versions? > (2) the application will have no way of determining if a dict > has changed as this function will have to reset the flag. I wonder how much this function is necessary at all -- after learning that some metadata has been updated (you don't know which one), you have to loop over all the metadata dicts anyway to pull the updated metadata from them. So replacing this function with a flag is not really more work. > As for > having the demuxer/protocol set the flag, it's nice for it to be set > on any change, rather than pushing that logic down onto all > demuxers/protocols. > Yes, having it work automagically is rather nice. But I think the price for that is rather high. > > As a side bonus, it will also avoid a full copy of the dict on each update > > check. > > Metadata is only copied when it's present, then it's cleared, so it > doesn't do a copy on each update check. Hmm, I missed that av_metadata_updated() resets the protocol-level metadata. That looks rather evil to me -- judging from its name I'd certainly not expect it to have such side effects. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
