On Tue, Apr 11, 2017 at 9:35 AM, Anton Khirnov <[email protected]> wrote: > Quoting Vittorio Giovara (2017-04-10 14:44:38) >> On Fri, Apr 7, 2017 at 3:13 PM, James Almer <[email protected]> wrote: >> > On 4/7/2017 3:20 PM, Vittorio Giovara wrote: >> >> On Fri, Apr 7, 2017 at 8:13 PM, James Almer <[email protected]> wrote: >> >>> On 4/7/2017 1:48 PM, Vittorio Giovara wrote: >> >>>> On Fri, Apr 7, 2017 at 2:27 PM, Steve Lhomme <[email protected]> wrote: >> >>>>> + * @note The struct should be allocated with >> >>>>> av_mastering_display_metadata_alloc() >> >>>>> + * and its size is not a part of the public ABI. >> >>>>> + */ >> >>>>> +typedef struct AVMasteringDisplayMetadata { >> >>>>> +} AVMasteringDisplayMetadata; >> >>>>> + >> >>>>> +/** >> >>>>> + * Allocate an AVMasteringDisplayMetadata structure and set its >> >>>>> fields to >> >>>>> + * default values. The resulting struct can be freed using av_freep(). >> >>>>> + * >> >>>>> + * @return An AVMasteringDisplayMetadata filled with default values >> >>>>> or NULL >> >>>>> + * on failure. >> >>>>> + */ >> >>>>> +AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc(void); >> >>>> >> >>>> This signature might be problematic, it should host a size_t* which >> >>>> should be filled with the size of the struct, like it's done for other >> >>>> side data (except stereo3d for historical reasons). >> >>> >> >>> That will mean different signature between projects. >> >>> >> >>> And for that matter, why were you against me trying to add a replacement >> >>> alloc function with this parameter, then? >> >> >> >> Because I don't think it's an issue big enough to warrant an API >> >> change over existing code. >> >> >> >> On the other hand this new code in Libav so hopefully the code should >> >> be fixed before committing and there is no need to break anything. >> >> It's unfortunate that the signature will be different but either >> >> documentation or ABI is currently violated in ffmpeg, there is no need >> >> to replicate this behaviour in libav, in my opinion. >> > >> > We could add a function that returns size for this and every other >> > similar struct, even if they already have an alloc function that >> > also returns the struct size like Spherical. >> > Something like that is in any case needed for av_*_new_side_data(), >> > as those function do their own memory allocation. >> >> Hi James, >> this could be a plausible workaround, which should be done in addition >> to fixing the signature in my opinion. >> Let's wait for other people opinions and ideas on this though. > > Why should we add workarounds when a perfectly working pattern exists > already (the size_t* parameter to the alloc function). If mismatching > signatures are a concern for you then the function could be renamed (its > name is overly long for my taste anyway).
I'd tend to agree, probably on the extreme side: I see little point of having "metadata" in every function name, header and types, it is intrinsic to the information that is being carried, in my opinion. -- Vittorio _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
