[FFmpeg-devel] [PATCH 10/15] avformat/matroskaenc: Avoid seeking when writing level 1 elements

Hendrik Leppkes h.leppkes at gmail.com
Tue Apr 2 19:23:27 EEST 2019


On Tue, Apr 2, 2019 at 5:55 PM Andreas Rheinhardt via ffmpeg-devel
<ffmpeg-devel at 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 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.

>
> 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


More information about the ffmpeg-devel mailing list