Quoting James Almer (2021-03-25 14:37:20) > On 3/25/2021 8:55 AM, Nicolas George wrote: > > Same situation as av_add_index_entry(). And if you look at the signature > of the (3) function in my last proposal, i kept the av_index_* namespace > free precisely in case a new API in the future needs to be added for > this reason, for both add() and get(). One that could work directly with > the AVIndexEntry struct after the internal entries array was redesigned, > perhaps using AVTreeNode, so returned pointers or entries are safer to > handle.
On the topic of av_add_index_entry() - is there any reason that function should be public? Seems like it's internal-use only. > > > > > Option (4) has the obvious practical drawback that misusing the API > > causes undefined behavior. > > > > The constraint of using a pointer immediately on risk of undefined > > behavior is actually a frequent one, in FFmpeg but also in C at large: > > gethosbtyname, localtime, etc. > > > > For me, that makes it approximately on par with the risk of messing the > > order of the many arguments. > > > > Which leaves more typing, NULL checks overhead or useless variables > > (still more typing). > > > > It is tiny, I have no trouble admitting, but it is tiny in favor of one > > solution. > > > > If you do not agree with these estimates, please explain exactly where. > > I don't know if you remember how in this one imgutils patch i sent some > time ago i was against functions with tons of output parameters, because > i considered it ugly and typical of enterprise software API. That hasn't > changed. But here, being the exact counterpart of an existing add() > function put it above the other approach i dislike slightly more of > returning an internal pointer and not being able to tell the user just > what may invalidate it. > > > > >> If some other developer wants to chime in and comment which approach they > >> prefer, then that would be ideal. FWIW in this specific case exporting a short-lived pointer seems less problematic than the other options. But on the other hand I wonder about exporting AVIndexEntry exactly as is internally. Are all these fields useful or even meaningful to external callers? Perhaps we could make a new struct that would export only those fields people actually use. And have the new API return a pointer to something like AVFormatInternal.index_entry_for_the_caller. Also a naming note - I'd prefer the function names to start with avformat, so it's clearer where they belong. "index" can mean many different things. -- Anton Khirnov _______________________________________________ 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".
