[FFmpeg-devel] [PATCH] lavf/mov.c: Guess video codec delay based on PTS while parsing MOV header.

[Hendrik Leppkes]

On Mon, Nov 20, 2017 at 11:06 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Mon, Nov 20, 2017 at 09:58:05PM +0530, Sasi Inguva wrote:
>> On Sun, Nov 19, 2017 at 1:17 AM, Michael Niedermayer <michael at niedermayer.cc
>> > wrote:
>>
>> > On Sat, Nov 18, 2017 at 11:12:17AM -0800, Sasi Inguva wrote:
>> > > Signed-off-by: Sasi Inguva <isasi at google.com>
>> > > ---
>> > >  libavformat/mov.c                | 54 ++++++++++++++++++++++++++++++
>> > ++++++++++
>> > >  tests/fate/mov.mak               |  5 ++++
>> > >  tests/ref/fate/mov-guess-delay-1 |  3 +++
>> > >  tests/ref/fate/mov-guess-delay-2 |  3 +++
>> > >  4 files changed, 65 insertions(+)
>> > >  create mode 100644 tests/ref/fate/mov-guess-delay-1
>> > >  create mode 100644 tests/ref/fate/mov-guess-delay-2
>> > >
>> > > diff --git a/libavformat/mov.c b/libavformat/mov.c
>> > > index fd170baa57..7354652c6e 100644
>> > > --- a/libavformat/mov.c
>> > > +++ b/libavformat/mov.c
>> > > @@ -3213,6 +3213,58 @@ static int64_t add_ctts_entry(MOVStts**
>> > ctts_data, unsigned int* ctts_count, uns
>> > >      return *ctts_count;
>> > >  }
>> > >
>> > > +static void mov_guess_video_delay(MOVContext *c, AVStream* st) {
>> > > +    MOVStreamContext *msc = st->priv_data;
>> > > +    int ind;
>> > > +    int ctts_ind = 0;
>> > > +    int ctts_sample = 0;
>> > > +    int64_t curr_pts = AV_NOPTS_VALUE;
>> > > +    int64_t prev_pts = AV_NOPTS_VALUE;
>> > > +    int64_t prev_max_pts = AV_NOPTS_VALUE;
>> > > +    int num_swaps = 0;
>> > > +
>> > > +    if (st->codecpar->video_delay <= 0 && msc->ctts_data &&
>> > > +        (st->codecpar->codec_id == AV_CODEC_ID_MPEG2VIDEO ||
>> > > +         st->codecpar->codec_id == AV_CODEC_ID_MPEG4 ||
>> > > +         st->codecpar->codec_id == AV_CODEC_ID_VC1 ||
>> > > +         st->codecpar->codec_id == AV_CODEC_ID_H263 ||
>> > > +         st->codecpar->codec_id == AV_CODEC_ID_H264 ||
>> > > +         st->codecpar->codec_id == AV_CODEC_ID_HEVC)) {
>> >
>> > Do all these cases really need this ?
>> >
>> > I just wanted to list all codecs with B-frames. H264, HEVC need it in
>> case, the bitstream_restriction_flag is not set in the bitstream. Other
>> codecs might need it only if the bitstream has been written incorrectly
>> i.e. writing 0 for the delay bit when there are B-frames etc. I can
>> restrict to H264 , HEVC if needed.
>
> yes, please restrict this.
> I very much doubt that the mov/mp4 demuxer has more correct data
> about the codec than the codec itself when such data is needed to
> decode and cannot be ommited.
>
> I kind of find it a poor design choice that this can be ommited in
> h264 & hevc, but theres nothing we can do about this
>

I don't belive hevc needs this value to be set, our decoder certainly
doesn't. It seems to fill it straight from the SPS, without conditions
(and the decoding logic never actually reads it).
Our h264 decoder is the only one that actually needs this value to not
drop frames, most other decoders just set it (potentially for the
generic timestamp stuff?)

- Hendrik