"Same as source" FPS Improvements

[van]

Done as http://trac.handbrake.fr/changeset/1250.

The big downside for your render fixes is that the improvement is so dramatic that now I have to go back and re-encode all my anime

[sdm]

Seems to be perfect! I too am doing a mass re-encoding.
It seems be safe to use the vfr/detelecine on all NTSC material.
Am I correct here?

I've seen QT report an average framerate of 23.97fps rather than 23.98fps. Dropping below 23.98fps shouldn't occur in ntsc material. Am I right?

Thanks for any thoughts...
--sdm.

[jbrjake]

The big downside for your render fixes is that the improvement is so dramatic that now I have to go back and re-encode all my anime

=)

Seems to be perfect! I too am doing a mass re-encoding.
It seems be safe to use the vfr/detelecine on all NTSC material.
Am I correct here?

Wellll....that's the theory. Whether it holds up in practice or not has yet to be seen. So if you come across any it doesn't work on, report them here.

[nightstrm]

So far I haven't come across anything that doesn't seem to play nicely with the new VFR. I'm hoping to start sending Band of Brothers episodes through soon (once I finish my Futurama/Simpsons encodes), which was well known for giving the old code major fits.

[sdm]

@nightstorm
I just did the first two episodes of Band of Brothers yesterday (seems to me some, but not all of the interviews were hard-telecined to ntsc video. credits are also ntsc video). I haven't watched the episodes but looking through it seems good!

-sdm.

[van]

jbrjake,

I've been trying to find problem cases for vfr (it's hard to do - it works *really* well). My wife bought me a set of Dorothy Sayers Mystery DVDs for Christmas that show some small problems. (I haven't looked in detail but they're hard telecined probably with a 5:6 pulldown from PAL progressive.) The VFR conversion was good but there was some juddering. I did a quick hack to print out the distribution of frame times and got:

Code: Select all

average frame 3302 ticks (27.259 fps)
3003  55834  69.3%
3754  18648  23.1%
4505   5309   6.6%
5255    480   0.6%
6007    203   0.3%
6758     99   0.1%
7509      4   0.0%
8255      1   0.0%
9010      2   0.0%

(the first column is the frame size in 90KHz ticks, the second is the number of frames of that size, the third is the number as % of total frames)

After seeing this I added code to print out the frame number of 6000 tick (60ms) and greater frames then scrubbed to those regions in qt player and verified that they were causing the juddering. After a quick look at the detelecine code I redid the encode using exactly the same parameters but setting strict_breaks to 'drop less' (-1) in the detelecine filter options. The result was pretty amazing - all the juddering went away but the detelecine was still perfect. This was the frame size distribution with strict_breaks=-1:

Code: Select all

average frame 3216 ticks (27.983 fps)
ticks number %total
3003  59340  71.7%
3754  23266  28.1%
4505    108   0.1%
5255      6   0.0%

so basically all it did was get rid of the long duration frames, turning most of them into 3754's.

I'd also been seeing some juddering when converting recent episodes of Soundstage which I thought was coming from the detelecine code not being tuned for 1080i HD frame sizes. But I re-ran the most recent four episodes with strict_breaks=-1 and, again, perfection - all the juddering went away but there were no interlace artifacts at all.

So if you see juddering try "strict_breaks" - it seems to bring VFR back to its normal state of perfection even on weird content. (I did the tests with the CLI since strict_breaks isn't accessible in the GUI but it might be worthwhile to add it at some point.)

Here's the shell script to display the frame size distribution if you want to play with it:
http://pastebin.ca/894740

Code: Select all

#! /bin/sh
# durdist <mp4file> - compute and print the distribution of video frame
#                     durations in <mp4file>
#
# This is a mickey mouse script to help look at the behavior of variable frame
# rate video encoding.  It requires that the mpeg4ip tool "mp4videoinfo" be
# somewhere in the current path.

mp4videoinfo "$1" | awk '/^sampleId/ {
    ts = int( substr($6, 1, index($6, "(") - 1)*90000/48000 + 0.5 )
    if (++totalFrames > 1)
        ++nframes[ts - l];
    l = ts
}
END {
    # group the frame lengths into classes
    for (i in nframes) {
        fclass = int(i/100)
        class[i] = fclass
        csum[fclass] += nframes[i]
        wsum += i * nframes[i] / totalFrames
    }
    # compute the weighted average length for each class
    for (i in nframes) {
        fclass = int(i/100)
        cwsum[fclass] += i * nframes[i] / csum[fclass]
    }
    printf "ticks number %%total\n"
    for (i in cwsum) {
        printf "%d %6d %5.1f%%\n", int(cwsum[i]+0.5), csum[i], csum[i]*100/totalFrames
    }
    printf "average frame %d ticks (%.3f fps)\n", int(wsum + 0.5), 90000/wsum 
}' | sort -n

[jbrjake]

Cool, useful research van. Thanks.

I've been thinking about strict breaks too, albeit in the other direction. The other day, I noticed that on one really, really nasty source ( http://samples.mplayerhq.hu/MPEG-VOB/telecine/ ), using denoising before detelecining would increase pullup's accuracy, dropping more frames. The heavier the denoising, the more interlacing it could detect. Then, when I switched to strict breaks 1, I got the same result with no denoising.

However, I'm somewhat wary of using strict breaks, whether they be positive or negative. First off, because I figure there has to be some reason Richard Felker made it use sb=0 by default. Second, because the gloss in the MPlayer docs kinda scares me. On the one hand, using 1 (what I needed for that Lost in Space sample linked above -- and it didn't even work completely with that-- has the caveat that:

Setting this option to 1 will reduce the chances of pullup generating an occasional mismatched frame, but it may also cause an excessive number of frames to be dropped during high motion sequences.

...while using -1, like you had to, has the danger that:

This may help processing of video where there is slight blurring between the fields, but may also cause there to be interlaced frames in the output.

We make that the default, and users will start complaining that if they have to deinterlace anyway to remove what pullup skipped, why bother detelecining at all?

I'll check how -1 works on that Lost in Space sample.

Maybe it's time to move on to something a little more sophisticated, and figure out a basic comb detection filter like rhester wants. Then we could use strict breaks = -1, run comb detection after ivtc, and if it finds interlacing, pass the GOP over to yadif.

[jbrjake]

Maybe it's time to move on to something a little more sophisticated, and figure out a basic comb detection filter like rhester wants. Then we could use strict breaks = -1, run comb detection after ivtc, and if it finds interlacing, pass the GOP over to yadif.

Wow, a lot's happened since February. The above has been accomplished -- detelecine defaults to breaks = -1, and comb detection can be run afterwards. But that's not why I'm reviving this old thread....

I don't know if there's any interest in this patch and, if so, if it's something that should wait until after 0.9.2. But we might want to at least add the instrumentation for the SyncVideo drops & dups to help diagnose future bug reports in this area.

I would guess there is huge interest, and I tend to agree that adding the instrumentation would be very useful for 0.9.2 but I'd certainly wait on the actual "fix" until the following release. It's too fundamental to the core to risk in 0.9.2.

I'd like to get this stuff into 0.9.2, but only as an option. Van, can you set it up so sync.c uses the old method of rescaling to inexact CFR speeds, except when job->vfr is true? Then, for 0.9.3, sync can always use the real frame durations

A couple months ago, after 0.9.2, van made the real frame duration stuff in sync for everything but avi. Unfortunately, there's been some negative feedback from users. As a temporary fix, van suggested using the old sync code if the title vrate was different from the job vrate. That worked great for people wanting to detelecine down to 24fps, or who have 59.94 broadcast captures. However, it leaves problems for people with 29.97 or 25fps or 23.976fps stuff, who want to keep it at that framerate, and who insist on using constant frame rates.

So, here's job->cfr. When it's true, sync will use the old constant frame rate code. When it's false, it will pass through the true frame durations. For the CLI, it will set job->cfr to 1 whenever the user provides a frame rate. This means the WinGui should default to not using the -r argument, unless a user gives a specific fps.

(this patch also contains some clean-up for the job struct to remove hard tabs and add comments)

Code: Select all

Index: test/test.c
===================================================================
--- test/test.c	(revision 1494)
+++ test/test.c	(working copy)
@@ -87,6 +87,7 @@
 static int    preset        = 0;
 static char * preset_name   = 0;
 static int    vfr           = 0;
+staitc int    cfr           = 0;
 static int    mp4_optimize  = 0;
 static int    ipod_atom     = 0;
 
@@ -884,6 +885,7 @@
             }
             if( vrate )
             {
+                job->cfr = 1;
                 job->vrate = 27000000;
                 job->vrate_base = vrate;
             }
Index: libhb/sync.c
===================================================================
--- libhb/sync.c	(revision 1493)
+++ libhb/sync.c	(working copy)
@@ -512,7 +512,7 @@
         }
 
         int64_t duration;
-        if ( job->mux & HB_MUX_AVI || job->title->rate_base != job->vrate_base )
+        if ( job->mux & HB_MUX_AVI || job->cfr )
         {
             /*
              * The concept of variable frame rate video was a bit too advanced
Index: libhb/common.h
===================================================================
--- libhb/common.h	(revision 1493)
+++ libhb/common.h	(working copy)
@@ -133,12 +133,13 @@
          width:               must be a multiple of 16
          height:              must be a multiple of 16
          keep_ratio:          used by UIs
+         grayscale:           black and white encoding
          pixel_ratio:         store pixel aspect ratio in the video
          pixel_aspect_width:  numerator for pixel aspect ratio
          pixel_aspect_height: denominator for pixel aspect ratio
-		 maxWidth:			  keep width below this
-		 maxHeight:			  keep height below this */
-
+         modulus:             set a number besides 16 for dimensions to be multiples of
+         maxWidth:            keep width below this
+         maxHeight:           keep height below this */
     int             crop[4];
     int             deinterlace;
     hb_list_t     * filters;
@@ -150,18 +151,21 @@
     int             pixel_aspect_width;
     int             pixel_aspect_height;
     int             modulus;
-	int				maxWidth;
-	int				maxHeight;
+    int             maxWidth;
+    int             maxHeight;
 
-
     /* Video settings:
          vcodec:            output codec
          vquality:          output quality (0.0..1.0)
-                            if < 0.0 or > 1.0, bitrate is used instead
+                            if < 0.0 or > 1.0, bitrate is used instead,
+                            except with x264, to use its real QP/RF scale
          vbitrate:          output bitrate (kbps)
+         vrate, vrate_base: output framerate is vrate / vrate_base
+         vfr:               boolean for variable frame rate detelecine
+         cfr:               boolean to use constant frame rates instead of
+                            passing through the source's frame durations.
          pass:              0, 1 or 2 (or -1 for scan)
-         vrate, vrate_base: output framerate is vrate / vrate_base
-         h264_level:        boolean for whether or not we're encoding for iPod
+         h264_level:        vestigial boolean to decide if we're encoding for iPod
          crf:               boolean for whether to use constant rate factor with x264
          x264opts:          string of extra x264 options
          areBframes:        boolean to note if b-frames are included in x264opts */
@@ -176,13 +180,14 @@
     int             vbitrate;
     int             vrate;
     int             vrate_base;
+    int             vfr;
+    int             cfr;
     int             pass;
     int             h264_13;
     int             h264_level;
     int             crf;
     char            *x264opts;
     int             areBframes;
-    int             vfr;
 
     /* List of audio settings. */
     hb_list_t     * list_audio;
@@ -191,7 +196,7 @@
          subtitle: index in hb_title_t's subtitles list, starting
          from 0. -1 means no subtitle */
     int             subtitle;
-    int 			subtitleSmartAdjust;
+    int             subtitleSmartAdjust;
 
     /* Muxer settings
          mux:  output file format

GUI wise, this naturally becomes "Same as Source" framerate, finally making that moniker legitimate. If a user moves from that default to a set frame rate, job->cfr should be set to 1. I'll leave the GUI code to Joe and John.

[dynaflash]

GUI wise, this naturally becomes "Same as Source" framerate, finally making that moniker legitimate. If a user moves from that default to a set frame rate, job->cfr should be set to 1. I'll leave the GUI code to Joe and John.

Here is the implementation for the macgui:

Code: Select all

Index: macosx/Controller.mm
===================================================================
--- macosx/Controller.mm	(revision 1495)
+++ macosx/Controller.mm	(working copy)
@@ -1534,11 +1534,18 @@
         job->vrate      = 27000000;
         job->vrate_base = hb_video_rates[[fVidRatePopUp
             indexOfSelectedItem]-1].rate;
+        /* We are not same as source so we set job->cfr to 1 
+         * to enable constant frame rate since user has specified
+         * a specific framerate*/
+        job->cfr = 1;
     }
     else
     {
         job->vrate      = title->rate;
         job->vrate_base = title->rate_base;
+        /* We are same as source so we set job->cfr to 0 
+         * to enable true same as source framerate */
+        job->cfr = 0;
     }
 
     switch( [fVidQualityMatrix selectedRow] )

[jbrjake]

These changes have been checked in as r1496 and r1497.

And that, I think, finally brings this topic to a close.