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

[Michael Niedermayer]

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 ?

video_delay = 0 is also a valid value so it can be set to 0 already
intentionally


    > +        st->codecpar->video_delay = 0;
> +        for(ind = 0; ind < st->nb_index_entries && ctts_ind < msc->ctts_count; ++ind) {
> +            curr_pts = st->index_entries[ind].timestamp + msc->ctts_data[ctts_ind].duration;
> +

> +            // This is used as an indication that the previous GOP has ended and a
> +            // new GOP has started.

this is not neccesary a GOP uless i misread the code


> +            if (curr_pts > prev_max_pts) {
> +                st->codecpar->video_delay = FFMIN(FFMAX(st->codecpar->video_delay, num_swaps), 16);
> +                num_swaps = 0;
> +                prev_max_pts = curr_pts;
> +            }
> +
> +            // Compute delay as the no. of "drop"s in PTS inside a GOP.
> +            // Frames: I0 I1 B0 B1 B2
> +            // PTS:     0  4  1  2  3 -> num_swaps = delay = 1 (4->1)
> +            //
> +            // Frames: I0 I1 B1 B0 B2
> +            // PTS:     0  4  2  1  3 -> num_swaps = delay = 2 (4->2, 2->1)

This may be incorret

consider
PTS    0  5  2  1  4  3
BUFFER 0  05 25 25 45 45 5
OUT          0  1  2  3  4  5

So this is a delay = 2 but your code would set 3 i think


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171118/506c757e/attachment.sig>