[Committed] He is Bob, eager for fun. He wears a smile.

[jbrjake]

So an interlaced, 30fps (3003 tick) frame contains two fields, images from two different moments in time. Ideally, it's supposed to be shown on an interlaced display running at 60hz, and each field gets displayed separately, half as long -- or at twice the framerate: 60 fields per second, ~1500 ticks a piece.

Currently, when HandBrake deinterlaces, it scales the first of those fields to full frame height. FPS remains the same -- that one field gets shown for a full 3003 ticks and the second field is thrown away.

With bob deinterlacing, we can scale both fields, and cut the time they're displayed down to ~1500 ticks. This bumps it up to 60fps, and in the process doubles the temporal resolution. After all, if the info's there in the source, why throw it away?

Patch: http://handbrake.fr/pastebin/pastebin.php?show=736

Changes entailed:

I'm using a job->bob variable to say whether or not the source is being bobbed. At the beginning of work.c, it'll walk the filter chain and see if decomb is there and, if so, if the first parameter includes bobbing. Messy, but it allows me easily let other parts of the lib know what's going on.

The obvious way of doing this would be to take advantage of already having the second field in the filter and just outputting two frames at once. However, due to the placement of the filters in the render workflow, that's just not feasible. It's in the middle of a long series of tasks that all assume they're working with one buffer at a time.

A few months ago, Ed suggested that I take advantage of hb_fifo_push_head(), which works great. That means most of the changes for this are in render.c. It copies every other frame into a new video buffer and pushes it back onto the sync fifo head. Decomb alternates which field it interpolates from. Then, when frames come out of decomb, their times gets halved in render.c.

Now, things get a little more interesting because we can combine this with combing detection for VFR deinterlacing.

Instead of copying all the frames at render time, we can wait until the decomb filter runs. Then, if can inform render that the frame currently being processed is/isn't combed. When the frame is combed, the unfiltered input is pushed back onto the fifo, and the current frame's time is halved.

Result: 23.976fps segments at 23.976fps, deinterlaced-in-the-source 29.97fps segments at 29.97fps, and interlaced-in-the-source 29.97fps segments at 59.9fps.

Bob is enabled in --decomb by adding 64 to the mode. So, for example, a basic yadif bob is --decomb=65, and a EEDI2->yadif->mcdeint bob is --decomb=89 .

Still to be done:

- The flow I'm using here won't work beyond this proof of concept. Yadif keeps a 1-frame delay. That means this isn't working precisely for sources that contain progressive frames. Render's info is one frame out of date -- it only knows it needs to dupe frame 0 once frame 1 has been sent to the filter, and that's too late. I'm going to have to move to a different model. Maybe dupe every frame and control it with FILTER_DROP messages from decomb.

- Frame durations are really rough since I've yet to make the numbers precise...I'm losing a tick on every other frame since I'm not storing the bias yet, just dividing 3003 by 2.

- There are some issues using this with the other filters that cache frames (detelecine and denoise).

- Sync/keyframe stuff

- Surely lots of other bugs to be worked through as well. I think there might still be some problems with frame parity, which frames show as combed, timecodes, etc.

[jbrjake]

A couple updates

- Brought the patch in line with the changes to framerate shaping in render.c through a one-line fix to pass through buffer settings.

- Inverted the bobbing flow. Instead of render.c duplicating frames based on whether the last frame it got out of decomb (with the penalty of a one frame delay from yadif) was combed, now render.c duplicates all frames, and throws away the copies on instructions from decomb when they aren't interlaced. An extra benefit is that this way, I'm able to set it up so blended frames don't get duplicated and shown twice. Now, the bobbing starts exactly when decomb starts employing yadif, and stops when decomb stops.

- Added yet another buffer setting to flag duplicate frames so they don't get copied more than once.

The next step is going to be figuring out how to make it play nice with detelecine. I need frames to get duplicated as they come out of detelecine, instead of before hitting any filters, and then I need to side-step detelecine when the frame coming down the pipeline is one that was duplicated in the prior iteration.

Patch:
http://handbrake.fr/pastebin/pastebin.php?show=1384

Code: Select all

Index: test/test.c
===================================================================
--- test/test.c	(revision 3277)
+++ test/test.c	(working copy)
@@ -1426,7 +1426,7 @@
                         fprintf(stderr, "ERROR: Invalid audio input track '%u', exiting.\n", 
                                 audio->in.track + 1 );
                         num_audio_tracks--;
-                        exit(3);
+//                        exit(3);
                     }
                 }
                 hb_list_rem(audios, audio);
Index: libhb/render.c
===================================================================
--- libhb/render.c	(revision 3277)
+++ libhb/render.c	(working copy)
@@ -32,6 +32,10 @@
     double               out_last_stop;     // where last frame ended (for CFR/PFR)
     int                  drops;             // frames dropped (for CFR/PFR)
     int                  dups;              // frames duped (for CFR/PFR)
+    
+    int                  alternator;        // 1 == the current frame is a dupe for bobbing
+    int                  last_bob_stop;     // for halving frame durs for bobbing
+    int                  last_frame_was_combed;
 };
 
 int  renderInit( hb_work_object_t *, hb_job_t * );
@@ -415,8 +419,44 @@
     /* Setup render buffer */
     hb_buffer_t * buf_render = hb_video_buffer_init( job->width, job->height );
 
+    /* Duplicate frames for bobbing */
+    if( job->bob )
+    {
+        if( !pv->alternator )
+        {
+            /* Duplicate this frame for bob deinterlacing */
+            hb_buffer_t * bob_buf = hb_video_buffer_init( title->width, title->height );
+            uint8_t * bobp = bob_buf->data;
+            uint8_t * inp = in->data;
+            int stride = title->width;
+            int w = title->width;
+            int h = title->height;
+            int k, y;
+            for( k = 0; k < 3; k++ )
+            {
+                if(k == 1)
+                {
+                    w /= 2;
+                    h /= 2;
+                }
+                for( y = h; y > 0 ; y-- )
+                {
+                    memcpy( bobp, inp, w );
+                    inp += w;
+                    bobp += w;
+                }
+            }
+            hb_buffer_copy_settings( bob_buf, in );
+            bob_buf->is_duplicate = 1;
+            hb_fifo_push_head( job->fifo_sync, bob_buf );
+        }
+
+        /* Toggles whether or not a frame gets duped.*/
+        pv->alternator = !pv->alternator;
+    }
+    
     /* Apply filters */
-    if( job->filters )
+    if( job->filters && buf_tmp_in )
     {
         int filter_count = hb_list_count( job->filters );
         int i;
@@ -429,9 +469,19 @@
             {
                 continue;
             }
-
+            
+            if( job->bob && buf_tmp_in->is_duplicate && filter->id == FILTER_DETELECINE )
+            {
+                /* We're bobbing, and there's no need to run detelecine
+                   on the frames that were duped for deinterlacing. */
+                continue;
+            }
+            
             hb_buffer_t * buf_tmp_out = NULL;
-
+            
+            /* Lets decomb know when to reset its parity alternation. */
+            buf_tmp_in->was_combed = pv->last_frame_was_combed;
+            
             int result = filter->work( buf_tmp_in,
                                        &buf_tmp_out,
                                        PIX_FMT_YUV420P,
@@ -447,6 +497,26 @@
              */
             if( result == FILTER_OK )
             {
+                if(filter->id == FILTER_DECOMB )
+                {
+                    pv->last_frame_was_combed = buf_tmp_out->was_combed;
+
+                    if( buf_tmp_out->was_combed == 1 )
+                    {
+                        if( job->bob )
+                        {
+                            /* Halve the frame's time for field rate. */
+                            uint64_t temp_duration = in->stop - in->start;
+                            in->start = pv->last_bob_stop;
+                            in->stop = pv->last_bob_stop + temp_duration / 2;
+                        }
+                    }
+                    
+                    /* So we know where to start the next bobbed frame. */
+                    pv->last_bob_stop = in->stop;
+                }
+                
+                hb_buffer_copy_settings( buf_tmp_out, buf_tmp_in );
                 buf_tmp_in = buf_tmp_out;
             }
             else if( result == FILTER_DELAY )
@@ -456,21 +526,24 @@
             }
             else if( result == FILTER_DROP )
             {
-                /* We need to compensate for the time lost by dropping this frame.
-                   Spread its duration out in quarters, because usually dropped frames
-                   maintain a 1-out-of-5 pattern and this spreads it out amongst the remaining ones.
-                   Store these in the lost_time array, which has 4 slots in it.
-                   Because not every frame duration divides evenly by 4, and we can't lose the
-                   remainder, we have to go through an awkward process to preserve it in the 4th array index. */
-                uint64_t temp_duration = buf_tmp_out->stop - buf_tmp_out->start;
-                pv->lost_time[0] += (temp_duration / 4);
-                pv->lost_time[1] += (temp_duration / 4);
-                pv->lost_time[2] += (temp_duration / 4);
-                pv->lost_time[3] += ( temp_duration - (temp_duration / 4) - (temp_duration / 4) - (temp_duration / 4) );
+                if( filter->id == FILTER_DETELECINE )
+                {
+                    /* We need to compensate for the time lost by dropping this frame.
+                       Spread its duration out in quarters, because usually dropped frames
+                       maintain a 1-out-of-5 pattern and this spreads it out amongst the remaining ones.
+                       Store these in the lost_time array, which has 4 slots in it.
+                       Because not every frame duration divides evenly by 4, and we can't lose the
+                       remainder, we have to go through an awkward process to preserve it in the 4th array index. */
+                    uint64_t temp_duration = buf_tmp_out->stop - buf_tmp_out->start;
+                    pv->lost_time[0] += (temp_duration / 4);
+                    pv->lost_time[1] += (temp_duration / 4);
+                    pv->lost_time[2] += (temp_duration / 4);
+                    pv->lost_time[3] += ( temp_duration - (temp_duration / 4) - (temp_duration / 4) - (temp_duration / 4) );
 
-                pv->total_lost_time += temp_duration;
-                pv->dropped_frames++;
-
+                    pv->total_lost_time += temp_duration;
+                    pv->dropped_frames++;
+                }
+                
                 /* Pop the frame's subtitle and dispose of it. */
                 hb_buffer_t * subtitles = hb_fifo_get( pv->subtitle_queue );
                 hb_buffer_close( &subtitles );
Index: libhb/internal.h
===================================================================
--- libhb/internal.h	(revision 3277)
+++ libhb/internal.h	(working copy)
@@ -71,6 +71,9 @@
     int           width;
     int           height;
 
+    int           was_combed;
+    int           is_duplicate;
+    
     hb_buffer_t * sub;
 
     hb_buffer_t * next;
Index: libhb/work.c
===================================================================
--- libhb/work.c	(revision 3277)
+++ libhb/work.c	(working copy)
@@ -868,6 +868,28 @@
         hb_log("work: only 1 chapter, disabling chapter markers");
     }
 
+    /* Check if we're bob deinterlacing */
+    if( hb_list_count( job->filters ) )
+    {
+        for( i = 0; i < hb_list_count( job->filters ); i++ )
+        {
+            hb_filter_object_t * filter = hb_list_item( job->filters, i );
+            if( filter->id == FILTER_DECOMB )
+            {
+                if (filter->settings)
+                {
+                    int decomb_mode;
+                    sscanf( filter->settings, "%d:", &decomb_mode);
+                    if( decomb_mode & 64 )
+                    {
+                        job->bob = 1;
+                    }
+                }
+            }
+        }
+    }
+    
+    
     /* Display settings */
     hb_display_job_info( job );
 
Index: libhb/fifo.c
===================================================================
--- libhb/fifo.c	(revision 3277)
+++ libhb/fifo.c	(working copy)
@@ -220,6 +220,8 @@
     dst->new_chap  = src->new_chap;
     dst->frametype = src->frametype;
     dst->flags     = src->flags;
+    dst->was_combed = src->was_combed;
+    dst->is_duplicate = src->is_duplicate;
 }
 
 hb_fifo_t * hb_fifo_init( int capacity, int thresh )
Index: libhb/decomb.c
===================================================================
--- libhb/decomb.c	(revision 3277)
+++ libhb/decomb.c	(working copy)
@@ -31,6 +31,7 @@
 #define MODE_EEDI2       8 // Use EEDI2 interpolation
 #define MODE_MCDEINT    16 // Post-process with mcdeint
 #define MODE_MASK       32 // Output combing masks instead of pictures
+#define MODE_BOB        64 // Double framerate
 
 /***** 
 These modes can be layered. For example, Yadif (1) + EEDI2 (8) = 9,
@@ -206,7 +207,8 @@
     hb_lock_t      ** eedi2_complete_lock;  // Thread has completed work
     eedi2_arguments_t *eedi2_arguments;    // Arguments to thread for work
 
-//    int              alternator;           // for bobbing parity when framedoubling
+    int              alternator;           // for bobbing parity when framedoubling
+    int              last_frame_was_combed;
 };
 
 hb_filter_private_t * hb_decomb_init( int pix_fmt,
@@ -1661,7 +1663,6 @@
     
     pv->cpu_count = hb_get_cpu_count();
     
-
     if( pv->mode & MODE_MCDEINT )
     {
         pv->mcdeint_mode = 2;
@@ -2101,6 +2102,15 @@
         return FILTER_FAILED;
     }
 
+
+    if( pv->mode & MODE_BOB && (pv->last_frame_was_combed != 1) && pv->alternator == 1 )
+    {
+        /* This frame isn't combed, so it's not going to get
+           duped by render. Reset the parity for the next frame. */
+        pv->alternator = 0;
+        return FILTER_DROP;
+    }
+    
     avpicture_fill( &pv->pic_in, buf_in->data,
                     pix_fmt, width, height );
 
@@ -2115,8 +2125,12 @@
         tff = (pv->parity & 1) ^ 1;
     }
 
-    /* Store current frame in yadif cache */
-    store_ref( (const uint8_t**)pv->pic_in.data, pv );
+    if( !( pv->mode & MODE_BOB ) || ( pv->mode & MODE_BOB && !pv->alternator ) )
+    {
+        /* Store current frame in yadif cache
+           unless this is a duped frame for bob. */
+        store_ref( (const uint8_t**)pv->pic_in.data, pv );        
+    }
 
     /* If yadif is not ready, store another ref and return FILTER_DELAY */
     if( pv->yadif_ready == 0 )
@@ -2133,12 +2147,30 @@
         return FILTER_DELAY;
     }
 
-    /* Perform yadif filtering */        
-    int frame;
-    for( frame = 0; frame <= ( ( pv->mode & MODE_MCDEINT ) ? 1 : 0 ) ; frame++ )
-// This would be what to use for bobbing: for( frame = 0; frame <= 0 ; frame++ )
+    /* Perform yadif filtering */
+
+    int frame, max_frame;
+
+    max_frame = 0;
+    
+    if( pv->mode & MODE_BOB )
     {
+        /* We're duping frames during rendering, so only
+           pass one field through the deinterlacer. */
+        max_frame = 0;
+    }
+    else if( pv->mode & MODE_MCDEINT )
+    {
+        /* When using mcdeint and not bobbing, it needs
+           to look at each frame twice to see each field. */
+        max_frame = 1;
+    }
 
+    int current_buffer = 1;
+    
+    for( frame = 0; frame <= max_frame; frame++ )
+    {
+
 #if 0        
         /* Perhaps skip the second run if the frame is uncombed? */
         if( frame && !pv->yadif_arguments[0].is_combed )
@@ -2148,21 +2180,30 @@
 #endif        
         int parity = frame ^ tff ^ 1;
 
-// This will be for bobbing
-#if 0
-        if( pv->alternator )
+        if( pv->mode & MODE_BOB )
         {
-            parity = !parity;
-            pv->alternator = 0;
+            if( pv->alternator )
+            {
+                /* This frame was duped at render time.
+                   Reverse the parity, so it looks at
+                   the second field, this time around. */
+                parity = !parity;
+                pv->alternator = 0;
+                current_buffer = 0;
+            }
+            else
+            {
+                /* This is the first run-through of this frame,
+                   mark it down so we know to reverse parity
+                   when it comes through a second time. */
+                pv->alternator = 1;
+            current_buffer = 1;
+            }
         }
-        else
-        {
-            pv->alternator = 1;
-        }
-#endif
+        
         pv->tff = !parity;
 
-        avpicture_fill( &pv->pic_out, pv->buf_out[!(frame^1)]->data,
+        avpicture_fill( &pv->pic_out, pv->buf_out[current_buffer]->data,
                         pix_fmt, width, height );
 
         /* XXX
@@ -2179,20 +2220,31 @@
         if( pv->mcdeint_mode >= 0 /* && pv->yadif_arguments[0].is_combed */)
         {
             /* Perform mcdeint filtering */
-            avpicture_fill( &pv->pic_in,  pv->buf_out[(frame^1)]->data,
+            avpicture_fill( &pv->pic_in,  pv->buf_out[!current_buffer]->data,
                             pix_fmt, width, height );
 
             mcdeint_filter( pv->pic_in.data, pv->pic_out.data, parity, pv );
+            *buf_out = pv->buf_out[!current_buffer];            
         }
+        else
+        {
+            *buf_out = pv->buf_out[current_buffer];
+        }
 
-        *buf_out = pv->buf_out[!(frame^1)];
     }
 
+    /* Inform render what the filter's decided for this frame. */
+    pv->buf_settings->was_combed = pv->yadif_arguments[0].is_combed;
+    pv->last_frame_was_combed = pv->yadif_arguments[0].is_combed;
+    
     /* Copy buffered settings to output buffer settings */
     hb_buffer_copy_settings( *buf_out, pv->buf_settings );
 
-    /* Replace buffered settings with input buffer settings */
-    hb_buffer_copy_settings( pv->buf_settings, buf_in );
+    if( !(pv->mode & MODE_BOB) || ( pv->mode & MODE_BOB && !pv->buf_settings->was_combed ) )
+    {
+        /* Replace buffered settings with input buffer settings */
+        hb_buffer_copy_settings( pv->buf_settings, buf_in );        
+    }
 
     /* don't let 'work_loop' send a chapter mark upstream */
     buf_in->new_chap  = 0;
Index: libhb/common.h
===================================================================
--- libhb/common.h	(revision 3277)
+++ libhb/common.h	(working copy)
@@ -230,6 +230,7 @@
     char            *x264opts;
     int             areBframes;
     int             color_matrix;
+    int             bob;
 
     /* List of audio settings. */
     hb_list_t     * list_audio;

[Deleted User 11865]

When you set the parity correctly, it seems to work (though tbh I only tested mode 65).

I did notice one thing though: without the patch, comparing the output of normal decomb (yadif, no combing detection, no frame doubling) to decomb off in QuickTime 7, the timestamps appear to be the same for both files (start at 0:00:00:00.00, then 0:00:00:00.07, 0:00:00:00.11, 0:00:00:00.15, 0:00:00:00.17 etc.).

However, after applying the patch, decomb off output is still identical to pre-patch, but "normal" decomb (same as above) has slightly different timestamps (start at 0:00:00:00.00, then 0:00:00:00.04, 0:00:00:00.08, 0:00:00:00.10, 0:00:00:00.16 etc.). All other data (number of frames, bitrate - both as reported by QT and the log - and the frames themselves) are identical.

No idea what this means. Anyway, sample and sample encodes: http://drop.io/gtrrpvk

[Deleted User 11865]

The universal subtitle input patch broke this patch in internal.h. It's a trivial fix, here's the same patch against svn3285: http://handbrake.fr/pastebin/pastebin.php?show=1399

[Deleted User 11865]

Regarding the timecode mismatch, I found out that if I comment out line 120 of the patch:

Code: Select all

                hb_buffer_copy_settings( buf_tmp_out, buf_tmp_in );

the output is the same as before the patch (same timecodes with decomb on and off). Of course it does break bobbing…

[jbrjake]

Thanks, Rodeo.

Nesting that settings copy in a bob-only conditional removes the regression. I should probably revisit this and figure out exactly what's wrong. Copying the settings shouldn't have had any ill effect without bobbing enabled. Makes me suspect that maybe render is incorrectly assigning the settings of the current frame to the frame that comes out of the filter chain, even when the frame coming out is older due to a filter delay.....

Updated patch:
http://handbrake.fr/pastebin/pastebin.php?show=1423

Code: Select all

Index: libhb/render.c
===================================================================
--- libhb/render.c	(revision 3307)
+++ libhb/render.c	(working copy)
@@ -32,6 +32,10 @@
     double               out_last_stop;     // where last frame ended (for CFR/PFR)
     int                  drops;             // frames dropped (for CFR/PFR)
     int                  dups;              // frames duped (for CFR/PFR)
+    
+    int                  alternator;        // 1 == the current frame is a dupe for bobbing
+    int                  last_bob_stop;     // for halving frame durs for bobbing
+    int                  last_frame_was_combed;
 };
 
 int  renderInit( hb_work_object_t *, hb_job_t * );
@@ -415,8 +419,44 @@
     /* Setup render buffer */
     hb_buffer_t * buf_render = hb_video_buffer_init( job->width, job->height );
 
+    /* Duplicate frames for bobbing */
+    if( job->bob )
+    {
+        if( !pv->alternator )
+        {
+            /* Duplicate this frame for bob deinterlacing */
+            hb_buffer_t * bob_buf = hb_video_buffer_init( title->width, title->height );
+            uint8_t * bobp = bob_buf->data;
+            uint8_t * inp = in->data;
+            int stride = title->width;
+            int w = title->width;
+            int h = title->height;
+            int k, y;
+            for( k = 0; k < 3; k++ )
+            {
+                if(k == 1)
+                {
+                    w /= 2;
+                    h /= 2;
+                }
+                for( y = h; y > 0 ; y-- )
+                {
+                    memcpy( bobp, inp, w );
+                    inp += w;
+                    bobp += w;
+                }
+            }
+            hb_buffer_copy_settings( bob_buf, in );
+            bob_buf->is_duplicate = 1;
+            hb_fifo_push_head( job->fifo_sync, bob_buf );
+        }
+
+        /* Toggles whether or not a frame gets duped.*/
+        pv->alternator = !pv->alternator;
+    }
+    
     /* Apply filters */
-    if( job->filters )
+    if( job->filters && ( (job->bob && buf_tmp_in) || !job->bob ) )
     {
         int filter_count = hb_list_count( job->filters );
         int i;
@@ -429,9 +469,19 @@
             {
                 continue;
             }
-
+            
+            if( job->bob && buf_tmp_in->is_duplicate && filter->id == FILTER_DETELECINE )
+            {
+                /* We're bobbing, and there's no need to run detelecine
+                   on the frames that were duped for deinterlacing. */
+                continue;
+            }
+            
             hb_buffer_t * buf_tmp_out = NULL;
-
+            
+            /* Lets decomb know when to reset its parity alternation. */
+            buf_tmp_in->was_combed = pv->last_frame_was_combed;
+            
             int result = filter->work( buf_tmp_in,
                                        &buf_tmp_out,
                                        PIX_FMT_YUV420P,
@@ -447,6 +497,26 @@
              */
             if( result == FILTER_OK )
             {
+                if( filter->id == FILTER_DECOMB && job->bob )
+                {
+                    pv->last_frame_was_combed = buf_tmp_out->was_combed;
+                    
+                    if( buf_tmp_out->was_combed == 1 )
+                    {
+                            /* Halve the frame's time for field rate. */
+                            uint64_t temp_duration = in->stop - in->start;
+                            in->start = pv->last_bob_stop;
+                            in->stop = pv->last_bob_stop + temp_duration / 2;
+                    }
+
+                    /* So we know where to start the next bobbed frame. */
+                    pv->last_bob_stop = in->stop;
+                    
+                    /* Ensure changes to frame flags by the bobber are preserved
+                       for the rest of the filter and render chain. */
+                    hb_buffer_copy_settings( buf_tmp_out, buf_tmp_in );
+                }
+                
                 buf_tmp_in = buf_tmp_out;
             }
             else if( result == FILTER_DELAY )
@@ -456,21 +526,24 @@
             }
             else if( result == FILTER_DROP )
             {
-                /* We need to compensate for the time lost by dropping this frame.
-                   Spread its duration out in quarters, because usually dropped frames
-                   maintain a 1-out-of-5 pattern and this spreads it out amongst the remaining ones.
-                   Store these in the lost_time array, which has 4 slots in it.
-                   Because not every frame duration divides evenly by 4, and we can't lose the
-                   remainder, we have to go through an awkward process to preserve it in the 4th array index. */
-                uint64_t temp_duration = buf_tmp_out->stop - buf_tmp_out->start;
-                pv->lost_time[0] += (temp_duration / 4);
-                pv->lost_time[1] += (temp_duration / 4);
-                pv->lost_time[2] += (temp_duration / 4);
-                pv->lost_time[3] += ( temp_duration - (temp_duration / 4) - (temp_duration / 4) - (temp_duration / 4) );
+                if( filter->id == FILTER_DETELECINE )
+                {
+                    /* We need to compensate for the time lost by dropping this frame.
+                       Spread its duration out in quarters, because usually dropped frames
+                       maintain a 1-out-of-5 pattern and this spreads it out amongst the remaining ones.
+                       Store these in the lost_time array, which has 4 slots in it.
+                       Because not every frame duration divides evenly by 4, and we can't lose the
+                       remainder, we have to go through an awkward process to preserve it in the 4th array index. */
+                    uint64_t temp_duration = buf_tmp_out->stop - buf_tmp_out->start;
+                    pv->lost_time[0] += (temp_duration / 4);
+                    pv->lost_time[1] += (temp_duration / 4);
+                    pv->lost_time[2] += (temp_duration / 4);
+                    pv->lost_time[3] += ( temp_duration - (temp_duration / 4) - (temp_duration / 4) - (temp_duration / 4) );
 
-                pv->total_lost_time += temp_duration;
-                pv->dropped_frames++;
-
+                    pv->total_lost_time += temp_duration;
+                    pv->dropped_frames++;
+                }
+                
                 /* Pop the frame's subtitle and dispose of it. */
                 hb_buffer_t * subtitles = hb_fifo_get( pv->subtitle_queue );
                 hb_buffer_close( &subtitles );
Index: libhb/internal.h
===================================================================
--- libhb/internal.h	(revision 3307)
+++ libhb/internal.h	(working copy)
@@ -72,6 +72,10 @@
     /* Holds the output PTS from x264, for use by b-frame offsets in muxmp4.c */
     int64_t     renderOffset;
 
+    /* For bobbing */
+    int           was_combed;
+    int           is_duplicate;
+
     // VOB subtitle packets:
     //   Location and size of the subpicture.
     int           x;
Index: libhb/work.c
===================================================================
--- libhb/work.c	(revision 3307)
+++ libhb/work.c	(working copy)
@@ -866,6 +866,28 @@
         hb_log("work: only 1 chapter, disabling chapter markers");
     }
 
+    /* Check if we're bob deinterlacing */
+    if( hb_list_count( job->filters ) )
+    {
+        for( i = 0; i < hb_list_count( job->filters ); i++ )
+        {
+            hb_filter_object_t * filter = hb_list_item( job->filters, i );
+            if( filter->id == FILTER_DECOMB )
+            {
+                if (filter->settings)
+                {
+                    int decomb_mode;
+                    sscanf( filter->settings, "%d:", &decomb_mode);
+                    if( decomb_mode & 64 )
+                    {
+                        job->bob = 1;
+                    }
+                }
+            }
+        }
+    }
+    
+    
     /* Display settings */
     hb_display_job_info( job );
 
Index: libhb/fifo.c
===================================================================
--- libhb/fifo.c	(revision 3307)
+++ libhb/fifo.c	(working copy)
@@ -220,6 +220,8 @@
     dst->new_chap  = src->new_chap;
     dst->frametype = src->frametype;
     dst->flags     = src->flags;
+    dst->was_combed = src->was_combed;
+    dst->is_duplicate = src->is_duplicate;
 }
 
 hb_fifo_t * hb_fifo_init( int capacity, int thresh )
Index: libhb/decomb.c
===================================================================
--- libhb/decomb.c	(revision 3307)
+++ libhb/decomb.c	(working copy)
@@ -31,6 +31,7 @@
 #define MODE_EEDI2       8 // Use EEDI2 interpolation
 #define MODE_MCDEINT    16 // Post-process with mcdeint
 #define MODE_MASK       32 // Output combing masks instead of pictures
+#define MODE_BOB        64 // Double framerate
 
 /***** 
 These modes can be layered. For example, Yadif (1) + EEDI2 (8) = 9,
@@ -206,7 +207,8 @@
     hb_lock_t      ** eedi2_complete_lock;  // Thread has completed work
     eedi2_arguments_t *eedi2_arguments;    // Arguments to thread for work
 
-//    int              alternator;           // for bobbing parity when framedoubling
+    int              alternator;           // for bobbing parity when framedoubling
+    int              last_frame_was_combed;
 };
 
 hb_filter_private_t * hb_decomb_init( int pix_fmt,
@@ -1661,7 +1663,6 @@
     
     pv->cpu_count = hb_get_cpu_count();
     
-
     if( pv->mode & MODE_MCDEINT )
     {
         pv->mcdeint_mode = 2;
@@ -2101,6 +2102,15 @@
         return FILTER_FAILED;
     }
 
+
+    if( pv->mode & MODE_BOB && (pv->last_frame_was_combed != 1) && pv->alternator == 1 )
+    {
+        /* This frame isn't combed, so it's not going to get
+           duped by render. Reset the parity for the next frame. */
+        pv->alternator = 0;
+        return FILTER_DROP;
+    }
+    
     avpicture_fill( &pv->pic_in, buf_in->data,
                     pix_fmt, width, height );
 
@@ -2115,8 +2125,12 @@
         tff = (pv->parity & 1) ^ 1;
     }
 
-    /* Store current frame in yadif cache */
-    store_ref( (const uint8_t**)pv->pic_in.data, pv );
+    if( !( pv->mode & MODE_BOB ) || ( pv->mode & MODE_BOB && !pv->alternator ) )
+    {
+        /* Store current frame in yadif cache
+           unless this is a duped frame for bob. */
+        store_ref( (const uint8_t**)pv->pic_in.data, pv );        
+    }
 
     /* If yadif is not ready, store another ref and return FILTER_DELAY */
     if( pv->yadif_ready == 0 )
@@ -2133,12 +2147,30 @@
         return FILTER_DELAY;
     }
 
-    /* Perform yadif filtering */        
-    int frame;
-    for( frame = 0; frame <= ( ( pv->mode & MODE_MCDEINT ) ? 1 : 0 ) ; frame++ )
-// This would be what to use for bobbing: for( frame = 0; frame <= 0 ; frame++ )
+    /* Perform yadif filtering */
+
+    int frame, max_frame;
+
+    max_frame = 0;
+    
+    if( pv->mode & MODE_BOB )
     {
+        /* We're duping frames during rendering, so only
+           pass one field through the deinterlacer. */
+        max_frame = 0;
+    }
+    else if( pv->mode & MODE_MCDEINT )
+    {
+        /* When using mcdeint and not bobbing, it needs
+           to look at each frame twice to see each field. */
+        max_frame = 1;
+    }
 
+    int current_buffer = 1;
+    
+    for( frame = 0; frame <= max_frame; frame++ )
+    {
+
 #if 0        
         /* Perhaps skip the second run if the frame is uncombed? */
         if( frame && !pv->yadif_arguments[0].is_combed )
@@ -2148,21 +2180,30 @@
 #endif        
         int parity = frame ^ tff ^ 1;
 
-// This will be for bobbing
-#if 0
-        if( pv->alternator )
+        if( pv->mode & MODE_BOB )
         {
-            parity = !parity;
-            pv->alternator = 0;
+            if( pv->alternator )
+            {
+                /* This frame was duped at render time.
+                   Reverse the parity, so it looks at
+                   the second field, this time around. */
+                parity = !parity;
+                pv->alternator = 0;
+                current_buffer = 0;
+            }
+            else
+            {
+                /* This is the first run-through of this frame,
+                   mark it down so we know to reverse parity
+                   when it comes through a second time. */
+                pv->alternator = 1;
+            current_buffer = 1;
+            }
         }
-        else
-        {
-            pv->alternator = 1;
-        }
-#endif
+        
         pv->tff = !parity;
 
-        avpicture_fill( &pv->pic_out, pv->buf_out[!(frame^1)]->data,
+        avpicture_fill( &pv->pic_out, pv->buf_out[current_buffer]->data,
                         pix_fmt, width, height );
 
         /* XXX
@@ -2179,20 +2220,31 @@
         if( pv->mcdeint_mode >= 0 /* && pv->yadif_arguments[0].is_combed */)
         {
             /* Perform mcdeint filtering */
-            avpicture_fill( &pv->pic_in,  pv->buf_out[(frame^1)]->data,
+            avpicture_fill( &pv->pic_in,  pv->buf_out[!current_buffer]->data,
                             pix_fmt, width, height );
 
             mcdeint_filter( pv->pic_in.data, pv->pic_out.data, parity, pv );
+            *buf_out = pv->buf_out[!current_buffer];            
         }
+        else
+        {
+            *buf_out = pv->buf_out[current_buffer];
+        }
 
-        *buf_out = pv->buf_out[!(frame^1)];
     }
 
+    /* Inform render what the filter's decided for this frame. */
+    pv->buf_settings->was_combed = pv->yadif_arguments[0].is_combed;
+    pv->last_frame_was_combed = pv->yadif_arguments[0].is_combed;
+    
     /* Copy buffered settings to output buffer settings */
     hb_buffer_copy_settings( *buf_out, pv->buf_settings );
 
-    /* Replace buffered settings with input buffer settings */
-    hb_buffer_copy_settings( pv->buf_settings, buf_in );
+    if( !(pv->mode & MODE_BOB) || ( pv->mode & MODE_BOB && !pv->buf_settings->was_combed ) )
+    {
+        /* Replace buffered settings with input buffer settings */
+        hb_buffer_copy_settings( pv->buf_settings, buf_in );        
+    }
 
     /* don't let 'work_loop' send a chapter mark upstream */
     buf_in->new_chap  = 0;
Index: libhb/common.h
===================================================================
--- libhb/common.h	(revision 3307)
+++ libhb/common.h	(working copy)
@@ -232,6 +232,7 @@
     char            *x264opts;
     int             areBframes;
     int             color_matrix;
+    int             bob;
 
     /* List of audio settings. */
     hb_list_t     * list_audio;

Would have had this up last week, but I thought there were further problems since I was comparing mode 7 (yadif+cubic/blend) to mode 65 (yadif bob) instead of to mode 72 (yadif+cubic/blend bob. Doh.

EDIT:
Oh yeah, and as a note to myself so I don't forget...the specific timecode problem the settings copy caused was that the first frame got a duration equal to its actual duration plus the time until the first sync point. So it was working with 0 == video track start instead of 0 == synchronized 0.

[Deleted User 11865]

FWIW, the new patch does appear to fix the issue I reported. Thanks.

[rlevis]

Just to let you know I, like many others, am waiting patiently for this patch to be committed and available in the GUI. A much appreciated feature!

[jamiemlaw]

I was wondering, does this patch work with the current SVN, or is it way out of date now?

[Deleted User 11865]

I was wondering, does this patch work with the current SVN, or is it way out of date now?

Still works for me.

[jamiemlaw]

I'm getting build errors when I apply the patch.

Code: Select all

jamie-imac:handbrake-svn2 jamie$ svn co svn://svn.handbrake.fr/HandBrake/trunk handbrake-svn

...

Checked out revision 3749.
jamie-imac:~ jamie$ cd handbrake-svn/

# I move the diff file to handbrake-svn manually.

jamie-imac:handbrake-svn jamie$ ls
1423.diff	COPYING		DoxyfileDotNet	DoxyfileLibHb	NEWS		TRANSLATIONS	contrib		gtk		macosx		pkg		scripts		win
AUTHORS		CREDITS		DoxyfileGtk	DoxyfileMac	THANKS		configure	doc		libhb		make		qt4		test
jamie-imac:handbrake-svn jamie$ patch -p0 < 1423.diff 
(Stripping trailing CRs from patch.)
patching file libhb/render.c
Hunk #5 succeeded at 526 with fuzz 2.
(Stripping trailing CRs from patch.)
patching file libhb/internal.h
Hunk #1 FAILED at 72.
1 out of 1 hunk FAILED -- saving rejects to file libhb/internal.h.rej
(Stripping trailing CRs from patch.)
patching file libhb/work.c
Hunk #1 succeeded at 908 (offset 42 lines).
(Stripping trailing CRs from patch.)
patching file libhb/fifo.c
Hunk #1 succeeded at 225 (offset 5 lines).
(Stripping trailing CRs from patch.)
patching file libhb/decomb.c
(Stripping trailing CRs from patch.)
patching file libhb/common.h
Hunk #1 succeeded at 238 (offset 6 lines).
jamie-imac:handbrake-svn jamie$ ./configure --launch --launch-jobs=0 ; open build/

...

  : ** BUILD FAILED **
  : 
  : 
  : The following build commands failed:
  : external:
  : 	ExternalBuildToolExecution external
  : HandBrakeCLI:
  : 	Ld ../build/HandBrakeCLI normal x86_64
  : HandBrake:
  : 	Ld ../build/HandBrake.app/Contents/MacOS/HandBrake normal x86_64
  : (3 failures)
  : 
  : make: *** [macosx.build] Error 1
-------------------------------------------------------------------------------
time end: Sat Jan 15 23:22:34 2011
duration: 10 minutes, 33 seconds (633.10s)
result: FAILURE (code 2)
-------------------------------------------------------------------------------
Build is finished!
You may now cd into ./build and examine the output.
jamie-imac:handbrake-svn jamie$ 

It seems to me that the patch file isn't applying correctly. Is that due to a bad command on my part, or does it need updating?

[Deleted User 11865]

It's possible my copy of the patch is updated. Let me check.

[Deleted User 11865]

It's possible my copy of the patch is updated. Let me check.

This should apply to the latest trunk: http://paste.handbrake.fr/pastebin.php?show=2016

P.S. you could see in your output that the patch didn't apply properly:

Code: Select all

jamie-imac:handbrake-svn jamie$ patch -p0 < 1423.diff 
(Stripping trailing CRs from patch.)
patching file libhb/render.c
Hunk #5 succeeded at 526 with fuzz 2.
(Stripping trailing CRs from patch.)
patching file libhb/internal.h
Hunk #1 FAILED at 72.
1 out of 1 hunk FAILED -- saving rejects to file libhb/internal.h.rej

[jamiemlaw]

You're a star. It's getting late over here so I'll test tomorrow.

I think I've been conditioned by these forums to assume that anything that goes wrong turns out to be my fault, even if it isn't!

[randomreuben]

EDIT: Deleted by me. Wrong post, wrong forum. My bad.

[tvperez]

This patch is great! It works really well. However, it still doesn't seem to work with Detelecine. Everytime I activate Detelecine, any frames that have been bobbed seems to run at half the speed (as if it were in slow motion). Another words, it appears that detelecine forces a 59.99 fps bobbed decombed segment to be played at 24 fps. Is anybody else having this issue?

[JohnAStebbins]

I'm not sure what detelecine is doing to cause this, but the detelecine filter runs before the decomb/deinterlace filter. I'll have to try it some time to see what the interaction is. So if anything, it's the bobber getting confused by progressive frames created by detelecine.

[tvperez]

I'm not sure what detelecine is doing to cause this, but the detelecine filter runs before the decomb/deinterlace filter. I'll have to try it some time to see what the interaction is. So if anything, it's the bobber getting confused by progressive frames created by detelecine.

Thanks.

Also, I have been trying the Bob on some massively interlaced sources (Anime: The Big O) and found that the audio and video becomes out-of-sync as the each episode is played. I am not sure what's causing this issue.

[Deleted User 11865]

Also, I have been trying the Bob on some massively interlaced sources (Anime: The Big O) and found that the audio and video becomes out-of-sync as the each episode is played. I am not sure what's causing this issue.

I've had desync issues too. Back when I was still using the patch, I always disabled combing detection when bobbing (<mode><otherdecombparams>).

[tvperez]

I've had desync issues too. Back when I was still using the patch, I always disabled combing detection when bobbing (<mode><otherdecombparams>).

So, are you saying that if I disabled the combing detection, bobbing would remain desync'ed? I have tried with decombing detection activated (with many different combinations), and they all seem to produce desync'ed files.

Thank you for your help.

[Deleted User 11865]

No; I meant that (IIRC) I didn't have desync issues if I disabled combing detection (in which case decomb considers all frames as combed, and basically just deinterlaces).

[tvperez]

That seemed to do the trick. Thank you for the advice.

EDIT: I think I found a solution to this issue. I made the following changes in two files:

decomb.c:
Line 2109

Code: Select all

//if( pv->mode & MODE_BOB && (pv->last_frame_was_combed != 1) && pv->alternator == 1 )
    //{
        /* This frame isn't combed, so it's not going to get
           duped by render. Reset the parity for the next frame. */
        //pv->alternator = 0;
        //return FILTER_DROP;
    //}

render.c:
Line 523

Code: Select all

                   //if( buf_tmp_out->was_combed == 1 )
                    //{
                            /* Halve the frame's time for field rate. */
                            uint64_t temp_duration = in->stop - in->start;
                            in->start = pv->last_bob_stop;
                            in->stop = pv->last_bob_stop + (temp_duration / 2);
                            /* So we know where to start the next bobbed frame. */
                            pv->last_bob_stop = in->stop;
                    //}

This seems to allow the decomb to process without desync'ing the audo.

[tvperez]

After looking at the code a bit further, I found that if I revert the decomb code:

Code: Select all

if( pv->mode & MODE_BOB && (pv->last_frame_was_combed != 1) && pv->alternator == 1 )
 {
        /* This frame isn't combed, so it's not going to get
           duped by render. Reset the parity for the next frame. */
        pv->alternator = 0;
        return FILTER_DROP;
 }

And added the following code in render.c (line 569):

Code: Select all

                if( filter->id == FILTER_DETELECINE )
                {
                    /* We need to compensate for the time lost by dropping this frame.
                       Spread its duration out in quarters, because usually dropped frames
                       maintain a 1-out-of-5 pattern and this spreads it out amongst the remaining ones.
                       Store these in the lost_time array, which has 4 slots in it.
                       Because not every frame duration divides evenly by 4, and we can't lose the
                       remainder, we have to go through an awkward process to preserve it in the 4th array index. */
                    uint64_t temp_duration = buf_tmp_out->stop - buf_tmp_out->start;
                    pv->lost_time[0] += (temp_duration / 4);
                    pv->lost_time[1] += (temp_duration / 4);
                    pv->lost_time[2] += (temp_duration / 4);
                    pv->lost_time[3] += ( temp_duration - (temp_duration / 4) - (temp_duration / 4) - (temp_duration / 4) );
                    
                    pv->total_lost_time += temp_duration;
                    pv->dropped_frames++;
                 }
	else
	{
	   uint64_t temp_duration = in->stop - in->start;
                    pv->lost_time[0] += 0;
                    pv->lost_time[1] += 0;
                    pv->lost_time[2] += 0;
                    pv->lost_time[3] +=  temp_duration/2;
                    
                    pv->total_lost_time += temp_duration/2;
                    pv->dropped_frames++;
	}

So far my tests have produced expected transcoded files. It seems Bob is looking good (with the exception of Detelecine, which is still not working).

[tvperez]

Finally go the Detelecine to work with Bob. It required a few changes to the code (all in render.c):

Line 35:

Code: Select all

    int                  alternator;
    int                  last_bob_stop;
    int                  last_frame_was_combed;
    int                  detelecineOccurred;

Line 406

Code: Select all

    /* This ensures that a frame that has been removed due to Detelecine isn't combed unnecessarily. */
    if (pv->detelecineOccurred)
    {
        pv->detelecineOccurred = 0;
        return HB_WORK_OK;
    }

Line 591

Code: Select all

         /* Mark the frame as deleted so to avoid recombing it again. */
         pv->alternator = 0;
         pv->detelecineOccurred = 1;

[JohnAStebbins]

Finally go the Detelecine to work with Bob. It required a few changes to the code (all in render.c):

This would all be much easier to read and understand if it were submitted as a patch.
Unfortunately, svn doesn't make it easy to create patches against already patched code. You could create 2 clean copies of the source, apply the base bob patch to one. Then apply the base bob patch plus your changes. Then diff -Naur hb.bob bh.mybob

Alternatively, git-svn is real handy for such things.

Code: Select all

# clone handbrake svn
git svn clone svn://svn.handbrake.fr/HandBrake/trunk hb.git
cd hb.git
# create a working branch
git branch bob
# make the branch your current workspace
git checkout bob
# apply the base bob patch
patch -p0 < hb.decomb-bob.diff
# commit it to your local git repo in the current branch bob
git commit -a -m "base bob patch"
<apply your changes>
git diff

You can also commit your changes so that you can keep history and checkpoints you can roll back to.
Then you can create diffs for any number of local commits.

Code: Select all

# create a diff from the last 2 commits, including any uncommitted changes
git diff HEAD~2
# create a diff for a range of commits
git diff HEAD~3 HEAD~2

If you want to update to the last svn, 'git stash' any current uncommitted changes (or commit them), then:

Code: Select all

git svn rebase
# git will prompt you to fix any merge conflicts

There are lots of other things git will make easier (once you get over the learning curve).

Post patch to http://paste.handbrake.fr/
And provide a link here or in the review board https://reviews.handbrake.fr/r/184/
I would prefer tracking code patches in review board.