[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