From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTPS id B2B434D3B9
for <ffmpegdev@gitmailbox.com>; Thu, 17 Apr 2025 17:57:59 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E9DCA687D57;
Thu, 17 Apr 2025 20:57:53 +0300 (EEST)
Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 738B4687BAE
for <ffmpeg-devel@ffmpeg.org>; Thu, 17 Apr 2025 20:57:47 +0300 (EEST)
X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org )
Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80])
by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 53HHvkdj030014
for <ffmpeg-devel@ffmpeg.org>; Thu, 17 Apr 2025 19:57:46 +0200
Received: by phare.normalesup.org (Postfix, from userid 1001)
id B08AF2EFD9; Thu, 17 Apr 2025 19:57:46 +0200 (CEST)
Date: Thu, 17 Apr 2025 19:57:46 +0200
From: Nicolas George <george@nsup.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <aAFBGsbuU5SG2ma4@phare.normalesup.org>
References: <20250417165241.3266643-1-michael@niedermayer.cc>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <20250417165241.3266643-1-michael@niedermayer.cc>
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3
(nef.ens.fr [129.199.96.32]); Thu, 17 Apr 2025 19:57:47 +0200 (CEST)
Subject: Re: [FFmpeg-devel] [PATCH v3] libavutil: Add AVMap
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/aAFBGsbuU5SG2ma4@phare.normalesup.org/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
Michael Niedermayer (HE12025-04-17):
> +AVMap *av_map_new(AVMapCompareFunc cmp_keyvalue, int cmp_flags, AVMapCopyFunc copy, AVMapFreeFunc freef)
> +{
> + AVMap *s = av_mallocz(sizeof(*s));
> + if (!s)
> + return NULL;
> +
> + s->copy = copy;
> + s->freef = freef;
> +
> + av_map_add_cmp_func(s, cmp_keyvalue, cmp_flags);
> +
> + return s;
> +}
If you do not want to do the version where dynamic allocation can be
avoided, would you at least consider using _alloc() for this? It is only
to letters more, it is consistent with the rest of the API and it would
leave _new() available if somebody adds the lighter version.
I really do not understand why you do not want to do this: it is a very
useful low-hanging fruit, and it is quite elegant and fun to implement.
Alas, you did not reply to my last round of arguments.
> + * @param keyvalue_cmp compare function, will be passed the key + value concatenated.
Please no. Pass the key and the value separately to the compare
function.
> +/**
> + * Add a compatible compare function to the map.
> + * The function will later be selected by using AV_MAP_CMP_* flags
> + *
> + * Functions must be added from most specific to least specific, that is if a AVMap is build
> + * with a case insensitive compare no case sensitive compare functions can be added. This is
> + * to avoid building non functional AVMaps.
> + *
> + *
> + * @see av_map_new
> + *
> + * @param cmp compare function to be added.
> + * cmp(a,b) must return 0 or be equal to the previously added compare function for (a,b), if it returns 0 it also must do so for all
> + * elements between a and b
> + *
> + * @param cmp_flags a combination of AV_MAP_CMP_*, note key/keyvalue and truncated vs non truncated
> + * are mandatory to be specified
> + *
> + * @return a negative error code if the cmp_flags are illegal or unsupported for this AVMap
> + * If you know your flags are valid, then you dont need to check the return
> + */
> +int av_map_add_cmp_func(AVMap *m, AVMapCompareFunc cmp, int cmp_flags);
This whole thing with multiple compare functions makes the code quite
hard to understand. And not just the code: it makes the API itself hard
to understand.
I think the practical goal it server could be achieved in a simpler way.
If I understand correctly, it servers to allow the equivalent of
AV_DICT_IGNORE_SUFFIX and such. It would be simpler to have a flag to
av_map_find() to let it return the first key >= to the searched key.
Regards,
--
Nicolas George
_______________________________________________
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".