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

[Gyan]

On 01-10-2019 11:53 PM, James Almer wrote:
> On 10/1/2019 3:11 PM, Gyan wrote:
>>
>> On 01-10-2019 11:26 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);
>>> +    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);
>>>        }
>> See http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-October/250871.html
>>
>> Gyan
> That patch was already NAKed, and a fixed version sent in replacement.

I intended to link to the start of the thread, not the patch.

Gyan