avs devs: plz integrate nicefps() into changefps()/assumefps()

[bond]

changefps/assumefps() allow to input float fps and people make extensively use of this
now the problem is that it doesnt seem to output for eg 23.976 the standard 24000/1001 numerator,denominator but some other high values

this now leads to that for example the x264 encoder is forced to use 64bit precision times, which again causes players like mplayer or quicktime not handling these 64bit, but expecting 32bit, to bork

now we had lots of people reporting problems in the mpeg-4 avc forum reporting these problems and they could be tracked down to the useage of change/assumefps() with float fps input or with input .avi files that have been created from such scripts

now there are three solutions for fixing this:
1) make the whole world use change/assumefps(24000/1001) instead of changefps(23.976) or similar for other framerates
2) make the whole world use the nicefps() filter, as discussed here, which scales the framerate in a "nice" way
3) change the behaviour of change/assumefps() to not output these big values anymore by maybe integrating the nicefps()'s function into change/assumefps() or making change/assumefps process the string itself instead of using float math

now i would be really happy if 3) could be done (as 1) and 2) is surely impossible), cause it would remove like "50%" of the x264 error reports

[stickboy]

If only AviSynth used doubles instead of floats, then it more easily could rationalize 23.976 to 2997/125.

Overloading AssumeFPS to take a string might be okay (please, don't add any more *FPS functions), but then you still have the problem of educating people to use it.

[Bidoche]

Originally Posted by stickboy

If only AviSynth used doubles instead of floats, then it more easily could rationalize 23.976 to 2997/125.

I don't see how that would help...
Users generally mean 24000/1001 when they put 23.976 and not 2997/125.

Besides FPS is rational, not float or even double, every api with some sense use it like that.
VFW certainly does, avs should do the same (3.0 does)

but then you still have the problem of educating people to use it

Easy : add overloaded *FPS functions and change all the old ones to throw a message reporting them deprecated and asking to use the new ones.

[tritical]

Originally Posted by stickboy

If only AviSynth used doubles instead of floats, then it more easily could rationalize 23.976 to 2997/125.

I think the main thing is that because it uses floats one has to realize that the precision of the input value is limited to roughly 7 decimal digits. However, that is still enough for values like 23.976 or 29.970. For example, if the current code in assumefps, which is:

double n = args[1].AsFloat();
int d = 1;
while (n < 16777216 && d < 16777216) { n*=2; d*=2; }
return new AssumeFPS(args[0].AsClip(), int(n+0.5), d, args[2].AsBool(false), env);

were changed to:

double n = args[1].AsFloat();
int d = 1;
while (n < 1000000.0 && d < 1000000) { n*=10.0; d*=10; }
int x = int(n+0.5), y = d, t;
while (y) { t = x%y; x = y; y = t; }
return new AssumeFPS(args[0].AsClip(), int(n+0.5)/x, d/x, args[2].AsBool(false), env);

then Assumefps(23.976) would result in 2997/125, Assumefps(29.970) would result in 2997/100, Assumefps(119.88) would result in 2997/25, etc... As opposed to what you currently get which is 12570329/524288, 15712911/524288, and 982057/8192. The problem with 30000/1001 or 24000/1001 is that those are repeating decimals and cannot be accurately expressed using a limited precision decimal number. Thus, I don't see any way avisynth could deduce 24000/1001 from a decimal input. To me it seems that any rational fps that requires more than 7 decimal digits of precision (or 15 if it was changed to double) would need to be set using assumefps(numerator,denominator).

[sh0dan]

Can anyone predict problems with the patch above, otherwise it'll go into 2.6.

[stickboy]

Originally Posted by Bidoche

I don't see how that would help...
Users generally mean 24000/1001 when they put 23.976 and not 2997/125.

Well, a conventional algorithm of repeatedly adding the numerators and denominators of low/high bounds rationalizes 23.976 fine for doubles but not for floats.

And rationalizing to 2997/125 is still an improvement over 12570329/524288 that AviSynth currently generates, yes? Would 2997/125 not address bond's "high-values lead to unnecessary 64-bit computation" issue?

Easy : add overloaded *FPS functions and change all the old ones to throw a message reporting them deprecated and asking to use the new ones.

Sure, if you want to break backwards compatibility.

[foxyshadis]

The drift between 2997/125 and 24000/1001 is only one-hundredth of a second after three hours. If you never told anyone they'd probably never find out.

[Fizick]

IMO, assumefps(numerator, denominator) is most correct way.
So, if somebody use it explicitly, Avisynth must not change these values.

[Mug Funky]

another possibility is to simply pick the /1001 framerate that fits the input the best.

AFAIK the only rational framerates are those of (colour) NTSC. when someone types 29.97 it is plainly obvious that they mean 30000/1001, so really this number can be assumed without any drawbacks (as stickboy says, who's going to notice 1/100th of a second after 3 hours? considering that 1-frame desync is quite common on DVDs anyway, particularly if they've been standards converted, and nobody seems to notice that).

presets would be interesting too - something like "assumefps("film")" to give 24000/1001, or "assumefps("NTSC") gives 30000/1001, etc. this would certainly be easier to type (and understand...) than 30000/1001 etc.

[Bidoche]

Originally Posted by stickboy

Sure, if you want to break backwards compatibility.

I was answering on how educating users.
That method certainly works

presets would be interesting too - something like "assumefps("film")" to give 24000/1001, or "assumefps("NTSC") gives 30000/1001, etc. this would certainly be easier to type (and understand...) than 30000/1001 etc.

Looks interesting

[d'Oursse]

afaik, 24000/1001 is not the fps of the "films". it's the fps used to get the NTSC framerate from a "photographic material produced for the cinema". the framerate of the films is 24 fps (http://www.doom9.org/ivtc-tut.htm). Maybe the word "film" is not correct.

[tritical]

another possibility is to simply pick the /1001 framerate that fits the input the best.

AFAIK the only rational framerates are those of (colour) NTSC. when someone types 29.97 it is plainly obvious that they mean 30000/1001, so really this number can be assumed without any drawbacks (as stickboy says, who's going to notice 1/100th of a second after 3 hours? considering that 1-frame desync is quite common on DVDs anyway, particularly if they've been standards converted, and nobody seems to notice that).

While this would work for getting the 24000/1001 and 30000/1001 ratios, I personally don't think this is a good idea. When someone uses assumefps(23.976) they should, IMHO, get what they asked for. Adding work arounds to have it adjust to another value is just going to add to the confusion when someone really does want 23.976 and not 24000/1001 for whatever reason.

presets would be interesting too - something like "assumefps("film")" to give 24000/1001, or "assumefps("NTSC") gives 30000/1001, etc. this would certainly be easier to type (and understand...) than 30000/1001 etc.

Sounds like a good idea.

[bond]

so will my proposal be followed?

[tritical]

If anyone is interested in testing, I have merged the code I posted before and Mug Funky's preset idea (though I used the names "ntsc_film", "ntsc_video", "ntsc_double" for 24000/1001, 30000/1001, 60000/1001 respectively) into the changefps/convertfps/assumefps filters in the custom version of avisynth I keep on my website.

[IanB]

Avery's latest blog, Wikipedia and AVI fractions refers to this article on continued fractions. This might give the narrow minded something to broaden their horizons.

As for guessing what the true FPS the user really intended from a single precision float approximations of FPS, I think I would have to class that with vodoo and other paranomal superstition. Doing so is very fraught. The current code is able to exactly return to the same single precision floating point value originally set, none of the above suggested code mods provide this.

If we change the definition of 29.97 and 23.976 FPS then there will be some lazy users who may be transiently happy but in the long run smart alec hacks, like the proposed, always return to bite you big time. And what do we do with the well intensioned 29.97002997 or 23.976023976 values (which don't fit is a float anyway).

For a case in point consider :- A current user builds a script and set the fps to 23.976 (not 24000/1001), they add an audio track edited in an external editor that has explicit knowledge of single precision 23.976 fps framerate. It works as expected. 9 months later we have a new AVS version, with hacked code, the user is not happy his previous working script inexplicably is now getting audio desync....


The idea for easy to spec string equivalences has merit. Of course it can be implemented immediatly as a simple .avsi script, no need to polute the core.

Bottom line is if you want 30000/1001 fps or 24000/1001 fps then that is what you should spec. So maybe we need to provide text presets to make this easy to understand for the innumerate masses.


P.S. For the uninformed this code

while (n < 16777216 && d < 16777216) { n*=2; d*=2; }

is crudely extracting the mantisa and exponent from the float and storinging them loselessly (well for values less than 2^31) as a rational fraction.

[stickboy]

BTW, would it be useful to modify Info() so that instead of:

Frames per second: 23.9760

it showed:

Frames per second: 23.9760 (24000/1001)

?

[sh0dan]

@stickboy: Info fractions added to CVS.

[Wilbert]

Some discussion on #x264 with respect to this problem

<funmain> bond: high in your opinion is above 23.976 right?
<|bond|> (funmain) bond: high in your opinion is above 23.976 right? <- no
<funmain> |bond|: define 'other high values'
<|bond|> funmain: well it was more meant as a joke that you post there, but it would definitely dont hurt if you would post there
<funmain> 23.976 is based on really really old standards (they should stick to it), i will do bond
<Haali> hehe you seriously propose changing avisynth because of broken qt player?
<|bond|> funmain: 23.976 can be represented as 24000/1001, but thats not 100% correct. avs uses some very high values which make use of 64bit. quicktime doesnt handle that
<|bond|> (Haali) hehe you seriously propose changing avisynth because of broken qt player? <- and mplayer and ffmpeg
<pengvado> we propose to change avisynth because avisynth is _broken_
<pengvado> just because some players happen to play the resulting file doesn't make it correct
<|bond|> (@pengvado) we propose to change avisynth because avisynth is _broken_ <- post your opinion here, cause they dont seem to change it: http://forum.doom9.org/showthread.php?t=104681
<Haali> avisynth is definitely not broken
<|bond|> at least they could add the feature of nicefps()
<|bond|> it wouldnt hurt and help newbies
<Haali> it either passes through what it got from source file or what you input in the script
<Haali> if you do mean 24000/1001 then say so in the script
<|bond|> newbies have no clue about that
<pengvado> except that it doesn't
<pengvado> assumefps(23.976) doesn't set fps=23976/1000
<Haali> because you'd use assumefps(23976,1000) for that, no?
<Haali> anyway, this could be nicely improved by treating the argument as a string, not a double
<pengvado> it seems perfectly reasonable to me to expect assumefps(23.976) and assumefps(23976,1000) to do the same thing
<pengvado> hell, even a double would work. just single precision isn't enough.
<Haali> i agree it can be handled better
<Haali> but in this instance it's the players that can't read valid files, not broken avisynth
<Haali> and pretty obscure players
<Haali> and at least ffmpeg shouldn't be too hard to fix
<funmain> Haali: have u been reading the topic on doom9?
<Haali> yes
<|bond|> some tried to fix ffmpeg to handle 64bit but its still not working correctly and he doesnt answer anymore
<|bond|> noone on the mplayer maillist responded
<Haali> send them some sample they'd definitely want to watch
<|bond|> mplayer ftp isnt working
<Haali> pengvado: btw avisynth uses doubles for args already
<pengvado> I didn't look at the code. but the fps you get from assumefps(23.976) is _exactly_ the float representation on 23.976, not the double representation.
<Haali> yeah, because assumefps only extracts 24 bits from it
<Haali> should have done all 32 :P
<pengvado> ok, so they just need to do the double->rational using nicefps's algo instead of truncating
<Haali> i'd think avoiding a round trip through double can be a better idea
<funmain> pengvado: you dont know.. ?
<pengvado> I know how to fix it, but the patch has already been posted. we just need some avisynth dev to agree to apply it, and I'm not one.
<algern0n> pengvado: time for you to become an avisynth dev then
<Koepi> bond is right. you should make an announcement

[tritical]

Originally Posted by IanB

The current code is able to exactly return to the same single precision floating point value originally set, none of the above suggested code mods provide this.

This is definitely not true for all cases... why do you think avisynth currently returns a num/den of 982057/8192 for assumefps(119.88) when the actual binary single precision fp approximation of 119.88 is equal to 15712911/131072? The two reasons it doesn't work for all cases are:

1.) d is limited to < 2^24, which means various values in the range 0 < x < 0.5 (granted it's not a common fps value) will not be translated into a num/den combo that when converted back to float will equal the original. Also, in the current code n only needs to be limited to < 2^23 for the loop condition since single precision floating point only has 23 bits for the mantissa.

2.) It will not work for other various values (119.88 is one example), due to how avisynth's script parser converts to float. Take a look at the floating point part of the Tokenizer::GetNumber() routine, it should be doing all the intermediate stuff in double (otherwise too much error accumulates), but it currently does it in float. For example, if you do assumefps(119.88) avisynth spits out 982057/8192 for a num/den. The 982057/8192 comes from assumefps() calling vi.setfps(num,den) which finds the gcd and and divides it out, in this case it is 32. That means original numbers that get passed are 31425824/262144 while the actual binary fp representation of 119.88 is equal to 31425822/262144. The error comes from the script parser conversion, which, as written, doesn't convert 119.88 to 0x42efc28f (hex value of the 4 bytes at the float memory location), but 0x42efc290. If that routine is changed to use doubles for intermediate steps then the correct value (0x42efc28f) is produced.

Anyways, whether or not the code is changed doesn't really matter to me (that's why I keep a custom version ).

[bond]

another problem with parsers on such 64bit requiring framerates:
http://forum.doom9.org/showthread.php?t=105154

edit: is the behaviour the same than with assume/changefps when setting the fps via directshowsource?

can we plz change the handling of 29.97, 23.976 aso in avisynth :>