[FFmpeg-devel] [PATCH] avcodec: try to document timebase and framerate a bit more

[Michael Niedermayer]

On Sat, Mar 05, 2016 at 07:55:47PM +0100, Reimar Döffinger wrote:
> On Sat, Mar 05, 2016 at 06:56:30PM +0100, Michael Niedermayer wrote:
> > On Sat, Mar 05, 2016 at 06:01:11PM +0100, Reimar Döffinger wrote:
> > > On Sat, Mar 05, 2016 at 01:41:32PM +0100, Michael Niedermayer wrote:
> > > > +     * Note, for variable framerate material this has undefined meaning currently
> > > > +     * and is not set to the actual framerate nor {0,1} but can be set to
> > > > +     * 1/timebase (FIXME) the code in some parts assumes framerate == 1/timebase
> > > > +     * which is generally not true but these parts need to be fixed before framerate
> > > > +     * can be fixed.
> > > 
> > > This is too complex, maybe it can be split in multiple sentences.
> > > Suggestion (but I took a lot of liberty and it might not say what you
> > > meant!):
> > > 
> > > Note: meaning is not yet defined for variable frame rate content.
> > > Ideally it should be set to either the current frame rate or {0,1}
> > > for that case.
> > > However for this some code assuming framerate == 1/timebase needs to
> > > be fixed first - until then this field may incorrectly be set
> > > to 1/timebase.
> > 
> > there are 2 cases
> > 1. encoding where the users sets it
> > 2. deoding where lavc sets it
> > 
> > my comment was about decoding "issues"/"bugs"
> 
> I understood that, but I admit my description did not capture that,
> so maybe clearer with adding "when decoding" and "libavcodec sets"
> 
> > > Note: when decoding meaning is not yet defined for variable frame
> > > rate content.
> > > Ideally libavcodec should set it to either the current frame rate or {0,1}
> > > for that case.
> > > However for this some code assuming framerate == 1/timebase needs to
> > > be fixed first - until then libavcodec may incorrectly set this field
> > > to 1/timebase.
> 
> But as said it's just a suggestion. Even your original one is better
> than nothing. I just thought it quite tough to read.

i realized that, the framerate dox is really more or less a bug
description, it doesnt exactly fit in the public header where the patch
would put it.
It was more intended as a note for ffmpeg developers and less for
application developers, iam not sure where to put it or how to word
it exactly
not sure i should apply it (patch 2 of 2) or not ? or how to word it

the other patch (about timebase 1 of 2) ive applied

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160307/927efffb/attachment.sig>