[FFmpeg-devel] [PATCH 2/4] Change the PTS time base to AV_TIME_BASE before to process a frame in the filter chain, and change it back to the stream time base in the filtered frame.

[Stefano Sabatini]

On date Monday 2010-07-19 20:32:23 +0200, Michael Niedermayer encoded:
> On Mon, Jul 19, 2010 at 06:26:20PM +0200, Stefano Sabatini wrote:
> > On date Tuesday 2010-06-15 03:05:41 +0200, Michael Niedermayer encoded:
> > > On Sat, Jun 12, 2010 at 12:45:29AM +0200, Stefano Sabatini wrote:
> > > > On date Wednesday 2010-06-09 00:17:27 +0200, Stefano Sabatini encoded:
> > > > > This is required, since all the frames in the filterchain are supposed
> > > > > to use a time base of AV_TIME_BASE.
> > > > > ---
> > > > >  ffplay.c |    3 ++-
> > > > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > > > 
> > > > > diff --git a/ffplay.c b/ffplay.c
> > > > > index 129cd28..dd3cba0 100644
> > > > > --- a/ffplay.c
> > > > > +++ b/ffplay.c
> > > > > @@ -1678,7 +1678,7 @@ static int input_request_frame(AVFilterLink *link)
> > > > >      }
> > > > >      av_free_packet(&pkt);
> > > > >  
> > > > > -    picref->pts = pts;
> > > > > +    picref->pts = av_rescale_q(pkt.pts, priv->is->video_st->time_base, AV_TIME_BASE_Q);
> > > > >      picref->pos = pkt.pos;
> > > > >      picref->pixel_aspect = priv->is->video_st->codec->sample_aspect_ratio;
> > > > >      avfilter_start_frame(link, picref);
> > > > > @@ -1838,6 +1838,7 @@ static int video_thread(void *arg)
> > > > >              SDL_Delay(10);
> > > > >  #if CONFIG_AVFILTER
> > > > >          ret = get_filtered_video_frame(filt_out, frame, &pts_int, &pos);
> > > > > +        pts_int = av_rescale_q(pts_int, AV_TIME_BASE_Q, is->video_st->time_base);
> > > > >  #else
> > > > >          ret = get_video_frame(is, frame, &pts_int, &pkt);
> > > > >  #endif
> > > > 
> > > > Ping? (That's required by the setpts patch).
> > > 
> > > we need the timebase for muxing, a lazy filter could always
> > > set that to AV_TIME_BASE_Q if it likes but we should support keeping track
> > > of it.
> > > If you disagree then which timebase should the muxer store?
> > > some containers dont like it if the timebase is 1000000 times smaller than
> > > 1/average fps
> > 
> > Have a look at the attached patch, the ffplay patch is merely intended
> > to show how this stuff is supposed to be used.
> > 
> > I have also a settb filter I'm using here for testing.
> > 
> > Regards.
> > -- 
> > FFmpeg = Furious & Fierce Meaningless Pitiless Exxagerate Guru
> 
> >  avfilter.c |   14 ++++++++++++++
> >  avfilter.h |   15 ++++++++++++++-
> >  2 files changed, 28 insertions(+), 1 deletion(-)
> > 578baaf67db407587df6072646d398c3eb051912  0002-Add-a-time_base-field-to-AVFilterPad.patch
> > >From 54a936a6cb3af89f6c4042e77750a69b56f68d39 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > Date: Mon, 19 Jul 2010 12:55:27 +0200
> > Subject: [PATCH 2/5] Add a time_base field to AVFilterPad.
> > 
> > This is required for allowing a filter to use a time base different
> > from AV_TIME_BASE_Q, as it was previously assumed.
> > ---
> >  libavfilter/avfilter.c |   14 ++++++++++++++
> >  libavfilter/avfilter.h |   15 ++++++++++++++-
> >  2 files changed, 28 insertions(+), 1 deletions(-)
> > 
> > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> > index e81fa48..5b4e4e7 100644
> > --- a/libavfilter/avfilter.c
> > +++ b/libavfilter/avfilter.c
> > @@ -149,11 +149,15 @@ int avfilter_config_links(AVFilterContext *filter)
> >              if(avfilter_config_links(link->src))
> >                  return -1;
> >  
> > +            if (link_spad(link).time_base.num == 0 && link_spad(link).time_base.den == 0)
> > +                link_spad(link).time_base = AV_TIME_BASE_Q;
> >              if(!(config_link = link_spad(link).config_props))
> >                  config_link  = avfilter_default_config_output_link;
> >              if(config_link(link))
> >                  return -1;
> >  
> > +            if (link_dpad(link).time_base.num == 0 && link_dpad(link).time_base.den == 0)
> > +                link_dpad(link).time_base = link_spad(link).time_base;
> >              if((config_link = link_dpad(link).config_props))
> >                  if(config_link(link))
> >                      return -1;
> > @@ -240,6 +244,7 @@ void avfilter_start_frame(AVFilterLink *link, AVFilterPicRef *picref)
> >  {
> >      void (*start_frame)(AVFilterLink *, AVFilterPicRef *);
> >      AVFilterPad *dst = &link_dpad(link);
> > +    AVFilterPad *src = &link_spad(link);
> >  
> >      FF_DPRINTF_START(NULL, start_frame); ff_dprintf_link(NULL, link, 0); dprintf(NULL, " "); ff_dprintf_picref(NULL, picref, 1);
> >  
> > @@ -263,6 +268,15 @@ void avfilter_start_frame(AVFilterLink *link, AVFilterPicRef *picref)
> >      else
> >          link->cur_pic = picref;
> >  
> > +    if (dst->time_base.num != src->time_base.num || dst->time_base.den != src->time_base.den) {
> 
> av_cmp_q()
> 
> patch ok except that

I did more tests and I noticed this problem.

In the case of a filterchain of the kind:

buffer -> filter -> ffmpeg_output

After configuration this results like this:

buffer [TB1] <-> [TB1] filter [AVTB] <-> [TB1] ffmpeg_output

while I want to avoid conversions, that is I want this:

buffer [TB1] <-> [TB1] filter [TB1] <-> [TB1] ffmpeg_output

This is done configuring by default the output link to make it use the
same timebase of the *first* input pad if it exists.

Patch updated.
-- 
FFmpeg = Fostering and Frenzy Magnificient Pitiless Extreme Gargoyle