Re: [FFmpeg-devel] [PATCH v2 5/6] libavutil: Add AVMap

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2044 bytes --]

On Tue, Apr 15, 2025 at 09:14:42PM +0200, Nicolas George wrote:
> Michael Niedermayer (HE12025-04-15):
> > Where exactly would that benefit FFmpeg ?
> > Dictionaries generally are used to move stuff aorund, like metadata or
> > options.
> > Or may be used in the future to keep track of some mappings like
> > timestamps to file positions during demuxing or muxing
> > Or maybe codecs/formats lookup from name
> > None of these are confined to a single local function
> 
> 	AVMap opts = av_map_create(cmp, NULL, NULL);
> 	av_map_add(opts, "threads", "12");
> 	av_map_add(opts, "crf", "18");
> 	av_map_add(opts, "preset", "veryslow");
> 	av_map_assert_small(opts);
> 	ret = av_codec_open3(ctx, codec, opts);

Allocating and creating 12 threads and encoding a x264 video
(i guess this is x264) with "preset", "veryslow" will overshadow the
25 nanoseconds of a malloc() call

or is the goal to avoid the error handling?
I think the 12 threads encoder will fail if the map run out of memory

also in the specific example
av_codec_open3(ctx, codec, NULL);
will no longer work as NULL is a pointer and AVMap here looks like
it would not be


> 
> > i like av_map_new() because "new" is short
> 
> The exact name does not matter to me. I forgot that we usually use
> _alloc() for the case you implemented, so _new() is absolutely fine for
> this.
> 
> av_map_assert_small() should assert that no dynamic allocation happened,
> and at assert level 2 that the internal buffer is less than halfway full.

>
> > and sizeof(AVMap) is not public API
> 
> It should be public API like I did for BPrint, that is my point.

AVMap should be public but the implementation should not be any more
public than needed.
That way users cannot by mistake mess with internals and we can
improve the internal implementation without ABI/API breakage

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".