On Sat, 07 Jun 2014 08:00:43 +0200 Anton Khirnov <[email protected]> wrote:
> > 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? If you're arguing like this, even a single flag is strange. Why not put the flag outside of the dict? > > (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. Ideally, you'd get to know what changed without having to do your own change detection. Checking a map for equality is relatively inconvenient. And maybe it should also be possible to differentiate between no update and update to exactly the same data? (Although maybe this is not useful or needed in practice.) > > 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 for protocol -> demuxer transport, maybe you could just clear the metadata in the protocol after copying it from protocol to demuxer. If the protocol sets the metadata again to non-empty, it means that it has changed. > > > 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. > Oh, so much for that. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
