[FFmpeg-devel] [PATCH v3] libavutil: Add AVMap

[Nicolas George]

Michael Niedermayer (HE12025-04-19):
> I dont know what you are trying to acomplish here.

I am trying to help make this API as good as possible before it becomes
public and cannot be changed. I am nowhere as good as you in many areas,
but I think I can say that architecture, and in particular API design,
is the thing I am especially good at.

> The key-value part is a fundamental part of the design and it leads to
> multiple advantages.
> it makes the code simpler, faster and enables several features that
> otherwise would be messy/complex
> 
> Also I have no interrest nor time for working on someone elses design
> maybe if that design would convince me but this one does not.

I am not trying to impose my own design. In fact, I think would have
mostly done the same as you

> I think you should resubmit AVWriter and work on it. I think most people
> will respect your design choices unless there are really good technical
> reasons to do something differently.
> The same way I also expect you to respect my choice in AVMap unless theres
> really something better

I am precisely trying to steer a small part of the design towards
something better. You already changed your mind once about the compare
functions, can you not take two steps back, look at it and just consider
changing your mind a second time.

> There is no need for a "user context" pointer in the core API because the user
> specified key is always passed in the first argument of the compare function.
> A struct {Context *c, char *key} can be passed in that if someone really ever
> needs this, noone needed this till now it seems.

There is a need for a user context to the compare function if you want
to have a dictionary in German and a dictionary in French, because the
rules for alphabetical order are slightly different.

But that can be added later.

> You are the mathematican, my terminology is likely off by at least a bit if not more

Sorry for the nitpicking. Math expresses things slightly differently, so
there is no single word to express it in the case of 3-valued compare
functions. You can call it either “partial order” or “total preorder”,
but get rid of the word “strict”.

> Basically by chaining increasingly specific compare functions in code
> (like av_strcasecmp and strcmp in the example)
> it is later possible to stop before all functions are run and have
> wider matches (like case insensitve in  the example)
> 
> Of course for adding elements, always the same and most specific function
> is used so everything is always ordered the same way. Its only the
> find/get stuff that can use these other functions

That makes sense.

> But all this really is under the hood and the user doesnt need to know
> the user just asks for case insensitve and gets it as long as the av_map
> was settup in a way that allows it.

The user needs to understand that if they want to make a map of their
own type and need to provide the compare function.

Let me make you a promise: if you can write an introduction to the API
(as all non-trivial APIs should have) that makes other major developers
say “that was clear”, I will leave the matter alone.

But as is, I still think this is needlessly complex.

Regards,

-- 
  Nicolas George