On 4/2/2019 1:23 PM, Hendrik Leppkes wrote: > On Tue, Apr 2, 2019 at 5:55 PM Andreas Rheinhardt via ffmpeg-devel > <ffmpeg-devel@ffmpeg.org> wrote: >> >> Hello, >> >> thanks for taking a look at this. >> >> Hendrik Leppkes: >>> On Tue, Apr 2, 2019 at 3:36 PM Andreas Rheinhardt via ffmpeg-devel >>> <ffmpeg-devel@ffmpeg.org> wrote: >>>> >>>> Up until now, the writing process for level 1 elements (those elements >>>> for which CRC-32 elements are written by default) was this in case the >>>> output was seekable: Write the EBML ID, write an "unkown length" EBML >>>> number of the desired length, then write the element into a dynamic >>>> buffer, then write the dynamic buffer (after possible calculation and >>>> writing of the CRC-element), then seek back to the size element and >>>> overwrite the unknown-size element with the real size. The seeking and >>>> overwriting part has been eliminated by not writing the size initially. >>>> >>> >>> I'm not particularly happy that it stops using start_ebml_master and >>> basically duplicates its functionality. This adds possible maintenance >>> in the future, or hidden bugs. >>> >> 1. start_ebml_master has the disadvantage that the length of the size >> element is chosen in advance; if one doesn't want to use another >> dynamic buffer for every master element (I'm thinking about the >> thousands of cues for which this would be mostly useless as they are >> written with one byte length fields anyway) and doesn't want to waste >> bytes for writing lots of elements (the level 1 elements), then these >> two functions need to be separated. > > I understand why its done, it just doesn't seem .. ideal. > > I didn't check the code entirely, but can this function theoretically > still be used for non-L1 elements after your changes, if one is > desired to have a CRC32 further down the EBML tree? > If not, it might be sensible to rename it.
Any element can have a CRC32 child element, but it's not required and i don't think it's worth trying. Only Level 1 elements should have one, and that's what's currently being done. > >> >> 6. Or do you mean by making "relative positions valid again" that you >> want that the offset in s->pb + the offset of an element in the >> dynamic buffer coincides with the position where this element will >> actually by written in the output? This would effectively mean that >> one can't use the minimum required number of bytes for the cluster's >> size field and instead would have to reserve so many that it always >> works. The only advantage of this would be that the "Writing block at >> offset" log message can really spit out the real output offset of the >> block (which is impossible if the length of the cluster's size field >> hasn't been chosen before writing the block). That's a very slim >> advantage to me. >> > > I forgot that relative positions within an element are supposed to be > counted only from the size element, so nevermind that part. > > - Hendrik > _______________________________________________ > 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". > _______________________________________________ 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".
