Page 1 of 1

[PATCH] deinterlace & chapters enabled adds bogus chapte

Posted: Wed Aug 29, 2007 8:08 am
by van
Using any of the good deinterlacers ("slow/slower/...") with chapter marks enabled can output double the number of chapter marks present in the source material with the bogus extras appearing slightly before the real chapter break. The problem is that hb_deinterlace_work can internally buffer frames. It correctly manages the buf_out new_chap flag so the flag is on the right buffer when sent upstream to the encoder but, unfortunately, work_loop doesn't know that frames are being delayed and it always blindly copies any non-zero new_chap from buf_in to buf_out. Since the buf_out returned by hb_deinterlace_work is usually an earlier delayed frame you end up with an extra chapter mark slightly ahead of the real one. [detelecine also delays frames and can potentially have the same problem but I haven't tested it.]

A quick fix (attached) is to make hb_deinterlace_work clear buf_in->new_chap when it delays frames. A better fix might be to remove the lines in work_loop that copy new_chap from buf_in to buf_out and instead explicitly copy new_chap from buf_in to buf_out in all the video encoder modules (enc{x264,avcodec,xvid}Work). Since encx264Work in particular can also delay frames (when using bidirectional b frames) this would prevent it adding bogus chapter marks if quicktime ever evolves to the point that we're routinely using bidirectional B's.

Code: Select all

Index: libhb/deinterlace.c
===================================================================
--- libhb/deinterlace.c (revision 889)
+++ libhb/deinterlace.c (working copy)
@@ -65,7 +65,7 @@
                                            int height,
                                            char * settings );
 
-int hb_deinterlace_work( const hb_buffer_t * buf_in,
+int hb_deinterlace_work( hb_buffer_t * buf_in,
                          hb_buffer_t ** buf_out,
                          int pix_fmt,
                          int width, 
@@ -497,7 +497,7 @@
     free( pv );
 }
 
-int hb_deinterlace_work( const hb_buffer_t * buf_in,
+int hb_deinterlace_work( hb_buffer_t * buf_in,
                          hb_buffer_t ** buf_out,
                          int pix_fmt,
                          int width, 
@@ -552,6 +552,9 @@
         
         hb_buffer_copy_settings( pv->buf_settings, buf_in );
 
+       /* don't let 'work_loop' send a chapter mark upstream */
+        buf_in->new_chap  = 0;
+
         pv->yadif_ready = 1;
         
         return FILTER_DELAY;
@@ -589,6 +592,9 @@
     /* 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;
+
     return FILTER_OK;
 }

Tested

Posted: Fri Aug 31, 2007 1:06 am
by eddyg
Hi van,

I can confirm that the bug you reported exists, and I can confirm that your workaround does indeed work.

Thanks for working on this issue.

Cheers, Ed.

Posted: Wed Sep 26, 2007 8:46 am
by alehel
I really hope there is a new version of handbrake released soon so that people like me who have no idea how to compile this stuff can get proper chapter support. This really is a show stopper for me as I want to be able to watch movies on my iPod touch and also navigate them the way they were supposed to, but the normal deinterlace makes Rocky Balboa (my favourite movie) get a whole bunch of jagged lines all the time.

I won't actually be recieving my iPod touch for about two weeks. I live in Norway and Apple isn't sending us their first shipment of iPod touch untill Friday. So hopefully there won't be any bad screens making their ways to Norway and hopefully a new version of handbrake will be out then.

Posted: Wed Sep 26, 2007 10:18 am
by hawkman
alehel wrote:Rocky Balboa (my favourite movie) get a whole bunch of jagged lines all the time.
Does this movie really need deinterlacing?

Posted: Wed Sep 26, 2007 10:39 am
by eddyg
alehel wrote:I really hope there is a new version of handbrake released soon so that people like me who have no idea how to compile this stuff can get proper chapter support. This really is a show stopper for me as I want to be able to watch movies on my iPod touch and also navigate them the way they were supposed to, but the normal deinterlace makes Rocky Balboa (my favourite movie) get a whole bunch of jagged lines all the time.

I won't actually be recieving my iPod touch for about two weeks. I live in Norway and Apple isn't sending us their first shipment of iPod touch untill Friday. So hopefully there won't be any bad screens making their ways to Norway and hopefully a new version of handbrake will be out then.
Note that the fix only shifts the chapter starts by a second or so, it's not exactly a show stopper.

As for the deinterlace, if you are seeing jagged lines, then something is wrong, because the existing deinterlace works fine, albeit by losing half the resolution.

Cheers, Ed.

Posted: Wed Sep 26, 2007 2:24 pm
by alehel
The fact that I get two chapter markers for every chapter is a bit anoying. It's not just the fact that they aren't placed exactly.

It's not just Rocky Balboa I've been ripping, so it is possible that it doesn't need it. I havn't checked it specifically for this.

Incidently, the handbrake guide says I should expect jagged lines using the old deinterlace version.
HandBrake's traditional deinterlacer, "Fast" uses something called line doubling. First, it discards half the fields. This creates a half-height image. Then, to return to the full vertical resolution, it doubles the remaining fields. The result looks just plain nasty, so be sure to only deinterlace using "Fast" when you have to. Sometimes people will say it's easier to just always enable it...that it's too much trouble to check each video for interlacing. But when you run the "Fast" deinterlacer, the result will be stair-stepping: jagged lines along any diagonals or curves.

Posted: Wed Sep 26, 2007 3:03 pm
by hawkman
I would be immensely surprised if any vaguely modern movie required deinterlacing. The passage you quoted warns against exactly what you seem to be doing - always deinterlacing, no matter what the source - and so also contains the solution to your problem.

If there is interlaced material you need to encode, use the Fast setting, disable chapter markers or wait for 0.9.1.

Posted: Wed Sep 26, 2007 3:13 pm
by dynaflash
alehel wrote:It's not just Rocky Balboa I've been ripping, so it is possible that it doesn't need it. I havn't checked it specifically for this.
Rocky Balboa is not interlaced, therefore does not need deinterlacing.

Posted: Wed Sep 26, 2007 7:53 pm
by alehel
I feel like an idiot now. Thanks for correcting me.