[FFmpeg-devel] [PATCH] ivfdec/ivfenc: Match behaviour of libvpx and chromium

[James Almer]

On 10/1/2019 2:56 PM, Calvin Walton wrote:
> The ffmpeg code read and wrote a 64bit duration field (in timebase units) in the ivf
> header, where the libvpx and chromium code instead use a 32bit frame count field, and
> then 32bits of unused (reserved?) space.
> 
> Switch ffmpeg to match the behaviour of libvpx & chromium.
> 
> Note that libvpx writes 0 to the frame count field when initially writing the header
> then seeks back and overwrites it with the real frame count. ffmpeg used to write
> 0xFFFFFFFF - I've changed the behaviour to match libvpx.
> 
> References:
> https://github.com/webmproject/libvpx/blob/v1.8.1/ivfenc.c#L16
> Which is called from:
> https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1191 (initial header)
> https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1209 (rewrite with frame count)
> And the chromium parser:
> https://chromium.googlesource.com/chromium/src/media/+/1681b9abff73fe0e3d0932aefdab4f039a284d1a/filters/ivf_parser.h
> ---
>  libavformat/ivfdec.c |  3 ++-
>  libavformat/ivfenc.c | 11 ++++-------
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c
> index 40ae464b76..2eaa5164ff 100644
> --- a/libavformat/ivfdec.c
> +++ b/libavformat/ivfdec.c
> @@ -53,7 +53,8 @@ static int read_header(AVFormatContext *s)
>      st->codecpar->height     = avio_rl16(s->pb);
>      time_base.den         = avio_rl32(s->pb);
>      time_base.num         = avio_rl32(s->pb);
> -    st->duration          = avio_rl64(s->pb);
> +    st->nb_frames         = avio_rl32(s->pb);

The demuxer will report N/A as duration after this.

> +    avio_skip(s->pb, 4); // 32 bits unused
>  
>      st->need_parsing      = AVSTREAM_PARSE_HEADERS;
>  
> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
> index adf72117e9..85ca6045ba 100644
> --- a/libavformat/ivfenc.c
> +++ b/libavformat/ivfenc.c
> @@ -22,8 +22,7 @@
>  #include "libavutil/intreadwrite.h"
>  
>  typedef struct IVFEncContext {
> -    unsigned frame_cnt;
> -    uint64_t last_pts, sum_delta_pts;
> +    uint32_t frame_cnt;
>  } IVFEncContext;
>  
>  static int ivf_write_header(AVFormatContext *s)
> @@ -53,7 +52,8 @@ static int ivf_write_header(AVFormatContext *s)
>      avio_wl16(pb, par->height);
>      avio_wl32(pb, s->streams[0]->time_base.den);
>      avio_wl32(pb, s->streams[0]->time_base.num);
> -    avio_wl64(pb, 0xFFFFFFFFFFFFFFFFULL);
> +    avio_wl32(pb, 0); // frame count
> +    avio_wl32(pb, 0); // unused
>  
>      return 0;
>  }
> @@ -66,10 +66,7 @@ static int ivf_write_packet(AVFormatContext *s, AVPacket *pkt)
>      avio_wl32(pb, pkt->size);
>      avio_wl64(pb, pkt->pts);
>      avio_write(pb, pkt->data, pkt->size);
> -    if (ctx->frame_cnt)
> -        ctx->sum_delta_pts += pkt->pts - ctx->last_pts;
>      ctx->frame_cnt++;
> -    ctx->last_pts = pkt->pts;
>  
>      return 0;
>  }
> @@ -83,7 +80,7 @@ static int ivf_write_trailer(AVFormatContext *s)
>          size_t end = avio_tell(pb);
>  
>          avio_seek(pb, 24, SEEK_SET);
> -        avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1));
> +        avio_wl32(pb, ctx->frame_cnt);
>          avio_seek(pb, end, SEEK_SET);
>      }

Similarly, old versions of the ivf demuxer will read bogus duration
values after this change.