- get rid of 'chapter merging'.

- check for 'beginning of cell' before 'end of cell' so zero length cells don't
  make us advance to the next cell too early & fail to generate a chapter mark.
- don't read beyond the last cell of a title.
- if we get a read error just skip to the next vobu instead of giving up.
- fix the next_vobu mask (the top two bits have special meaning, not just
  the MSB), make the next vobu decision logic a bit closer to what a hardware
  dvd player uses (interpret the 2^30 bit as "look for a better vobu pointer
  in this navpack), make the 'end of vobu chain' recognition more robust.
- when we're searching for a navpack because we couldn't read the block
  'next_vobu' points to, recognize when we walk out of the current cell &
  just go to the next cell for this title.
- treat an internal vobu with a null prev_vobu pointer as if it were end-of-cell
  and advance to the next cell.
- use consistent case in log messages (always "dvd:" rather than a random mix
  of upper & lower case).


git-svn-id: svn://svn.handbrake.fr/HandBrake/trunk@1215 b64f7644-9d1e-0410-96f1-a4d463321fa5

1 files changed, 143 insertions, 110 deletions

diff --git a/libhb/dvd.c b/libhb/dvd.c
index 94f020fca..b181402ec 100644
--- a/libhb/dvd.c
+++ b/libhb/dvd.c
@@ -36,6 +36,8 @@ struct hb_dvd_s
     int            next_vobu;
     int            in_cell;
     int            in_sync;
+    uint16_t       cur_vob_id;
+    uint8_t        cur_cell_id;
 };
 
 /***********************************************************************
@@ -201,14 +203,6 @@ hb_title_t * hb_dvd_title_scan( hb_dvd_t * d, int t )
             title->cell_end, title->block_start, title->block_end,
             title->block_count );
 
-    if( title->block_count < 2048  )
-    {
-        hb_log( "scan: title too short (%d blocks), ignoring",
-                title->block_count );
-        goto fail;
-    }
-
-
     /* Get duration */
     title->duration = 90LL * dvdtime2msec( &d->pgc->playback_time );
     title->hours    = title->duration / 90000 / 3600;
@@ -218,8 +212,10 @@ hb_title_t * hb_dvd_title_scan( hb_dvd_t * d, int t )
             title->hours, title->minutes, title->seconds,
             title->duration / 90 );
 
-    /* Discard titles under 10 seconds */
-    if( !( title->hours | title->minutes ) && title->seconds < 10 )
+    /* ignore titles under 10 seconds because they're often stills or
+     * clips with no audio & our preview code doesn't currently handle
+     * either of these. */
+    if( title->duration < 900000LL )
     {
         hb_log( "scan: ignoring title (too short)" );
         goto fail;
@@ -448,26 +444,8 @@ hb_title_t * hb_dvd_title_scan( hb_dvd_t * d, int t )
             FindNextCell( d );
             d->cell_cur = d->cell_next;
         }
-
-        if( chapter->block_count < 2048 && c > 1 )
-        {
-            hb_log( "scan: chapter %d(%d) too short (%d blocks, "
-                    "cells=%d->%d), merging", c, chapter->index,
-                    chapter->block_count, chapter->cell_start,
-                    chapter->cell_end );
-            chapter_old = hb_list_item( title->list_chapter, c - 2 );
-            chapter_old->cell_end    = chapter->cell_end;
-            chapter_old->block_end   = chapter->block_end;
-            chapter_old->block_count += chapter->block_count;
-            chapter_old->duration += chapter->duration;
-            free( chapter );
-            chapter = chapter_old;
-        }
-        else
-        {
-            hb_list_add( title->list_chapter, chapter );
-            c++;
-        }
+        hb_list_add( title->list_chapter, chapter );
+        c++;
     }
 
     /* The durations we get for chapters aren't precise. Scale them so
@@ -626,6 +604,7 @@ int hb_dvd_seek( hb_dvd_t * d, float f )
         if( count < sizeCell )
         {
             d->cell_cur = i;
+            d->cur_cell_id = 0;
             FindNextCell( d );
 
             /* Now let hb_dvd_read find the next VOBU */
@@ -702,13 +681,19 @@ int is_nav_pack( unsigned char *buf )
  **********************************************************************/
 int hb_dvd_read( hb_dvd_t * d, hb_buffer_t * b )
 {
+ top:
     if( !d->pack_len )
     {
         /* New pack */
         dsi_t dsi_pack;
-        int   error;
+        int   error = 0;
 
-        error = 0;
+        // if we've just finished the last cell of the title we don't
+        // want to read another block because our next_vobu pointer
+        // is probably invalid. Just return 'no data' & our caller
+        // should check and discover we're at eof.
+        if ( d->cell_cur > d->cell_end )
+            return 0;
         
         for( ;; )
         {
@@ -730,14 +715,14 @@ int hb_dvd_read( hb_dvd_t * d, hb_buffer_t * b )
 
             if( read_retry == 3 )
             {
-                hb_log( "dvd: Unrecoverable Read Error from DVD, potential HD or DVD Failure (blk: %d)", d->next_vobu );
+                hb_log( "dvd: Unrecoverable Read Error from DVD (blk: %d)", d->next_vobu );
                 return 0;
             }
 
             if ( !is_nav_pack( b->data ) ) { 
                 (d->next_vobu)++;
                 if( d->in_sync == 1 ) {
-                    hb_log("DVD: Lost sync, searching for NAV pack at blk %d", 
+                    hb_log("dvd: Lost sync, searching for NAV pack at blk %d", 
                            d->next_vobu);
                     d->in_sync = 0;
                 }
@@ -746,9 +731,67 @@ int hb_dvd_read( hb_dvd_t * d, hb_buffer_t * b )
 
             navRead_DSI( &dsi_pack, &b->data[DSI_START_BYTE] );
             
+            if ( d->in_sync == 0 && d->cur_cell_id &&
+                 (d->cur_vob_id != dsi_pack.dsi_gi.vobu_vob_idn ||
+                  d->cur_cell_id != dsi_pack.dsi_gi.vobu_c_idn ) )
+            {
+                // We walked out of the cell we're supposed to be in.
+                // If we're not at the start of our next cell go there.
+                hb_log("dvd: left cell %d (%u,%u) for (%u,%u) at block %u",
+                       d->cell_cur, d->cur_vob_id, d->cur_cell_id,
+                       dsi_pack.dsi_gi.vobu_vob_idn, dsi_pack.dsi_gi.vobu_c_idn,
+                       d->next_vobu );
+                if ( d->next_vobu != d->pgc->cell_playback[d->cell_next].first_sector )
+                {
+                    d->next_vobu = d->pgc->cell_playback[d->cell_next].first_sector;
+                    d->cur_cell_id = 0;
+                    continue;
+                }
+            }
+
             block     = dsi_pack.dsi_gi.nv_pck_lbn;
             pack_len  = dsi_pack.dsi_gi.vobu_ea;
-            next_vobu = block + ( dsi_pack.vobu_sri.next_vobu & 0x7fffffff );
+
+            // There are a total of 21 next vobu offsets (and 21 prev_vobu
+            // offsets) in the navpack SRI structure. The primary one is
+            // 'next_vobu' which is the offset (in dvd blocks) from the current
+            // block to the start of the next vobu. If the block at 'next_vobu'
+            // can't be read, 'next_video' is the offset to the vobu closest to it.
+            // The other 19 offsets are vobus at successively longer distances from
+            // the current block (this is so that a hardware player can do
+            // adaptive error recovery to skip over a bad spot on the disk). In all
+            // these offsets the high bit is set to indicate when it contains a
+            // valid offset. The next bit (2^30) is set to indicate that there's
+            // another valid offset in the SRI that's closer to the current block.
+            // A hardware player uses these bits to chose the closest valid offset
+            // and uses that as its next vobu. (Some mastering schemes appear to
+            // put a bogus offset in next_vobu with the 2^30 bit set & the
+            // correct offset in next_video. This works fine in hardware players
+            // but will mess up software that doesn't implement the full next
+            // vobu decision logic.) In addition to the flag bits there's a
+            // reserved value of the offset that indicates 'no next vobu' (which
+            // indicates the end of a cell). But having no next vobu pointer with a
+            // 'valid' bit set also indicates end of cell. Different mastering
+            // schemes seem to use all possible combinations of the flag bits
+            // and reserved values to indicate end of cell so we have to check
+            // them all or we'll get a disk read error from following an
+            // invalid offset.
+            uint32_t next_ptr = dsi_pack.vobu_sri.next_vobu;
+            if ( ( next_ptr & ( 1 << 31 ) ) == 0  ||
+                 ( next_ptr & ( 1 << 30 ) ) != 0 ||
+                 ( next_ptr & 0x3fffffff ) == 0x3fffffff )
+            {
+                next_ptr = dsi_pack.vobu_sri.next_video;
+                if ( ( next_ptr & ( 1 << 31 ) ) == 0 ||
+                     ( next_ptr & 0x3fffffff ) == 0x3fffffff )
+                {
+                    // don't have a valid forward pointer - assume end-of-cell
+                    d->block     = block;
+                    d->pack_len  = pack_len;
+                    break;
+                }
+            }
+            next_vobu = block + ( next_ptr & 0x3fffffff );
 
             if( pack_len >  0    &&
                 pack_len <  1024 &&
@@ -756,17 +799,6 @@ int hb_dvd_read( hb_dvd_t * d, hb_buffer_t * b )
                 ( block <= d->title_start + d->title_block_count ||
                   block <= d->title_end ) )
             {
-                /* XXX
-                   This looks like a valid VOBU, but actually we are
-                   just hoping */
-                if( error )
-                {
-#if 0
-                    hb_log( "dvd: found VOBU at %d (b %d, l %d, n %d)",
-                            d->next_vobu, block, pack_len, next_vobu );
-#endif
-                }
-
                 d->block     = block;
                 d->pack_len  = pack_len;
                 d->next_vobu = next_vobu;
@@ -774,13 +806,6 @@ int hb_dvd_read( hb_dvd_t * d, hb_buffer_t * b )
             }
 
             /* Wasn't a valid VOBU, try next block */
-            if( !error )
-            {
-#if 0
-                hb_log( "dvd: looking for a VOBU (%d)", d->next_vobu );
-#endif
-            }
-
             if( ++error > 1024 )
             {
                 hb_log( "dvd: couldn't find a VOBU after 1024 blocks" );
@@ -794,53 +819,88 @@ int hb_dvd_read( hb_dvd_t * d, hb_buffer_t * b )
         {
             if( d->in_sync == 0 )
             {
-                hb_log( "DVD: In sync with DVD at block %d", d->block );
+                hb_log( "dvd: In sync with DVD at block %d", d->block );
             }
             d->in_sync = 1;
         }
 
-        if( dsi_pack.vobu_sri.next_vobu == SRI_END_OF_CELL )
-        { 
-            hb_log( "DVD: End of Cell (%d) at block %d", d->cell_cur, 
-                    d->block );
-            d->cell_cur  = d->cell_next;
-            d->in_cell = 0;
-            d->next_vobu = d->pgc->cell_playback[d->cell_cur].first_sector;
-            FindNextCell( d );
-            d->cell_overlap = 1;
-            
-        }
-
         // Revert the cell overlap, and check for a chapter break
-        if( dsi_pack.vobu_sri.prev_vobu == SRI_END_OF_CELL )
+        // If this cell is zero length (prev_vobu & next_vobu both
+        // set to END_OF_CELL) we need to check for beginning of
+        // cell before checking for end or we'll advance to the next
+        // cell too early and fail to generate a chapter mark when this
+        // cell starts a chapter.
+        if( ( dsi_pack.vobu_sri.prev_vobu & (1 << 31 ) ) == 0 ||
+            ( dsi_pack.vobu_sri.prev_vobu & 0x3fffffff ) == 0x3fffffff )
         {
-            hb_log( "DVD: Beginning of Cell (%d) at block %d", d->cell_cur, 
-                   d->block );
-            if( d->in_cell )
+            // A vobu that's not at the start of a cell can have an
+            // EOC prev pointer (this seems to be associated with some
+            // sort of drm). The rest of the content in the cell may be
+            // booby-trapped so treat this like an end-of-cell rather
+            // than a beginning of cell.
+            if ( d->pgc->cell_playback[d->cell_cur].first_sector < dsi_pack.dsi_gi.nv_pck_lbn &&
+                 d->pgc->cell_playback[d->cell_cur].last_sector >= dsi_pack.dsi_gi.nv_pck_lbn )
             {
-                hb_error( "DVD: Assuming missed End of Cell (%d)", d->cell_cur );
+                hb_log( "dvd: null prev_vobu in cell %d at block %d", d->cell_cur, 
+                        d->block );
+                // treat like end-of-cell then go directly to start of next cell.
                 d->cell_cur  = d->cell_next;
+                d->in_cell = 0;
                 d->next_vobu = d->pgc->cell_playback[d->cell_cur].first_sector;
                 FindNextCell( d );
                 d->cell_overlap = 1;
-                d->in_cell = 0;
-            } else {
-                d->in_cell = 1;
+                goto top;
             }
-
-            if( d->cell_overlap )
+            else
             {
-                b->new_chap = hb_dvd_is_break( d );
-                d->cell_overlap = 0;
+                hb_log( "dvd: Beginning of Cell (%d) at block %d", d->cell_cur, 
+                       d->block );
+                if( d->in_cell )
+                {
+                    hb_error( "dvd: Assuming missed End of Cell (%d)", d->cell_cur );
+                    d->cell_cur  = d->cell_next;
+                    d->next_vobu = d->pgc->cell_playback[d->cell_cur].first_sector;
+                    FindNextCell( d );
+                    d->cell_overlap = 1;
+                    d->in_cell = 0;
+                } else {
+                    d->in_cell = 1;
+                }
+                d->cur_vob_id = dsi_pack.dsi_gi.vobu_vob_idn;
+                d->cur_cell_id = dsi_pack.dsi_gi.vobu_c_idn;
+
+                if( d->cell_overlap )
+                {
+                    b->new_chap = hb_dvd_is_break( d );
+                    d->cell_overlap = 0;
+                }
             }
         }
+
+        if( ( dsi_pack.vobu_sri.next_vobu & (1 << 31 ) ) == 0 ||
+            ( dsi_pack.vobu_sri.next_vobu & 0x3fffffff ) == 0x3fffffff )
+        { 
+            hb_log( "dvd: End of Cell (%d) at block %d", d->cell_cur, 
+                    d->block );
+            d->cell_cur  = d->cell_next;
+            d->in_cell = 0;
+            d->next_vobu = d->pgc->cell_playback[d->cell_cur].first_sector;
+            FindNextCell( d );
+            d->cell_overlap = 1;
+            
+        }
     }
     else
     {
         if( DVDReadBlocks( d->file, d->block, 1, b->data ) != 1 )
         {
-            hb_error( "reader: DVDReadBlocks failed (%d)", d->block );
-            return 0;
+            // this may be a real DVD error or may be DRM. Either way
+            // we don't want to quit because of one bad block so set
+            // things up so we'll advance to the next vobu and recurse.
+            hb_error( "dvd: DVDReadBlocks failed (%d), skipping to vobu %u",
+                      d->block, d->next_vobu );
+            d->pack_len = 0;
+            goto top;  /* XXX need to restructure this routine & avoid goto */
         }
         d->pack_len--;
     }
@@ -913,35 +973,8 @@ int hb_dvd_is_break( hb_dvd_t * d )
         // This must not match against the start cell.
         if( pgc->cell_playback[cell].first_sector == d->block && cell != d->cell_start )
         {
-            /* Check to see if we merged this chapter into the previous one... */
-            /* As a note, merging chapters is probably bad practice for this very reason */
-            chapter_length = 0;
-            
-            if( i == nr_of_ptts - 1 )
-            {
-                cell_end = d->pgc->nr_of_cells;
-            }
-            else
-            {
-                cell_end = pgc->program_map[pgn] - 1;
-            }
-            
-            for( j = cell; j < cell_end; j++ )
-            {
-                chapter_length += pgc->cell_playback[j].last_sector + 1 - 
-                                  pgc->cell_playback[j].first_sector;
-            }
-            
-            if( chapter_length >= 2048 )
-            {
-                hb_log("DVD: Chapter Break Cell Found");
-                /* We have a chapter break */
-                return 1;
-            }
-            else
-            {
-                hb_log("DVD: Cell Found (len=%d)", chapter_length);
-            }
+            hb_log("dvd: Chapter Break Cell Found");
+            return 1;
         }
     }
     
@@ -990,7 +1023,7 @@ static void FindNextCell( hb_dvd_t * d )
              i++;
         }
         d->cell_next = d->cell_cur + i + 1;
-        hb_log( "DVD: Skipping multi-angle cells %d-%d", 
+        hb_log( "dvd: Skipping multi-angle cells %d-%d", 
                 d->cell_cur, 
                 d->cell_next - 1 );
     }