[Ffmpeg-devel] [BUG] wrong fps in r_frame_rate ?

[Michael Niedermayer]

Hi

On Sat, Dec 09, 2006 at 03:24:47AM +0100, Baptiste Coudurier wrote:
> Michael Niedermayer a ?crit :
> >Hi
> >
> >On Sat, Dec 09, 2006 at 12:51:27AM +0100, Baptiste Coudurier wrote:
> >>Hi
> >>
> >>Michael Niedermayer a ?crit :
> >>>Hi
> >>>
> >>>On Fri, Dec 08, 2006 at 04:08:14PM +0100, Baptiste Coudurier wrote:
> >>>>Hi
> >>>>
> >>>>$subject
> >>>>
> >>>>latest svn. ffmpeg -i wrong_fps.mpg
> >>>>
> >>>>Seems that stream 0 comes from film source: 25.00 (25/1) -> 24.75 (99/4)
> >>>>Input #0, mpeg, from 'wrong_fps.mpg':
> >>>> Duration: 00:00:04.3, start: 0.552822, bitrate: 15583 kb/s
> >>>> Stream #0.0[0x1e0]: Video: mpeg2video, yuv422p, 720x608, 15000 kb/s,
> >>>>24.75 fps(r)
> >>>> Stream #0.1[0x1c0]: Audio: mp2, 48000 Hz, stereo, 192 kb/s
> >>>> Stream #0.2[0x1c1]: Audio: mp2, 48000 Hz, stereo, 192 kb/s
> >>>>Must supply at least one output file
> >>>>
> >>>>wrong_fps.mpg on mplayerhq as usual. File should be 25fps.
> >>>>
> >>>>in libavformat/utils.c if I remove || CODEC_ID_MPEG2VIDEO, or revert
> >>>>commit r6345, it is correctly detected.
> >>>neither is acceptable, the code should be executed
> >>>find another solution :)
> >>>also it might be interresting if you would dump the timestamps for the
> >>>frames which are used in the r_frame_rate calculation ...
> >>>
> >>Ok that commit broke many more files than it solved. Can we revert it ?
> >
> >no because this isnt true, the code before should have failed for pretty 
> >much all telecined mpeg files
> >
> 
> I would prefer not to troll here, but I remember last time we were 
> talking about policy, it was said that a commit should not break working 
> scenario whatever it was, and that instead a clean a working for both 
> solution should be commited.
> 
> If the sample I supplied is broken, then Im ok to leave this but if not, 
> that file should be prefered over groundhog.vob, else this is a 
> regression, and IMHO not acceptable.

the only way to set r_frame_rate correctly is to read the whole file, this
isnt possible so we are left with a heuristic, also its not terribly
important variable, code which assumes it to be always correct should be
fixed ...

now to this specific case, the primary reason for the code IS telecined
mpeg and files which have no fps value but just timestamps and reverting
just the commit you complain about will break all telecined mpeg files 
as far as i can see ...

about policy, policy never said AFAIK you must not break anything, this is
of course not possible we are just humans ...
the intent is rather to not knowingly break something and i didnt, though
of course i should have tested the code more throughly

about the specific file, i didnt look at it yet but my guess would be that
it is broken otherwise the frame rate should be detected correctly but iam
just guessing here, ill look at it when i have time (its 4am here and i guess
i should sleep rather then playing with r_frame_rate ...)

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

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali