[FFmpeg-devel] [PATCH] Implement guessed_pts in avcodec_decode_video2.

Michael Niedermayer michaelni
Tue Feb 1 21:56:34 CET 2011


On Sun, Jan 30, 2011 at 08:28:39PM +0100, Nicolas George wrote:
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  cmdutils.c           |   27 ---------------------------
>  cmdutils.h           |   24 ------------------------
>  ffmpeg.c             |   14 ++++++--------
>  ffplay.c             |   10 +++-------
>  libavcodec/avcodec.h |   20 ++++++++++++++++++++
>  libavcodec/utils.c   |   43 ++++++++++++++++++++++++++++++++++++++++++-
>  6 files changed, 71 insertions(+), 67 deletions(-)
> 
> This patch moves the guess_correct_pts function from cmdutils to libavcodec.
> The feature is available through a new field in AVFrame, guessed_pts.
> 
> make test and ffplay still work, but I would like to have some more time to
> read it carefully, but I have things to do in the next few days, and this
> feature was discussed in another thread, so here the current version. It
> will conflict with some patches that will certainly soon be applied, so I
> will update and submit it again anyway.
> 
> Regards,
> 
> -- 
>   Nicolas George
> 
> diff --git a/cmdutils.c b/cmdutils.c
> index 58fe85c..8da138a 100644
> --- a/cmdutils.c
> +++ b/cmdutils.c
> @@ -739,33 +739,6 @@ int read_file(const char *filename, char **bufptr, size_t *size)
>      return 0;
>  }
>  
> -void init_pts_correction(PtsCorrectionContext *ctx)
> -{
> -    ctx->num_faulty_pts = ctx->num_faulty_dts = 0;
> -    ctx->last_pts = ctx->last_dts = INT64_MIN;
> -}
> -
> -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;
> -}
> -
>  FILE *get_preset_file(char *filename, size_t filename_size,
>                        const char *preset_name, int is_path, const char *codec_name)
>  {
> diff --git a/cmdutils.h b/cmdutils.h
> index c3d8a42..b35b99b 100644
> --- a/cmdutils.h
> +++ b/cmdutils.h
> @@ -235,30 +235,6 @@ 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;
> -
> -/**
> - * Reset the state of the PtsCorrectionContext.
> - */
> -void init_pts_correction(PtsCorrectionContext *ctx);
> -
> -/**
> - * Attempt to guess proper monotonic timestamps for decoded video frames
> - * which might have incorrect times. Input timestamps may wrap around, in
> - * which case the output will as well.
> - *
> - * @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 one of the input values, may be AV_NOPTS_VALUE
> - */
> -int64_t guess_correct_pts(PtsCorrectionContext *ctx, int64_t pts, int64_t dts);
> -
>  /**
>   * Get a file corresponding to a preset file.
>   *
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 691b73e..51f24b8 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -320,7 +320,6 @@ typedef struct AVInputStream {
>      int64_t       next_pts;  /* synthetic pts for cases where pkt.pts
>                                  is not defined */
>      int64_t       pts;       /* current pts */
> -    PtsCorrectionContext pts_ctx;
>      int is_start;            /* is 1 at the start and after a discontinuity */
>      int showed_multi_packet_warning;
>      int is_past_recording_time;
> @@ -1470,7 +1469,6 @@ static int output_packet(AVInputStream *ist, int ist_index,
>      void *buffer_to_free;
>      static unsigned int samples_size= 0;
>      AVSubtitle subtitle, *subtitle_to_free;
> -    int64_t pkt_pts = AV_NOPTS_VALUE;
>  #if CONFIG_AVFILTER
>      int frame_available;
>  #endif
> @@ -1486,6 +1484,8 @@ static int output_packet(AVInputStream *ist, int ist_index,
>          av_init_packet(&avpkt);
>          avpkt.data = NULL;
>          avpkt.size = 0;
> +        avpkt.dts = av_rescale_q(ist->next_pts, AV_TIME_BASE_Q,
> +                                 ist->st->time_base);
>          goto handle_eof;
>      } else {
>          avpkt = *pkt;
> @@ -1493,8 +1493,6 @@ static int output_packet(AVInputStream *ist, int ist_index,
>  
>      if(pkt->dts != AV_NOPTS_VALUE)
>          ist->next_pts = ist->pts = av_rescale_q(pkt->dts, ist->st->time_base, AV_TIME_BASE_Q);
> -    if(pkt->pts != AV_NOPTS_VALUE)
> -        pkt_pts = av_rescale_q(pkt->pts, ist->st->time_base, AV_TIME_BASE_Q);
>  
>      //while we have more to decode or while the decoder did output something on EOF
>      while (avpkt.size > 0 || (!pkt && ist->next_pts != ist->pts)) {

> @@ -1547,8 +1545,6 @@ static int output_packet(AVInputStream *ist, int ist_index,
>                      decoded_data_size = (ist->st->codec->width * ist->st->codec->height * 3) / 2;
>                      /* XXX: allocate picture correctly */
>                      avcodec_get_frame_defaults(&picture);
> -                    ist->st->codec->reordered_opaque = pkt_pts;
> -                    pkt_pts = AV_NOPTS_VALUE;
>  
>                      ret = avcodec_decode_video2(ist->st->codec,
>                                                  &picture, &got_picture, &avpkt);

the "pkt_pts" = AV_NOPTS_VALUE; should stay, aka the pts should be reset after
use.


> @@ -1559,7 +1555,10 @@ static int output_packet(AVInputStream *ist, int ist_index,
>                          /* no picture yet */
>                          goto discard_packet;
>                      }
> -                    ist->next_pts = ist->pts = guess_correct_pts(&ist->pts_ctx, picture.reordered_opaque, ist->pts);
> +                    ist->next_pts = ist->pts =
> +                        picture.guessed_pts == AV_NOPTS_VALUE ? AV_NOPTS_VALUE :
> +                        av_rescale_q(picture.guessed_pts, ist->st->time_base,
> +                                     AV_TIME_BASE_Q);;

double ;;
also this has scaling issues, as ist->*pts are effectively scaled back and forth
between 2 timebases all the time

also, even though independant of this patch i think a av_rescale_q that
preserved AV_NOPTS_VALUE could simplify quite a bit of code over our codebase.

and i dont think next_pts could have become AV_NOPTS_VALUE before, so a check
that could make it that now looks suspicious, just look:


>                      if (ist->st->codec->time_base.num != 0) {
>                          int ticks= ist->st->parser ? ist->st->parser->repeat_pict+1 : ist->st->codec->ticks_per_frame;
>                          ist->next_pts += ((int64_t)AV_TIME_BASE *

here things go wrong if its AV_NOPTS_VALUE


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

DNS cache poisoning attacks, popular search engine, Google internet authority
dont be evil, please
-------------- 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/20110201/05d78457/attachment.pgp>



More information about the ffmpeg-devel mailing list