On Sat, 7 Jun 2014 09:56:40 +0200, wm4 <[email protected]> wrote: > 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? >
Did it look like i'm arguing for a flag in AVDictionary? That's absolutely not what I meant. My idea is along the lines of having something like an 'event_flags' field in AVFormatContext, AVStream, etc. One of the flags would be 'metadata updated'. On each call to the demuxer (read or seek), lavf would either clear or set this flag to indicate whether this last call updated the metadata or not. (note that I'm not saying it has to be done this way, just that this solution looks a little cleaner to me than the proposed approach. better suggestions are welcome) > > > (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. Not sure what you mean here. The flag proposed above gives you pretty much the same information as the versioning. > 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. > Dunno, the upper layer modifying the metadata strikes me as rather ugly. Can't see a particularly elegant solution though. Maybe have a separate version counter exported as an AVoption? Will have to think about this some more. > > > > 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. What, did you think that it was a good idea for a function that's supposed to check for metadata updates to also modify the metadata? It's also rather weird/fragile that protocol-level metadata gets exported to global _only_ when this update checking function is used. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
