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

[Roman Arutyunyan]

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 :)


>
> [...]
>
> thanks
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Its not that you shouldnt use gotos but rather that you should write
> readable code and code with gotos often but not always is less readable
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>