[FFmpeg-devel] [PATCH] Add a time_base field to AVFilterPad.

[Michael Niedermayer]

On Thu, Sep 30, 2010 at 10:35:41PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2010-09-28 20:29:48 +0200, Michael Niedermayer encoded:
> > On Tue, Sep 28, 2010 at 02:34:05AM +0200, Stefano Sabatini wrote:
> > > On date Monday 2010-09-27 23:28:47 +0200, Stefano Sabatini encoded:
> > > > On date Thursday 2010-07-22 00:47:32 +0200, Stefano Sabatini encoded:
> > > > > On date Wednesday 2010-07-21 21:12:04 +0200, Michael Niedermayer encoded:
> > > > > > On Wed, Jul 21, 2010 at 05:56:27PM +0200, Stefano Sabatini wrote:
> > > > > [...]
> > > > > > > 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.
> > > > > > 
> > > > > > by default the output tb should be choose so that all input timestamps can be
> > > > > > exactly represented or if this timebase exceeds 32/32bit then AVTB
> > > > > > 
> > > > > > and (inexact) convertion should of course be avoided when possible as its
> > > > > > a recipe for turning timestamps into non strictly increasing sequences
> > > > > > and that will cause problems at the muxer
> > > > > 
> > > > > Note that this isn't possible when configuring each single link, as
> > > > > that requires a global knowledge of the whole filtergraph.
> > > > > 
> > > > > We may still do that as a post configuration operation, for example we
> > > > > could have a sort of avfilter_graph_reconfigure_timebases().
> > > > > 
> > > > > Note that even in this scenario we may have some filter for which the
> > > > > timebase is assumed to be not changeable (e.g. a source which computes
> > > > > the time base depending on the frame rate set).
> > > > > 
> > > > > We may auto-insert some settb filters to manage that, and we would
> > > > > need to define a policy to determine when the timebase can be changed
> > > > > and when not (and we still lack a re-configuration mechanism).
> > > > > 
> > > > > Note that for the use in ffmpeg.c/ffplay.c this practically is not an
> > > > > issue, as most filters (with the only settb exception) don't change
> > > > > the time base, so with this patch we end up with all the filters in
> > > > > the linear sequence from the input source to the output sink using the
> > > > > same time base, with no information loss, so I suggest to apply this
> > > > > patch.
> > > > 
> > > > Patch updated. I'll try to find an acceptable solution in the next
> > > > days...
> > > 
> > > So we have this scenario:
> > > 
> > > buffer [TB1] <-> [X] filter [Y] <-> [Z] ffmpeg_output
> > > 
> > > Z can be set using the TB provided in the output video stream, so we
> > > have:
> > > 
> > > buffer [TB1] <-> [X] filter [Y] <-> [TB2] ffmpeg_output
> > 
> > X=TB1
> > Y=X unless filter wants something else
> > Z=Y
> > ffmpeg has to configure itself to use Z or rescale, there never is a timebase
> > moving backward in the graph
> > 
> > or in other words i fail to see where the problem you seem to have comes from
> > but i might be missing something of course
> 
> Updated patch implements this logic.
> Example:
> 
> $ echo "color,null,null,settb=23:35,null,settb=365:234,null,null,nullsink" | tools/graph2dot
> digraph G {
> node [shape=box]
> rankdir=LR
> "Filter 0 color (color)" -> "Filter 1 null (null)" [ label= "[1/1000000] fmt:argb w:320 h:240 [1/1000000]"];
> "Filter 1 null (null)" -> "Filter 2 null (null)" [ label= "[1/1000000] fmt:argb w:320 h:240 [1/1000000]"];
> "Filter 2 null (null)" -> "Filter 3 settb (settb)" [ label= "[1/1000000] fmt:argb w:320 h:240 [23/35]"];
> "Filter 3 settb (settb)" -> "Filter 4 null (null)" [ label= "[23/35] fmt:argb w:320 h:240 [23/35]"];
> "Filter 4 null (null)" -> "Filter 5 settb (settb)" [ label= "[23/35] fmt:argb w:320 h:240 [365/234]"];
> "Filter 5 settb (settb)" -> "Filter 6 null (null)" [ label= "[365/234] fmt:argb w:320 h:240 [365/234]"];
> "Filter 6 null (null)" -> "Filter 7 null (null)" [ label= "[365/234] fmt:argb w:320 h:240 [365/234]"];
> "Filter 7 null (null)" -> "Filter 8 nullsink (nullsink)" [ label= "[365/234] fmt:argb w:320 h:240 [365/234]"];
> }
> 
> It is not yet clear to me how to interpret this sentence from you:
> 
> |by default the output tb should be choose so that all input timestamps can be
> |exactly represented or if this timebase exceeds 32/32bit then AVTB

a filter  that has 2 inputs has 2 input timebases ...
ill review your patch once you confirm that its supposed to work in such cases

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20101001/0f7cdc20/attachment.pgp>