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

[Michael Niedermayer]

On Sun, Feb 06, 2011 at 08:44:55PM +0100, Nicolas George wrote:
> Le tridi 13 pluvi?se, an CCXIX, Michael Niedermayer a ?crit?:
> > the "pkt_pts" = AV_NOPTS_VALUE; should stay, aka the pts should be reset after
> > use.
> 
> The version you reviewed killed the pkt_pts variable altogether. Not
> anymore.
> 
> But this pkt_pts variable seems strange: if I understand the code correctly,
> if a video packet has no PTS, pkt_pts will stay from a previous audio
> packet. This seems wrong, as audio frames have not the same duration as
> video size, thus the PTS will not be a multiple of the framerate.
> Furthermore, some streams have the audio with a lot of advance with regard
> to the video.
> 
> Well, this is now a distinct issue from current version of the patch.
> 
> > double ;;
> 
> Fixed.
> 
> > also this has scaling issues, as ist->*pts are effectively scaled back and
> > forth between 2 timebases all the time
> 
> I did not like it either, but did not found a better solution. This is
> longer an issue.
> 
> > 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.
> 
> No longer an issue either, but I had the same thought when writing that
> line.
> 
> > 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:
> 
> Indeed.
> 
> > Ill look into this and apply it to videolan if its ok
> 
> I would hate for it to be the start of a API difference between
> git.videolan.org and git.ffmpeg.org, though.
> 
> Anyway, here is a new version, rebased on 1fe2d4d "Adopt pkt_dts/pkt_pts in
> lavc clients" that I cherry-picked, as it looks to me to do the right thing.
> 
> As you can see, 1fe2d4d avoids the rescaling issues by injecting the
> rescaled timestamps in the packet before decoding. The patch has now only to
> move the guess_correct_pts into libavcodec.
> 
> (I still dislike the fact that ffmpeg rescales timestamps to 1/1000000
> between ist->time_base and ost->time_base, but that is completely
> unrelated.)
> 
> Regards,
> 
> -- 
>   Nicolas George

>  cmdutils.c           |   27 ---------------------------
>  cmdutils.h           |   24 ------------------------
>  ffmpeg.c             |    4 +---
>  ffplay.c             |    7 ++-----
>  libavcodec/avcodec.h |   18 ++++++++++++++++++
>  libavcodec/utils.c   |   43 ++++++++++++++++++++++++++++++++++++++++++-
>  6 files changed, 63 insertions(+), 60 deletions(-)
> 2b7ff42ffe638b47e5dbfceb767788985f0818c4  0001-Implement-guessed_pts-in-avcodec_decode_video2.patch
> From 12ffa4d8754d8f36fa9a6c5ed22c4289c69525e1 Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Sun, 30 Jan 2011 20:18:31 +0100
> Subject: [PATCH] Implement guessed_pts in avcodec_decode_video2.

this segfaults during fate for me

ive tried make clean but that didnt help

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- 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/20110206/7c04c658/attachment.pgp>