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

[Jean-Daniel Dupas]

Le 28 sept. 2010 ? 04:53, Alexander Strange a ?crit :

> 
> 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.

I know I'm late about this one, but wouldn't it be preferable to move this code in libavcore instead ? This code should be usable by all FFmpeg client, not only the built-in command lines utility isn't it ?

-- Jean-Daniel