[FFmpeg-devel] [PATCH 1/5] Move timestamp correction code from ffplay to cmdutils

[Alexander Strange]

On Sep 13, 2010, at 5:16 PM, Michael Niedermayer wrote:

> On Sun, Sep 12, 2010 at 11:05:06PM -0400, Alexander Strange wrote:
>> On Tue, Jul 27, 2010 at 8:52 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>> On Mon, Jul 26, 2010 at 01:16:06PM -0700, Alexander Strange wrote:
>>>> ---
>>>>  cmdutils.c |   27 +++++++++++++++++++++++++++
>>>>  cmdutils.h |   24 ++++++++++++++++++++++++
>>>>  ffplay.c   |   31 ++++++-------------------------
>>>>  3 files changed, 57 insertions(+), 25 deletions(-)
>>>> 
>>>> diff --git a/cmdutils.c b/cmdutils.c
>>>> index 978b73c..cd0b194 100644
>>>> --- a/cmdutils.c
>>>> +++ b/cmdutils.c
>>> 
>>> 
>>>> 
>>>> -            is->last_dts_for_fault_detection=
>>>> -            is->last_pts_for_fault_detection= INT64_MIN;
>>>> +            init_pts_correction(&is->pts_ctx);
>>> 
>>>> @@ -663,3 +663,30 @@ int read_file(const char *filename, char **bufptr, size_t *size)
>>>>      fclose(f);
>>>>      return 0;
>>>>  }
>>>> +
>>>> +void init_pts_correction(PtsCorrectionContext *ctx)
>>>> +{
>>>> +    ctx->num_faulty_pts = ctx->num_faulty_dts = 0;
>>>> +    ctx->last_pts = ctx->last_dts = AV_NOPTS_VALUE;
>>> 
>>> INT64_MIN is correct, AV_NOPTS_VALUE is wrong, the code depends on it being
>>> the smallest integer, it does not check == AV_NOPTS_VALUE
>> 
>> Fixed. Not sure where that came from.
>> 
>>>> +}
>>>> +
>>>> +int64_t guess_correct_pts(PtsCorrectionContext *ctx, int64_t reordered_pts, int64_t dts)
>>>> +{
>>>> +    int64_t pts = AV_NOPTS_VALUE;
>>>> +
>>>> +    if (dts != AV_NOPTS_VALUE) {
>>>> +        ctx->num_faulty_dts += dts <= ctx->last_dts;
>>>> +        ctx->last_dts = dts;
>>>> +    }
>>>> +    if (reordered_pts != AV_NOPTS_VALUE) {
>>>> +        ctx->num_faulty_pts += reordered_pts <= ctx->last_pts;
>>>> +        ctx->last_pts = reordered_pts;
>>>> +    }
>>>> +    if ((ctx->num_faulty_pts<ctx->num_faulty_dts || dts == AV_NOPTS_VALUE)
>>>> +       && reordered_pts != AV_NOPTS_VALUE)
>>>> +        pts = reordered_pts;
>>>> +    else
>>>> +        pts = dts;
>>>> +
>>>> +    return pts;
>>>> +}
>>> 
>>> you are loosing the decoder_reorder_pts code here
>> 
>> Merged the next patch into this one.
>> 
>>>> diff --git a/cmdutils.h b/cmdutils.h
>>>> index d48abab..f2ad34a 100644
>>>> --- a/cmdutils.h
>>>> +++ b/cmdutils.h
>>>> @@ -220,4 +220,28 @@ int read_yesno(void);
>>>>   */
>>>>  int read_file(const char *filename, char **bufptr, size_t *size);
>>>> 
>>>> +typedef struct {
>>>> +    int64_t num_faulty_pts; /// Number of incorrect PTS values so far
>>>> +    int64_t num_faulty_dts; /// Number of incorrect DTS values so far
>>>> +    int64_t last_pts;       /// PTS of the last frame
>>>> +    int64_t last_dts;       /// DTS of the last frame
>>>> +} PtsCorrectionContext;
>>>> +
>>>> +/**
>>>> + * Resets the state of the PtsCorrectionContext.
>>>> + */
>>>> +void init_pts_correction(PtsCorrectionContext *ctx);
>>>> +
>>>> +/**
>>>> + * Attempts to guess proper monotonic timestamps for decoded video frames
>>>> + * which might have incorrect times.
>>>> + *
>>>> + * @param pts The pts field of the decoded AVPacket, as passed through
>>>> + * AVCodecContext.reordered_opaque
>>>> + * @param dts The dts field of the decoded AVPacket
>>> 
>>>> + * @return Whichever of pts or dts is more correct; will not be less than
>>>> + * or equal to the previous value. May be AV_NOPTS_VALUE.
>>> 
>>> this is impossible, mpeg-ps/ts have timestamp discontinuities and thus the
>>> values may very well be less than previous
>> 
>> Changed the comment.
>> 
>> Note right now the function only returns one of the input values (so
>> @return mentions that), but I think it might need to do more in the
>> case of files that come out with duplicate pts, like that
>> mpeg+mpeg2video+ac3++non_monotone_timestamps.mpg. But I don't want to
>> hide too many future bugs in lavf, so I'm not sure that's a good idea
>> now.
> 
>> cmdutils.c |   27 +++++++++++++++++++++++++++
>> cmdutils.h |   24 ++++++++++++++++++++++++
>> ffplay.c   |   35 ++++++++++++-----------------------
>> 3 files changed, 63 insertions(+), 23 deletions(-)
>> f917f2e1bf6e7633b7030e37b08d24c20f9fd655  0001-Move-timestamp-correction-code-from-ffplay-to-cmduti.patch
>> From 86434ec1c52877f3d1b1a98319ee0b56f551a442 Mon Sep 17 00:00:00 2001
>> From: Alexander Strange <astrange at ithinksw.com>
>> Date: Sat, 26 Jun 2010 21:21:29 -0700
>> Subject: [PATCH 1/3] Move timestamp correction code from ffplay to cmdutils
> 
> ok if tested

Applied.