On date Monday 2023-05-22 11:23:24 +0200, Marvin Scholz wrote: > On 22 May 2023, at 1:52, Stefano Sabatini wrote: > > > On date Monday 2023-05-01 13:44:54 +0200, Marvin Scholz wrote: > >> This new API allows to remove an entry and obtain ownership of the > >> key/value that was associated with the removed entry. > > Thanks for the review! > > >> --- > >> doc/APIchanges | 4 ++++ > >> libavutil/dict.c | 27 +++++++++++++++++++++++++++ > >> libavutil/dict.h | 20 ++++++++++++++++++++ > >> libavutil/tests/dict.c | 34 ++++++++++++++++++++++++++++++++++ > >> libavutil/version.h | 2 +- > >> tests/ref/fate/dict | 12 ++++++++++++ > >> 6 files changed, 98 insertions(+), 1 deletion(-) > >> > >> diff --git a/doc/APIchanges b/doc/APIchanges > >> index 0b609e3d3b..5b807873b7 100644 > >> --- a/doc/APIchanges > >> +++ b/doc/APIchanges > >> @@ -2,6 +2,10 @@ The last version increases of all libraries were on > >> 2023-02-09 > >> > >> API changes, most recent first: > >> > >> +2023-04-29 - xxxxxxxxxx - lavu 58.7.100 - dict.c > >> + Add av_dict_pop() to remove an entry from a dict > >> + and get ownership of the removed key/value. > >> + > >> 2023-04-10 - xxxxxxxxxx - lavu 58.6.100 - frame.h > >> av_frame_get_plane_buffer() now accepts const AVFrame*. > >> > >> diff --git a/libavutil/dict.c b/libavutil/dict.c > >> index f673977a98..ac41771994 100644 > >> --- a/libavutil/dict.c > >> +++ b/libavutil/dict.c > >> @@ -173,6 +173,33 @@ int av_dict_set_int(AVDictionary **pm, const char > >> *key, int64_t value, > >> return av_dict_set(pm, key, valuestr, flags); > >> } > >> > >> +int av_dict_pop(AVDictionary **pm, const char *key, > >> + char **out_key, char **out_value, int flags) > >> +{ > >> + AVDictionary *m = *pm; > >> + AVDictionaryEntry *entry = NULL; > >> + entry = (AVDictionaryEntry *)av_dict_get(m, key, NULL, flags); > >> + if (!entry) > >> + return AVERROR(ENOENT); > >> + > >> + if (out_key) > >> + *out_key = entry->key; > >> + else > >> + av_free(entry->key); > >> + > >> + if (out_value) > >> + *out_value = entry->value; > >> + else > >> + av_free(entry->value); > >> + > >> + *entry = m->elems[--m->count]; > > > >> + if (m && !m->count) { > >> + av_freep(&m->elems); > >> + av_freep(pm); > >> + } > > > > I'm not sure this is the right behavior. Should we clear the > > dictionary when it is empty? What if you need to refill it later? > > >
> Thats the same behaviour as if you use av_dict_set to remove all items > and IMO this should be consistent. > Additionally NULL means an empty AVDictionary, suddenly > having a non-NULL but empty dictionary seems like a very bad idea. Sorry for the slow reply, I see. [...] > >> +/** > >> + * Remove the entry with the given key from the dictionary. > >> + * > > > >> + * Search for an entry matching `key` and remove it, if found. Optionally > > > > Not sure the `foo` syntax is supported by doxygen (and probably we > > should eschew it for consistency with the other doxys). > > > > I tested it locally and it works fine and its much more readable than the > alternatives. > > However if you feel it should be removed I am happy to do that, I have no > strong opinions there. Please let's avoid to add more syntax variance (also I'm not sure when the `var` syntax was introduced). [...] Should we also support the case with multiple same-key values? Also maybe we should mention that this operation might alterate the order of the entries (unless we add a new flag to shift the trailing data when an entry is removed). Another general question, since I see that dict.h is deprecated, do you think it might be possible to switch to tree.h? _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".