[FFmpeg-devel] [PATCH 10/15] avformat/matroskaenc: Avoid seeking when writing level 1 elements
jamrial at gmail.com
Tue Apr 2 19:40:17 EEST 2019
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 at ffmpeg.org> wrote:
>> 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 at 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 at ffmpeg.org
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel