[FFmpeg-devel] [PATCH] Implemented arrange feature to convert from low timebase formats to high timebase

[Michael Niedermayer]

On Thu, Oct 18, 2012 at 09:47:21AM +0400, Roman Arutyunyan wrote:
> 2012/10/18 Roman Arutyunyan <arutyunyan.roman at gmail.com>
> 
> > 2012/10/18 Michael Niedermayer <michaelni at gmx.at>
> >
> >> On Wed, Oct 17, 2012 at 07:14:07PM +0400, Roman Arutyunyan wrote:
> >> > 2012/10/17 Michael Niedermayer <michaelni at gmx.at>
> >> >
> >> > > On Wed, Oct 17, 2012 at 11:34:58AM +0400, Roman Arutyunyan wrote:
> >> > > [...]
> >> > > >  AVRational av_codec_get_pkt_timebase         (const AVCodecContext
> >> > > *avctx);
> >> > > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> >> > > > index 05c4b7f..0b59eeb 100644
> >> > > > --- a/libavformat/utils.c
> >> > > > +++ b/libavformat/utils.c
> >> > > > @@ -3498,6 +3498,14 @@ static int
> >> compute_pkt_fields2(AVFormatContext
> >> > > *s, AVStream *st, AVPacket *pkt){
> >> > > >      if(pkt->pts == AV_NOPTS_VALUE && pkt->dts != AV_NOPTS_VALUE &&
> >> > > delay==0)
> >> > > >          pkt->pts= pkt->dts;
> >> > > >
> >> > > > +    if (st->codec->arrange_threshold && pkt->dts != AV_NOPTS_VALUE
> >> &&
> >> > > st->pts.val != AV_NOPTS_VALUE){
> >> > > > +        int64_t diff = pkt->dts - st->pts.val;
> >> > > > +        int64_t maxdiff = av_rescale(st->codec->arrange_threshold,
> >> > > st->time_base.den, AV_TIME_BASE * (int64_t)st->time_base.num);
> >> > > > +        av_dlog(s, "Arrange dts=%"PRId64" cur_dts=%"PRId64"
> >> > > diff=%"PRId64" maxdiff=%"PRId64"\n", pkt->dts, st->pts.val, diff,
> >> maxdiff);
> >> > > > +        if (diff > -maxdiff && diff < maxdiff)
> >> > > > +            pkt->dts = pkt->pts = AV_NOPTS_VALUE;
> >> > > > +    }
> >> > >
> >> > > this doesnt look correct. The correction should be done where the
> >> > > problem is currently introduced. As done here , ffmpeg.c would send
> >> > > some timestamps to the muxer but the muxer would use different
> >> > > timestamps. its ffmpeg*.c or a common utility function used by
> >> > > ffmpeg*.c where the correction should be done.
> >> > >
> >> > > Currently timestamps are rounded toward "nearest", what should
> >> > > instead be done, is to round toward the exact duration + last_time
> >> > > see av_get_audio_frame_duration() to find the exact frame duration.
> >> > >
> >> >
> >> > I'm absolutely agree with you. The reason why I've done that way is
> >> there's
> >> > nothing
> >> > that looks like last_time on earlier stages of frame processing
> >> (ffmpeg.c).
> >>
> >> if at the location where the issue is introduced there really is no
> >> last timestamp variable then its quite easy to add one.
> >>
> >>
> > I looked into write_frame in ffmpeg.c. It operates with ost->st->...
> > fields a lot.
> > So what do you think about moving the code from where it
> > is now to write_frame with accessing the same data (ost->st->pts.val) from
> > there.
> > because ost->st->pts.val is exactly what we need (last_time).
> >
> > Michael, is that a good solution to reset the timestamps (pkt->dts =
> > pkt->pts = AV_NOPTS_VALUE)
> > to make them receive auto-generated values? The code which assign the
> > auto-generated
> > values is marked with "FIXME this is a temporary hack until all encoders
> > output pts"
> >
> dated back to 2004. A long-living hack :)
> >
> 
> I mean this (not tested this version, just an example).
> btw I've named the feature "align_aframes"

implemented differently, please test.

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121027/982cc516/attachment.asc>