[MPlayer-dev-eng] enhancement for yuv4mpeg output driver

[D Richard Felker III]

On Wed, Jan 19, 2005 at 10:00:13PM -0800, Trent Piepho wrote:
> On Wed, 19 Jan 2005, D Richard Felker III wrote:
> > 
> > where did 23.976 come from? did you just type it in yourself? if so
> 
> Into the email message, yes.  Mplayer only prints the fps value to three
> places when you play a file, so I only copied it down to three places.  I
> didn't think anyone would feel 10 decimal places were necessary for an example
> in an email message.....

But the whole precision is there in MPlayer's fps variable, and will
get passed to av_reduce if you use it right.

> > lots or broken avi files have stupid framerates that aren't exactly
> > 24000/1001 in their headers, due to broken software like mplayer. so
> > there it's correct for av_reduce to reduce to something different.
> 
> You might want to add every NTSC DVD, VCD, and SVCD in existence too, as
> av_reduce always gives the incorrect result.  If you have an avi file with
> 30000/1001 fps correctly in its header, and av_reduce() gives 30000/1001 from
> mplayer's float fps value, then please speak up, because I've never managed to
> find a single one.

Works fine in mencoder, but it uses its own reduce function. Maybe
that's different from av_reduce?

> > > Is it acceptable for the vo_yuv4mpeg module to call a routine from libavcodec? 
> > > Is there a problem with the new dependency?  If it's ok, I'll make a new patch
> > 
> > hmm, probably not ok. could you just copy and paste the reduce code? i
> > imagine it's reallt small.
> 
> It's not necessary to reduce the fractions, as the mjpegtools API will
> automatically reduce them to lowest terms anyway when the stream is read.  If
> the code is already there in mplayer, might was well use it, but if it's not,
> I don't think it's worth the trouble to write some just for the yuv4mpeg
> output driver.

"Reduce" also means (in our case) find the fraction within a given
precision that most closely matches a stupid float we're given to
begin with..

> > > that uses it to reduce fraction, but it's still necessary to make a special
> > > case to detect NTSC x/1001 framerates.
> > 
> > no, it's not, and in fact this should not be done. if the error in the
> > source file is large, it _will_ cause a/v desync to alter the output
> > framerate like this. if the error is very small just use -fps
> > 24000/1001 to 'correct' it. but always check the a/v sync afterwards!!
> 
> I'm not talking about altering the output framerate, I'm trying to get
> yuv4mpeg output with a correct header.  It's not going to change a/v sync one
> bit, because everyone who uses yuv4mpeg output currently IGNORES the framerate
> in the header, which is wrong, and replaces it with the correct value at some
> stage.  I wrote this patch so mplayer would get it right to begin with.
> 
> Secondly, even if I specify -fps 30000/1001, av_reduce() reduces it to
> 10999/367!  My float->fraction code gets it right.

But only for special cases... BTW if this is true and av_reduce is
that broken, it needs to be fixed.

Rich