- validate frame sync the way the standard suggests (via checking the frame crc) rather than looking at multiple frames. This should reduce the probability of mis-identifying random junk as AC-3 to less than 1 in 2^32.

 - check the crc on every frame so we don't let corrupted data into the decoder.
 - interpret the PTS as per the standard (it's the time of the first frame that starts in the packet, not the time of the first byte of the packet). Incorrect interpretation was resulting in an average 15ms timing error (worse case 31ms).
 - do all the PTS calculations in doubles so we don't get round-off error that will desync the audio & video with 44.1KHz audio sources (these can't appear on DVDs but do show up in avi/mkv/... files).
 - don't rely on the container to give us large enough frame fragments to validate the sync (some containers split audio frames into really small pieces). Instead use the 8K of unused space in the esconfig of the audio object as an accumulation buffer. 


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

2 files changed, 169 insertions, 97 deletions

diff --git a/libhb/deca52.c b/libhb/deca52.c
index 55590242e..f983a6528 100644
--- a/libhb/deca52.c
+++ b/libhb/deca52.c
@@ -7,6 +7,7 @@
 #include "hb.h"
 
 #include "a52dec/a52.h"
+#include "libavutil/crc.h"
 
 struct hb_work_private_s
 {
@@ -19,23 +20,18 @@ struct hb_work_private_s
     int           flags_out;
     int           rate;
     int           bitrate;
+    int           out_discrete_channels;
+    int           error;
+    int           frames;                   // number of good frames decoded
+    int           crc_errors;               // number of frames with crc errors
+    int           bytes_dropped;            // total bytes dropped while resyncing
     float         level;
     float         dynamic_range_compression;
-
-    int           error;
-    int           sync;
-    int           size;
-
-    int64_t       next_expected_pts;
-
-    int64_t       sequence;
-
+    double        next_expected_pts;
+    int64_t       last_buf_pts;
+    hb_list_t    *list;
+    const AVCRC  *crc_table;
     uint8_t       frame[3840];
-
-    hb_list_t   * list;
-
-	int           out_discrete_channels;
-
 };
 
 static int  deca52Init( hb_work_object_t *, hb_job_t * );
@@ -97,6 +93,7 @@ static int deca52Init( hb_work_object_t * w, hb_job_t * job )
 
     pv->job   = job;
 
+    pv->crc_table = av_crc_get_table( AV_CRC_16_ANSI );
     pv->list      = hb_list_init();
     pv->state     = a52_init( 0 );
 
@@ -112,9 +109,6 @@ static int deca52Init( hb_work_object_t * w, hb_job_t * job )
     pv->level     = 32768.0;
     pv->dynamic_range_compression = audio->config.out.dynamic_range_compression;
 
-    pv->next_expected_pts = 0;
-    pv->sequence = 0;
-
     return 0;
 }
 
@@ -126,6 +120,12 @@ static int deca52Init( hb_work_object_t * w, hb_job_t * job )
 static void deca52Close( hb_work_object_t * w )
 {
     hb_work_private_t * pv = w->private_data;
+
+    if ( pv->crc_errors )
+    {
+        hb_log( "deca52: %d frames decoded, %d crc errors, %d bytes dropped",
+                pv->frames, pv->crc_errors, pv->bytes_dropped );
+    }
     a52_free( pv->state );
     hb_list_empty( &pv->list );
     free( pv );
@@ -152,7 +152,12 @@ static int deca52Work( hb_work_object_t * w, hb_buffer_t ** buf_in,
         return HB_WORK_DONE;
     }
 
-    pv->sequence = (*buf_in)->sequence;
+    if ( (*buf_in)->start < -1 && pv->next_expected_pts == 0 )
+    {
+        // discard buffers that start before video time 0
+        *buf_out = NULL;
+        return HB_WORK_OK;
+    }
 
     hb_list_add( pv->list, *buf_in );
     *buf_in = NULL;
@@ -161,7 +166,6 @@ static int deca52Work( hb_work_object_t * w, hb_buffer_t ** buf_in,
     *buf_out = buf = Decode( w );
     while( buf )
     {
-        buf->sequence = pv->sequence;
         buf->next = Decode( w );
         buf       = buf->next;
     }
@@ -180,64 +184,89 @@ static hb_buffer_t * Decode( hb_work_object_t * w )
     hb_buffer_t * buf;
     hb_audio_t  * audio = w->audio;
     int           i, j, k;
-    uint64_t      pts, pos;
+    int           size = 0;
 
-    /* Get a frame header if don't have one yet */
-    if( !pv->sync )
+    // check that we're at the start of a valid frame and align to the
+    // start of a valid frame if we're not.
+    // we have to check the header & crc so we need at least
+    // 7 (the header size) + 128 (the minimum frame size) bytes
+    while( hb_list_bytes( pv->list ) >= 7+128 )
     {
-        while( hb_list_bytes( pv->list ) >= 7 )
+        /* check if this is a valid header */
+        hb_list_seebytes( pv->list, pv->frame, 7 );
+        size = a52_syncinfo( pv->frame, &pv->flags_in, &pv->rate, &pv->bitrate );
+        if ( size > 0 )
         {
-            /* We have 7 bytes, check if this is a correct header */
-            hb_list_seebytes( pv->list, pv->frame, 7 );
-            pv->size = a52_syncinfo( pv->frame, &pv->flags_in, &pv->rate,
-                                    &pv->bitrate );
-            if( pv->size )
+            // header looks valid - check the crc1
+            if( size > hb_list_bytes( pv->list ) )
             {
-                /* It is. W00t. */
+                // don't have all the frame's data yet
+                return NULL;
+            }
+            int crc1size = (size >> 1) + (size >> 3);
+            hb_list_seebytes( pv->list, pv->frame, crc1size );
+            if ( av_crc( pv->crc_table, 0, pv->frame + 2, crc1size - 2 ) == 0 )
+            {
+                // crc1 is ok - say we have valid frame sync
                 if( pv->error )
                 {
-                    hb_log( "a52_syncinfo ok" );
+                    hb_log( "output track %d: ac3 in sync after skipping %d bytes",
+                            audio->config.out.track, pv->error );
+                    pv->bytes_dropped += pv->error;
+                    pv->error = 0;
                 }
-                pv->error = 0;
-                pv->sync  = 1;
                 break;
             }
-
-            /* It is not */
-            if( !pv->error )
-            {
-                hb_log( "a52_syncinfo failed" );
-                pv->error = 1;
-            }
-
-            /* Try one byte later */
-            hb_list_getbytes( pv->list, pv->frame, 1, NULL, NULL );
         }
+        // no sync - discard one byte then try again
+        hb_list_getbytes( pv->list, pv->frame, 1, NULL, NULL );
+        ++pv->error;
     }
 
-    if( !pv->sync ||
-        hb_list_bytes( pv->list ) < pv->size )
+    // we exit the above loop either in error state (we didn't find sync
+    // or don't have enough data yet to validate sync) or in sync. If we're
+    // not in sync we need more data so just return.
+    if( pv->error || size <= 0 || hb_list_bytes( pv->list ) < size )
     {
         /* Need more data */
         return NULL;
     }
 
-    /* Get the whole frame */
-    hb_list_getbytes( pv->list, pv->frame, pv->size, &pts, &pos );
-    if (pts == -1)
+    // Get the whole frame and check its CRC. If the CRC is wrong
+    // discard the frame - we'll resync on the next call.
+
+    uint64_t ipts;
+    hb_list_getbytes( pv->list, pv->frame, size, &ipts, NULL );
+    if ( av_crc( pv->crc_table, 0, pv->frame + 2, size - 2 ) != 0 )
     {
-        pts = pv->next_expected_pts;
+        ++pv->crc_errors;
+        return NULL;
     }
+    ++pv->frames;
+    if ( ipts != pv->last_buf_pts )
+    {
+        pv->last_buf_pts = ipts;
+    }
+    else
+    {
+        // spec says that the PTS is the start time of the first frame
+        // that starts in the PES frame so we only use the PTS once then
+        // get the following frames' PTS from the frame length.
+        ipts = -1;
+    }
+
+    double pts = ( ipts != -1 ) ? ipts : pv->next_expected_pts;
+    double frame_dur = (6. * 256. * 90000.) / pv->rate;
 
     /* AC3 passthrough: don't decode the AC3 frame */
     if( audio->config.out.codec == HB_ACODEC_AC3 )
     {
-        buf = hb_buffer_init( pv->size );
-        memcpy( buf->data, pv->frame, pv->size );
-        buf->start = pts + ( pos / pv->size ) * 6 * 256 * 90000 / pv->rate;
-        buf->stop  = buf->start + 6 * 256 * 90000 / pv->rate;
-        pv->next_expected_pts = buf->stop;
-        pv->sync = 0;
+        buf = hb_buffer_init( size );
+        memcpy( buf->data, pv->frame, size );
+        buf->start = pts;
+        pts += frame_dur;
+        buf->stop  = pts;
+        pv->next_expected_pts = pts;
         return buf;
     }
 
@@ -251,15 +280,10 @@ static hb_buffer_t * Decode( hb_work_object_t * w )
 
     /* 6 blocks per frame, 256 samples per block, channelsused channels */
     buf        = hb_buffer_init( 6 * 256 * pv->out_discrete_channels * sizeof( float ) );
-    buf->start = pts + ( pos / pv->size ) * 6 * 256 * 90000 / pv->rate;
-    buf->stop  = buf->start + 6 * 256 * 90000 / pv->rate;
-
-    /*
-       * To track AC3 PTS add this back in again.
-        *hb_log("AC3: pts is %lld, buf->start %lld buf->stop %lld", pts, buf->start, buf->stop);
-        */
-
-    pv->next_expected_pts = buf->stop;
+    buf->start = pts;
+    pts += frame_dur;
+    buf->stop  = pts;
+    pv->next_expected_pts = pts;
 
     for( i = 0; i < 6; i++ )
     {
@@ -280,56 +304,100 @@ static hb_buffer_t * Decode( hb_work_object_t * w )
         }
 
     }
-
-    pv->sync = 0;
     return buf;
 }
 
-static int deca52BSInfo( hb_work_object_t *w, const hb_buffer_t *b,
-                         hb_work_info_t *info )
+static int find_sync( const uint8_t *buf, int len )
 {
     int i;
-    int rate = 0, bitrate = 0, flags = 0;
-    int old_rate = 0, old_bitrate = 0;
-    uint8_t raw;
 
+    // since AC3 frames don't line up with MPEG ES frames scan the
+    // frame for an AC3 sync pattern.
+    for ( i = 0; i < len - 16; ++i )
+    {
+        int rate, bitrate, flags;
+        int size = a52_syncinfo( (uint8_t *)buf + i, &flags, &rate, &bitrate );
+        if( size > 0 )
+        {
+            // we have a plausible sync header - see if crc1 checks
+            int crc1size = (size >> 1) + (size >> 3); 
+            if ( i + crc1size > len )
+            {
+                // don't have enough data to check crc1
+                break;
+            }
+            if ( av_crc( av_crc_get_table( AV_CRC_16_ANSI ), 0,
+                         buf + i + 2, crc1size - 2 ) == 0 )
+            {
+                // crc checks - we've got sync
+                return i;
+            }
+        }
+    }
+    return -1;
+}
+
+static int deca52BSInfo( hb_work_object_t *w, const hb_buffer_t *b,
+                         hb_work_info_t *info )
+{
     memset( info, 0, sizeof(*info) );
 
-    /* since AC3 frames don't line up with MPEG ES frames scan the
-     * entire frame for an AC3 sync pattern.  */
-    for ( i = 0; i < b->size - 7; ++i )
+    // We don't know if the way that AC3 frames are fragmented into whatever
+    // packetization the container uses will give us enough bytes per fragment
+    // to check the CRC (we need at least 5/8 of the the frame). So we
+    // copy the fragment we got into an accumulation buffer in the audio object
+    // then look for sync over all the frags we've accumulated so far.
+    uint8_t *buf = w->audio->priv.config.a52.buf;
+    int len = w->audio->priv.config.a52.len, blen = b->size;
+    if ( len + blen > sizeof(w->audio->priv.config.a52.buf) )
     {
-        if( a52_syncinfo( &b->data[i], &flags, &rate, &bitrate ) != 0 )
+        // we don't have enough empty space in the accumulation buffer to
+        // hold the new frag - make room for it by discarding the oldest data.
+        if ( blen >= sizeof(w->audio->priv.config.a52.buf) )
         {
-            /*
-             * Got sync apparently, save these values and check that they
-             * also match when we get sync again.
-             */
-            if( old_rate ) 
-            {
-                if( rate == old_rate && bitrate == old_bitrate )
-                {
-                    break;
-                } 
-            } 
-            
-            old_rate = rate;
-            old_bitrate = bitrate;
-            raw = b->data[i+5];
+            // the frag is bigger than our accumulation buffer - copy all
+            // that will fit (the excess doesn't matter since the buffer
+            // is many times the size of a max length ac3 frame).
+            blen = sizeof(w->audio->priv.config.a52.buf);
+            len = 0;
+        }
+        else
+        {
+            // discard enough bytes from the front of the buffer to make
+            // room for the new stuff
+            int newlen = sizeof(w->audio->priv.config.a52.buf) - blen;
+            memcpy( buf, buf + len - newlen, newlen );
+            len = newlen;
         }
     }
-    if ( rate == 0 || bitrate == 0 )
+    // add the new frag to the buffer
+    memcpy( buf+len, b->data, blen );
+    len += blen;
+
+    int i;
+    if ( ( i = find_sync( buf, len ) ) < 0 )
     {
-        /* didn't find AC3 sync */
+        // didn't find sync - wait for more data
+        w->audio->priv.config.a52.len = len;
         return 0;
     }
 
-    /*
-     * bsid | bsmod | acmod | cmixlev | surmixlev | dsurmod | lfeon | dialnorm | compre
-     *    5       3       3         2           2         2       1          5        1
-     * [    byte1  ][         byte2                    ][   byte3                     ]
-     */
+    // got sync - extract and canoncalize the bitstream parameters
+    int rate = 0, bitrate = 0, flags = 0;
+    uint8_t raw = buf[i + 5];
+    a52_syncinfo( buf + i, &flags, &rate, &bitrate );
+
+    if ( rate == 0 || bitrate == 0 )
+    {
+        // invalid AC-3 parameters - toss what we have so we'll start over
+        // with the next buf otherwise we'll keep syncing on this junk.
+        w->audio->priv.config.a52.len = 0;
+        return 0;
+    }
 
+    // bsid | bsmod | acmod | cmixlv | surmixlv | dsurmod | lfeon | dialnorm | compre
+    //   5       3      3        2         2         2        1          5        1
+    //      byte1   |          byte2                 |    byte3  
 
     info->name = "AC-3";
     info->rate = rate;
diff --git a/libhb/internal.h b/libhb/internal.h
index 29a88dca6..19a3bd2b3 100644
--- a/libhb/internal.h
+++ b/libhb/internal.h
@@ -233,7 +233,11 @@ union hb_esconfig_u
     struct
     {
     	/* ac3flags stores the flags from the AC3 source, as found in scan.c */
-    	int  ac3flags;
+    	int     ac3flags;
+        // next two items are used by the bsinfo routine to accumulate small
+        // frames until we have enough to validate the crc.
+        int     len;        // space currently used in 'buf'
+        uint8_t buf[HB_CONFIG_MAX_SIZE-sizeof(int)];
     } a52;
 
     struct