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

[Michael Niedermayer]

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

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100913/6ef40b4f/attachment.pgp>