From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id D1E2A45872 for ; Fri, 26 May 2023 06:05:56 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D669F68C1D9; Fri, 26 May 2023 09:05:53 +0300 (EEST) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BCE3768C116 for ; Fri, 26 May 2023 09:05:47 +0300 (EEST) Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-3f68fc6b479so3993355e9.2 for ; Thu, 25 May 2023 23:05:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685081146; x=1687673146; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:from:to:cc:subject :date:message-id:reply-to; bh=ymIATBUHytlcE/DLs64iguBaaxO1MejQanlx3lh65GU=; b=MtNbXXvrJLEH3z5zNjALvcjQx29/WPznLShop+Je08/JWuI9g49je5UZa/yhGx0+yy ypEOCeM4d66loyozLM38eM4BrFDEU2AsAKh9Bi5yeh6Q9AxloxMFo6z/CPhkQGtQtQtY uUG+KBFL0EzVDKFw30wU68KpdAAgpemPojCpQMZO0iIAGm9IJGHHF/swpySQTdqA72Aa BQCL7vzwH2t+zdp+WQIdgr+B4TpmJd/2T3bDUJ2LUVuD3tFazZx8t8ER2BVpYnobyjl2 W2KullQnOMny4oJjefPmEC6TQqpUCDv6p+Hmp7aANQZFKJNzsVhLCqNlgTMSn0vF/IxU OPaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685081146; x=1687673146; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=ymIATBUHytlcE/DLs64iguBaaxO1MejQanlx3lh65GU=; b=PsD5rhTjGZkpPW/BDblngZTAhSyGAEVzZAEV6OIJ+bOnemcb2pQmf3/TjDGS984Jyv Y7heTXpBYoKxcbSW6WeQtgibaGS+KeXDCzuXqYZh6G9O36HFLNepRRJH700ijNblttSW 32JBYuTGO+dRALycM51mWBA5imsAEMPJE+cSzpQ523iy4wRiBWo2a7OtR5vQ4PAjmwoa UtCwhL0INkJ0DUKo6CKK5ig5tzA0+EOTeLkoHOQBn+GFpZDelQhNUiGBYInPw87vbo+W dVgQD+/H8YlOXc/ZYVYjN9cE1TG0AusrbalowPfVB/fcOIr/WdNqMY3zwwKdHqZigMQS pBrw== X-Gm-Message-State: AC+VfDw/K4p4IAoVVvOyRNvccThs+td9hT5rP9QQ61GnWvBy9WweYM/W zuuGvvf89RwvvygmrZvRsLHz4W+99HR7Kw== X-Google-Smtp-Source: ACHHUZ6064wCrYAOs/8it9hmAsu8ms+SYjVzb0wA94udyvbSftFqv4jyeDzU8hlyJ3Oq7BoOq9213Q== X-Received: by 2002:a7b:c8d5:0:b0:3f6:496:e240 with SMTP id f21-20020a7bc8d5000000b003f60496e240mr558976wml.27.1685081146155; Thu, 25 May 2023 23:05:46 -0700 (PDT) Received: from mariano ([91.254.67.164]) by smtp.gmail.com with ESMTPSA id t16-20020a5d4610000000b002f22c44e974sm3883167wrq.102.2023.05.25.23.05.45 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 May 2023 23:05:45 -0700 (PDT) Received: by mariano (Postfix, from userid 1000) id 4EF7ABFB73; Fri, 26 May 2023 08:05:44 +0200 (CEST) Date: Fri, 26 May 2023 08:05:44 +0200 From: Stefano Sabatini To: FFmpeg development discussions and patches Message-ID: <20230526060544.GB3241@mariano> Mail-Followup-To: FFmpeg development discussions and patches References: <20230501114456.13898-1-epirat07@gmail.com> <20230521235235.GC14958@mariano> <8CA999C7-1502-4FAB-ACD9-9BC02ED68058@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <8CA999C7-1502-4FAB-ACD9-9BC02ED68058@gmail.com> User-Agent: Mutt/1.13.2 (2019-12-18) Subject: Re: [FFmpeg-devel] [PATCH 1/3] avutil/dict: add av_dict_pop X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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-request@ffmpeg.org with subject "unsubscribe".