[09/12] avfilter/vf_frei0r: also set AVFilterLink.frame_rate

Message ID 1445978475-17688-1-git-send-email-stebbins@jetheaddev.com
State New
Headers show

Commit Message

John Stebbins Oct. 27, 2015, 8:41 p.m.
From: Michael Niedermayer <michaelni@gmx.at>

Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
(cherry picked from commit 353cf95f948ef7c6139c8ead79e9eeb9eb8d2e6e)
---
 libavfilter/vf_frei0r.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Luca Barbato Oct. 27, 2015, 9:12 p.m. | #1
On 27/10/15 21:41, John Stebbins wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
> (cherry picked from commit 353cf95f948ef7c6139c8ead79e9eeb9eb8d2e6e)
> ---
>  libavfilter/vf_frei0r.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> index 0122b8d..04f74bc 100644
> --- a/libavfilter/vf_frei0r.c
> +++ b/libavfilter/vf_frei0r.c
> @@ -459,6 +459,7 @@ static int source_config_props(AVFilterLink *outlink)
>      outlink->w = s->w;
>      outlink->h = s->h;
>      outlink->time_base = s->time_base;
> +    outlink->frame_rate = av_inv_q(s->time_base);
>  
>      if (s->destruct && s->instance)
>          s->destruct(s->instance);
> 

shouldn't it forward it from the inlink ?

(timebase can be 1/1000, a 1000fps kills some encoders)

lu
John Stebbins Oct. 27, 2015, 9:20 p.m. | #2
On Tue, 2015-10-27 at 22:12 +0100, Luca Barbato wrote:
> On 27/10/15 21:41, John Stebbins wrote:
> > From: Michael Niedermayer <michaelni@gmx.at>
> > 
> > Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
> > (cherry picked from commit 353cf95f948ef7c6139c8ead79e9eeb9eb8d2e6e)
> > ---
> >  libavfilter/vf_frei0r.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> > index 0122b8d..04f74bc 100644
> > --- a/libavfilter/vf_frei0r.c
> > +++ b/libavfilter/vf_frei0r.c
> > @@ -459,6 +459,7 @@ static int source_config_props(AVFilterLink
> > *outlink)
> >      outlink->w = s->w;
> >      outlink->h = s->h;
> >      outlink->time_base = s->time_base;
> > +    outlink->frame_rate = av_inv_q(s->time_base);
> >  
> >      if (s->destruct && s->instance)
> >          s->destruct(s->instance);
> > 
> 
> shouldn't it forward it from the inlink ?
> 
> (timebase can be 1/1000, a 1000fps kills some encoders)
> 
> lu
> 

I don't entirely understand what frei0r does. But it appears to generate
frames at the specified framerate (25 default). time_base is set to
1/framerate in source_init().  So the output framerate should not come
from the inlink.
Luca Barbato Oct. 27, 2015, 9:53 p.m. | #3
On 27/10/15 22:20, John Stebbins wrote:
> On Tue, 2015-10-27 at 22:12 +0100, Luca Barbato wrote:
>> On 27/10/15 21:41, John Stebbins wrote:
>>> From: Michael Niedermayer <michaelni@gmx.at>
>>>
>>> Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
>>> (cherry picked from commit 353cf95f948ef7c6139c8ead79e9eeb9eb8d2e6e)
>>> ---
>>>  libavfilter/vf_frei0r.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
>>> index 0122b8d..04f74bc 100644
>>> --- a/libavfilter/vf_frei0r.c
>>> +++ b/libavfilter/vf_frei0r.c
>>> @@ -459,6 +459,7 @@ static int source_config_props(AVFilterLink
>>> *outlink)
>>>      outlink->w = s->w;
>>>      outlink->h = s->h;
>>>      outlink->time_base = s->time_base;
>>> +    outlink->frame_rate = av_inv_q(s->time_base);
>>>  
>>>      if (s->destruct && s->instance)
>>>          s->destruct(s->instance);
>>>
>>
>> shouldn't it forward it from the inlink ?
>>
>> (timebase can be 1/1000, a 1000fps kills some encoders)
>>
>> lu
>>
> 
> I don't entirely understand what frei0r does. But it appears to generate
> frames at the specified framerate (25 default). time_base is set to
> 1/framerate in source_init().  So the output framerate should not come
> from the inlink.

Grrr, another upside down then.

It should take the input framerate if exists then.

lu
John Stebbins Oct. 27, 2015, 10:17 p.m. | #4
On Tue, 2015-10-27 at 22:53 +0100, Luca Barbato wrote:
> On 27/10/15 22:20, John Stebbins wrote:
> > On Tue, 2015-10-27 at 22:12 +0100, Luca Barbato wrote:
> > > On 27/10/15 21:41, John Stebbins wrote:
> > > > From: Michael Niedermayer <michaelni@gmx.at>
> > > > 
> > > > Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
> > > > (cherry picked from commit
> > > > 353cf95f948ef7c6139c8ead79e9eeb9eb8d2e6e)
> > > > ---
> > > >  libavfilter/vf_frei0r.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> > > > index 0122b8d..04f74bc 100644
> > > > --- a/libavfilter/vf_frei0r.c
> > > > +++ b/libavfilter/vf_frei0r.c
> > > > @@ -459,6 +459,7 @@ static int source_config_props(AVFilterLink
> > > > *outlink)
> > > >      outlink->w = s->w;
> > > >      outlink->h = s->h;
> > > >      outlink->time_base = s->time_base;
> > > > +    outlink->frame_rate = av_inv_q(s->time_base);
> > > >  
> > > >      if (s->destruct && s->instance)
> > > >          s->destruct(s->instance);
> > > > 
> > > 
> > > shouldn't it forward it from the inlink ?
> > > 
> > > (timebase can be 1/1000, a 1000fps kills some encoders)
> > > 
> > > lu
> > > 
> > 
> > I don't entirely understand what frei0r does. But it appears to
> > generate
> > frames at the specified framerate (25 default). time_base is set to
> > 1/framerate in source_init().  So the output framerate should not
> > come
> > from the inlink.
> 
> Grrr, another upside down then.
> 
> It should take the input framerate if exists then.
> 
> 

I'm a bit confused about what this filter does, and what it meant to do.
From what I can find online about fiei0r, it applies special effects to
a video stream.  Which implies to me that it should not be changing the
frame_rate or time_base of the incoming stream.  But the code (before
the frame_rate patch) is modifying the time_base, effectively forcing
constant frame_rate output at 1/time_base.  Why does this filter force
the time_base like this?

Also, in one place (filter_frame) it translates incoming frame pts with
inlink->time_base and in another (source_request_frame) it translates
frame pts with s->time_base.  It seems like this filter can only "work"
if the framerate option is set to 1/inlink->time_base.
John Stebbins Oct. 27, 2015, 10:25 p.m. | #5
On Tue, 2015-10-27 at 15:17 -0700, John Stebbins wrote:
> On Tue, 2015-10-27 at 22:53 +0100, Luca Barbato wrote:
> > On 27/10/15 22:20, John Stebbins wrote:
> > > On Tue, 2015-10-27 at 22:12 +0100, Luca Barbato wrote:
> > > > On 27/10/15 21:41, John Stebbins wrote:
> > > > > From: Michael Niedermayer <michaelni@gmx.at>
> > > > > 
> > > > > Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
> > > > > (cherry picked from commit
> > > > > 353cf95f948ef7c6139c8ead79e9eeb9eb8d2e6e)
> > > > > ---
> > > > >  libavfilter/vf_frei0r.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> > > > > index 0122b8d..04f74bc 100644
> > > > > --- a/libavfilter/vf_frei0r.c
> > > > > +++ b/libavfilter/vf_frei0r.c
> > > > > @@ -459,6 +459,7 @@ static int
> > > > > source_config_props(AVFilterLink
> > > > > *outlink)
> > > > >      outlink->w = s->w;
> > > > >      outlink->h = s->h;
> > > > >      outlink->time_base = s->time_base;
> > > > > +    outlink->frame_rate = av_inv_q(s->time_base);
> > > > >  
> > > > >      if (s->destruct && s->instance)
> > > > >          s->destruct(s->instance);
> > > > > 
> > > > 
> > > > shouldn't it forward it from the inlink ?
> > > > 
> > > > (timebase can be 1/1000, a 1000fps kills some encoders)
> > > > 
> > > > lu
> > > > 
> > > 
> > > I don't entirely understand what frei0r does. But it appears to
> > > generate
> > > frames at the specified framerate (25 default). time_base is set
> > > to
> > > 1/framerate in source_init().  So the output framerate should not
> > > come
> > > from the inlink.
> > 
> > Grrr, another upside down then.
> > 
> > It should take the input framerate if exists then.
> > 
> > 
> 
> I'm a bit confused about what this filter does, and what it meant to
> do.
> From what I can find online about fiei0r, it applies special effects
> to
> a video stream.  Which implies to me that it should not be changing
> the
> frame_rate or time_base of the incoming stream.  But the code (before
> the frame_rate patch) is modifying the time_base, effectively forcing
> constant frame_rate output at 1/time_base.  Why does this filter force
> the time_base like this?
> 
> Also, in one place (filter_frame) it translates incoming frame pts
> with
> inlink->time_base and in another (source_request_frame) it translates
> frame pts with s->time_base.  It seems like this filter can only
> "work"
> if the framerate option is set to 1/inlink->time_base.
> 
> 

Answering my own questions.  This file implements 2 things.  A filter
and a video source.  The "framerate" option is only applied for the
video source.  When used as a filter, time_base and frame_rate are
untouched.

Regarding upside down.  I turn it right side up if you would like.

Patch

diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
index 0122b8d..04f74bc 100644
--- a/libavfilter/vf_frei0r.c
+++ b/libavfilter/vf_frei0r.c
@@ -459,6 +459,7 @@  static int source_config_props(AVFilterLink *outlink)
     outlink->w = s->w;
     outlink->h = s->h;
     outlink->time_base = s->time_base;
+    outlink->frame_rate = av_inv_q(s->time_base);
 
     if (s->destruct && s->instance)
         s->destruct(s->instance);