[PATCH] [libhb] Handle bad blocks with dvdread

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
eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

[PATCH] [libhb] Handle bad blocks with dvdread

Post by eddyg »

Hi,

I encountered a DVD that had a load of bad blocks near the start of the first chapter. Handbrake using dvdnav or dvdread fails to handle this gracefully and skips to the start of the next cell as a recovery, which skipped the majority of the first chapter in my case.

In this patch I have change the dvdread bad block recovery to scan forwards through the blocks whenever a read failure is encountered.

I also changed the number of consecutive failures in dvdnav (from 10 to 500) so that it at least skipped forwards when encountering bad blocks rather than aborting the encode.

Note that these changes work when reading direct from a DVD, however they didn't work with a pre-ripped copy of the same DVD since MTR bad a mess of the bad blocks, it wrote bad blocks write to the end of the cell.

http://paste.handbrake.fr/pastebin.php?show=1905

Code: Select all

Index: dvdnav.c
===================================================================
--- dvdnav.c	(revision 3688)
+++ dvdnav.c	(working copy)
@@ -1503,7 +1503,7 @@
                 return 0;
             }
             error_count++;
-            if (error_count > 10)
+            if (error_count > 500)
             {
                 hb_error("dvdnav: Error, too many consecutive read errors");
                 return 0;
Index: dvd.c
===================================================================
--- dvd.c	(revision 3688)
+++ dvd.c	(working copy)
@@ -861,22 +861,37 @@
         {
             int block, pack_len, next_vobu, read_retry;
 
-            for( read_retry = 0; read_retry < 3; read_retry++ )
+            for( read_retry = 1; read_retry < 1024; read_retry++ )
             {
                 if( DVDReadBlocks( d->file, d->next_vobu, 1, b->data ) == 1 )
                 {
                     /*
                      * Successful read.
                      */
+                    if( read_retry > 1 && !is_nav_pack( b->data) )
+                    {
+                        // But wasn't a nav pack, so carry on looking
+                        read_retry = 1;
+                        d->next_vobu++;
+                        continue;
+                    }
                     break;
-                } else {
-                    hb_log( "dvd: Read Error on blk %d, attempt %d",
-                            d->next_vobu, read_retry );
+                } else {  
+                    // First retry the same block, then try the next one, 
+                    // adjust the skip increment upwards so that we can skip
+                    // large sections of bad blocks more efficiently (at the
+                    // cost of some missed good blocks at the end).
+                    hb_log( "dvd: vobu read error blk %d - skipping to next blk incr %d",
+                            d->next_vobu, (read_retry * 10));
+                    d->next_vobu += (read_retry * 10);
                 }
+                
             }
 
-            if( read_retry == 3 )
+            if( read_retry == 1024 )
             {
+                // That's too many bad blocks, jump to the start of the
+                // next cell.
                 hb_log( "dvd: vobu read error blk %d - skipping to cell %d",
                         d->next_vobu, d->cell_next );
                 d->cell_cur  = d->cell_next;
@@ -895,7 +910,7 @@
                     hb_log("dvd: Lost sync, searching for NAV pack at blk %d",
                            d->next_vobu);
                     d->in_sync = 0;
-                }
+                } 
                 continue;
             }
 
@@ -1030,7 +1045,7 @@
                 }
                 if( d->in_cell )
                 {
-                    hb_error( "dvd: assuming missed end of cell %d", d->cell_cur );
+                    hb_error( "dvd: assuming missed end of cell %d at block %d", d->cell_cur, d->block );
                     d->cell_cur  = d->cell_next;
                     d->next_vobu = d->pgc->cell_playback[d->cell_cur].first_sector;
                     FindNextCell( d );
Cheers, Ed.

Edit. I noticed that the comment is now incorrect regarding retrying the same block, I decided against that since it slows the whole process down, and anyway I'm sure the device driver is already doing retries when encountering issues.
noved
Novice
Posts: 73
Joined: Mon Feb 26, 2007 9:24 pm

Re: [PATCH] [libhb] Handle bad blocks with dvdread

Post by noved »

What was the outcome of this code tweak? Not in a past or current nightlies? The ability to fine tune the number of retries HB does when encountering bad blocks on a phsyical DVD could be really handy. Is there any prefs property that exists or be created to set error retry count? As in being able to do something like...

> defaults write org.m0k.handbrake dvdReadRetryMax 500

I've encountered several DVDs where the Apple DVD Player displays 'Skipping over bad sectors' but HB skips to the end of the entire chapter missing a lot of content. If Apple's DVD Player can skip over it and keep going, could they just be telling the device driver to retry more than HB does?
eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Re: [PATCH] [libhb] Handle bad blocks with dvdread

Post by eddyg »

I don't think I committed it, too close to the last release. And then forgot about it since it was in my builds anyway. You could try it on your troublesome disc and if it works I coul look at committing it.

I wouldn't want to commit it now without retesting against a bad disk, and I don't recall which disk it was.

Cheers Ed.
Deleted User 11865

Re: [PATCH] [libhb] Handle bad blocks with dvdread

Post by Deleted User 11865 »

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

Re: [PATCH] [libhb] Handle bad blocks with dvdread

Post by eddyg »

Golly gosh, I did commit it. 11 Months ago, which is many hundreds of beers ago, may explain it.

The bad thing is that it didn't fix your problem. The retry is not a simple retry, after one retry it moves forward trying to sync up with the disk again. No tuning knob was provided, since it worked on my disc as is.
noved
Novice
Posts: 73
Joined: Mon Feb 26, 2007 9:24 pm

Re: [PATCH] [libhb] Handle bad blocks with dvdread

Post by noved »

ok, cool. if I understand the changeset, using a current nightly, I should make sure I'm using libdvdread instead of libdvdnav to see if the former will possibly be able to continue with the current chapter encoding on the disks I've encountered bad sectors on. From your [eddyg] checkin comments, it sounds like libdvdnav doesn't give as much control over this kind of I/O handling.

thanx!
Deleted User 11865

Re: [PATCH] [libhb] Handle bad blocks with dvdread

Post by Deleted User 11865 »

SVN 3695 < SVN 3735 (HB 0.9.5), FWIW.
noved
Novice
Posts: 73
Joined: Mon Feb 26, 2007 9:24 pm

Re: [PATCH] [libhb] Handle bad blocks with dvdread

Post by noved »

here's a good news update...

using a current HB nightly (r4326), with libdvdread in use, I was able to successfully encode two DVDs with known physical bad spots on them. Previous to using libdvdread, HB would skip to the next chapter when bad sectors were encountered; which of course is the expected behavior with libdvdnav in use. HB *with* libdvdread did a great job of encoding over the troubled spots on both DVDs. In playback of one of the .m4v files, there's a very slight visual artifact-looking glitch (no audio prob at all) at the point in the scene where I know the bad spots were. Very slight as in damn close to the slight hiccup Apple's DVD Player has playing over the bad spot with the same physical DVD. Playback of the 2nd encoded .m4v file has no noticeable hiccup at all where the bad spot was on that disc. sweet!!

thanks you two!
Post Reply