On Sat, Mar 31, 2018 at 03:39:58PM +0100, Derek Buitenhuis wrote: > On 3/29/2018 7:55 PM, Michael Niedermayer wrote: > >> That is not what i meant > >> > >> what i meant is to align the APIs that handle timespan related information > >> and not have totally different interfaces for each. > >> For example they might share the same struct in some cases to deliver data > >> They do have in common that they all in/export data for a sequence of > >> timespans > > [...] > > >> They also may have in common that some formats store the data interleaved > >> at the time and some store it in a main header. > > Not sure I follow what this has to do with timelines? There is no format that > exists that store timeline data interleaved, as far as I know - it is a > purely theoretical scenario.
that surprises me. But if this case never occurs (or will occur) thats a good thing. Fewer cases to consider. > > >> If this is so the case of collecting all this info from the whole > >> duration of a file to store it in the output file header at the start > >> may be a situation common to multiple of these. > >> A similar API may allow simplifying whichever way we handle this > > [...] > > >> Iam sure there are more advantages of having APIs which deal with > >> similar types of data to be similar ... > > > > an example of an API that serves a similar kind of information but is > > differnt is AVChapter > > I think we should minimize the amount of different public structs that > > describe timespans when that is easily and cleanly possible > > What do you have in mind with regards to 'sharing'? I do not particularly > agree that chapters and edits should share stuff because they both happen to > have > start and end times. They're quite different things, even if some formats have > conflated them (design flaw IMO). I did not think deeply about this but the obvious 3 options are to either give each its own struct and align the APIs used to access them to show this just with one function: AVTimelineList *av_timeline_list_alloc(size_t timeline_count); AVChapterList *av_chapterlist_alloc(size_t chapter_count); AVTimelineMetadataList *av_timeline_metadata_list_alloc(size_t metadata_count); AVTimelineCodecList *av_timeline_codec_list_alloc(size_t codec_count); or to have some more common struct maybe similar to side data. It is after all some kind of side/meta data that is specific to timespans. or to put all in AVFormatContext as normal allocated arrays like chapters are currently. What i find a bit ugly is that each API uses the style popular at the time at which it is added and they are so far each completetly different even though they are semantically handling the same "unit" of data. that is information about "timespans". We have AVChapters which are a public field in AVFormatContext There is AV_PKT_DATA_STRINGS_METADATA which is a binary specified side data and there is the newly proposed timeline which is opaque sidedata with accessor functions Its not the inherent differences of the data that i want to push into a square hole but all the common parts that could be the same and are very different. If 2 things dont fit together iam certainly not intending to suggest to put them together. Nor that 2 things that have reason to be behind differnt APIs should be not. I mainly wanted to point out that our APIs for these things are quite different and that this should be thought about when adding a new API. Its not a specific cleanup or change i have in mind, more so i wanted to call attention to the inconsistency that there is between APIs so it is considered when adding a new API. If you determine that theres nothing that should be changed from what you intended previously, thats perfectly fine with me. Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I am the wisest man alive, for I know one thing, and that is that I know nothing. -- Socrates
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
