[FFmpeg-devel] [PATCH] avformat/ivf: Change the length field to 32 bits

[Calvin Walton]

On Tue, 2019-10-01 at 21:41 +0200, Carl Eugen Hoyos wrote:
> Am Di., 1. Okt. 2019 um 21:35 Uhr schrieb Raphaël Zumer <
> rzumer at tebako.net>:
> > On Tue, 2019-10-01 at 20:09 +0100, Derek Buitenhuis wrote:
> > > On 01/10/2019 18:25, James Almer wrote:
> > > > The value in the unused field will be 0xFFFFFFFF after this
> > > > change
> > > > instead of 0, since you're writing 32 bits as duration instead
> > > > of
> > > > 64
> > > > where the high 32 bits (corresponding to the unused field) are
> > > > zeroed.
> > > > That means the ivf demuxer prior to this patch will read bogus
> > > > duration
> > > > values from ivf files created after this patch.
> > > > 
> > > > Just leave the muxer as is.
> > > 
> > > Why not just write zero?
> > > 
> > > It's, to me, worse to leave a bogus 64-bit write to appease bugs
> > > in
> > > our
> > > own demuxer. It's confusing and misleading for any readers of the
> > > code.
> > 
> > In that case I would prefer changing the initial written value to 0
> > rather than 0xFFFFFFFFFFFFFFFFULL. Writing over the unused bytes
> > twice
> > to get around an old error is a bit odd as well.
> 
> That may needlessly break non-seekable output.

Writing a 0 as the initial value is consistent with the behaviour of
libvpx.

libvpx writes 0 initially:
https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1191
then updates afterwards with the length (if output is seekable):
https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1209

(for reference, the ivf_write_file_header function is here: 
https://github.com/webmproject/libvpx/blob/v1.8.1/ivfenc.c#L16 )

So we need to make sure that ffmpeg can handle 0 values in this field
regardless.

-- 
Calvin Walton <calvin.walton at kepstin.ca>