[PATCH] deinterlace & chapters enabled adds bogus chapte

Archive of historical development discussions
Discussions / Development has moved to GitHub
Forum rules
*******************************
Please be aware we are now using GitHub for issue tracking and feature requests.
- This section of the forum is now closed to new topics.

*******************************
Post Reply
van
Veteran User
Posts: 417
Joined: Wed Aug 29, 2007 6:35 am

[PATCH] deinterlace & chapters enabled adds bogus chapte

Post by van » Wed Aug 29, 2007 8:08 am

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;
 }

eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Tested

Post by eddyg » Fri Aug 31, 2007 1:06 am

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.

alehel
Regular User
Posts: 83
Joined: Wed Feb 14, 2007 11:08 am

Post by alehel » Wed Sep 26, 2007 8:46 am

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.

hawkman
Veteran User
Posts: 609
Joined: Sat Feb 17, 2007 9:46 pm

Post by hawkman » Wed Sep 26, 2007 10:18 am

alehel wrote:Rocky Balboa (my favourite movie) get a whole bunch of jagged lines all the time.
Does this movie really need deinterlacing?

eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Post by eddyg » Wed Sep 26, 2007 10:39 am

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.

alehel
Regular User
Posts: 83
Joined: Wed Feb 14, 2007 11:08 am

Post by alehel » Wed Sep 26, 2007 2:24 pm

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.

hawkman
Veteran User
Posts: 609
Joined: Sat Feb 17, 2007 9:46 pm

Post by hawkman » Wed Sep 26, 2007 3:03 pm

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.

dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash » Wed Sep 26, 2007 3:13 pm

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.

alehel
Regular User
Posts: 83
Joined: Wed Feb 14, 2007 11:08 am

Post by alehel » Wed Sep 26, 2007 7:53 pm

I feel like an idiot now. Thanks for correcting me.

Post Reply