encoders: save chapter markers in a list.

If chapters were a few seconds apart or less, we would encounter a new marker before the previous one had been placed in an output buffer. 


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

2 files changed, 121 insertions, 25 deletions

diff --git a/libhb/enc_qsv.c b/libhb/enc_qsv.c
index 90bb26a57..ca2158a67 100644
--- a/libhb/enc_qsv.c
+++ b/libhb/enc_qsv.c
@@ -58,11 +58,14 @@ struct hb_work_private_s
     av_qsv_space enc_space;
 
     mfxEncodeCtrl force_keyframe;
-    struct
+    hb_list_t *delayed_chapters;
+    int64_t next_chapter_pts;
+    // used in delayed_chapters list
+    struct chapter_s
     {
         int     index;
         int64_t start;
-    } next_chapter;
+    };
 
 #define BFRM_DELAY_MAX 16
     // for DTS generation (when MSDK API < 1.6 or VFR)
@@ -372,8 +375,8 @@ int encqsvInit(hb_work_object_t *w, hb_job_t *job)
     pv->force_keyframe.ExtParam    = NULL;
     pv->force_keyframe.Payload     = NULL;
 
-    pv->next_chapter.index = 0;
-    pv->next_chapter.start = INT64_MIN;
+    pv->next_chapter_pts = AV_NOPTS_VALUE;
+    pv->delayed_chapters = hb_list_init();
 
     // default encoding parameters
     if (hb_qsv_param_default_preset(&pv->param, &pv->enc_space.m_mfxVideoParam,
@@ -1154,6 +1157,16 @@ void encqsvClose( hb_work_object_t * w )
             }
             hb_list_close(&pv->list_dts);
         }
+        if (pv->delayed_chapters != NULL)
+        {
+            struct chapter_s *item;
+            while ((item = hb_list_item(pv->delayed_chapters, 0)) != NULL)
+            {
+                hb_list_rem(pv->delayed_chapters, item);
+                free(item);
+            }
+            hb_list_close(&pv->delayed_chapters);
+        }
     }
 
     free( pv );
@@ -1273,19 +1286,28 @@ int encqsvWork( hb_work_object_t * w, hb_buffer_t ** buf_in,
              */
             if (in->s.new_chap > 0 && job->chapter_markers)
             {
-                if (!pv->next_chapter.index)
+                if (pv->next_chapter_pts == AV_NOPTS_VALUE)
                 {
-                    pv->next_chapter.start = work_surface->Data.TimeStamp;
-                    pv->next_chapter.index = in->s.new_chap;
-                    work_control           = &pv->force_keyframe;
+                    pv->next_chapter_pts = work_surface->Data.TimeStamp;
                 }
-                else
+                /* insert an IDR */
+                work_control = &pv->force_keyframe;
+                /*
+                 * Chapter markers are sometimes so close we can get a new
+                 * one before the previous goes through the encoding queue.
+                 *
+                 * Dropping markers can cause weird side-effects downstream,
+                 * including but not limited to missing chapters in the
+                 * output, so we need to save it somehow.
+                 */
+                struct chapter_s *item = malloc(sizeof(struct chapter_s));
+                if (item != NULL)
                 {
-                    // however unlikely, this can happen in theory
-                    hb_log("encqsvWork: got chapter %d before we could write chapter %d, dropping marker",
-                           in->s.new_chap, pv->next_chapter.index);
+                    item->index = in->s.new_chap;
+                    item->start = work_surface->Data.TimeStamp;
+                    hb_list_add(pv->delayed_chapters, item);
                 }
-                // don't let 'work_loop' put a chapter mark on the wrong buffer
+                /* don't let 'work_loop' put a chapter mark on the wrong buffer */
                 in->s.new_chap = 0;
             }
 
@@ -1515,11 +1537,31 @@ int encqsvWork( hb_work_object_t * w, hb_buffer_t ** buf_in,
              * presentation time stamp is at or after the marker's time stamp,
              * use this as the chapter start.
              */
-            if (pv->next_chapter.index && buf->s.frametype == HB_FRAME_IDR &&
-                pv->next_chapter.start <= buf->s.start)
+            if (pv->next_chapter_pts != AV_NOPTS_VALUE &&
+                pv->next_chapter_pts <= buf->s.start   &&
+                (task->bs->FrameType & MFX_FRAMETYPE_IDR))
             {
-                buf->s.new_chap = pv->next_chapter.index;
-                pv->next_chapter.index = 0;
+                // we're no longer looking for this chapter
+                pv->next_chapter_pts = AV_NOPTS_VALUE;
+
+                // get the chapter index from the list
+                struct chapter_s *item = hb_list_item(pv->delayed_chapters, 0);
+                if (item != NULL)
+                {
+                    // we're done with this chapter
+                    buf->s.new_chap = item->index;
+                    hb_list_rem(pv->delayed_chapters, item);
+                    free(item);
+
+                    // we may still have another pending chapter
+                    item = hb_list_item(pv->delayed_chapters, 0);
+                    if (item != NULL)
+                    {
+                        // we're looking for this one now
+                        // we still need it, don't remove it
+                        pv->next_chapter_pts = item->start;
+                    }
+                }
             }
 
                 // shift for fifo
diff --git a/libhb/encx264.c b/libhb/encx264.c
index f85adb0fd..ef5eece2f 100644
--- a/libhb/encx264.c
+++ b/libhb/encx264.c
@@ -55,9 +55,16 @@ struct hb_work_private_s
 
     uint32_t       frames_in;
     uint32_t       frames_out;
-    int            chap_mark;   // saved chap mark when we're propagating it
     int64_t        last_stop;   // Debugging - stop time of previous input frame
-    int64_t        next_chap;
+
+    hb_list_t *delayed_chapters;
+    int64_t next_chapter_pts;
+    // used in delayed_chapters list
+    struct chapter_s
+    {
+        int     index;
+        int64_t start;
+    };
 
     struct {
         int64_t duration;
@@ -81,6 +88,8 @@ int encx264Init( hb_work_object_t * w, hb_job_t * job )
     w->private_data = pv;
 
     pv->job = job;
+    pv->next_chapter_pts = AV_NOPTS_VALUE;
+    pv->delayed_chapters = hb_list_init();
 
     if( x264_param_default_preset( &param, job->x264_preset, job->x264_tune ) < 0 )
     {
@@ -374,6 +383,17 @@ void encx264Close( hb_work_object_t * w )
 {
     hb_work_private_t * pv = w->private_data;
 
+    if (pv->delayed_chapters != NULL)
+    {
+        struct chapter_s *item;
+        while ((item = hb_list_item(pv->delayed_chapters, 0)) != NULL)
+        {
+            hb_list_rem(pv->delayed_chapters, item);
+            free(item);
+        }
+        hb_list_close(&pv->delayed_chapters);
+    }
+
     free( pv->grey_data );
     x264_encoder_close( pv->x264 );
     free( pv );
@@ -505,10 +525,30 @@ static hb_buffer_t *nal_encode( hb_work_object_t *w, x264_picture_t *pic_out,
                frame's presentation time stamp is at or after
                the marker's time stamp, use this as the
                chapter start. */
-            if( pv->next_chap != 0 && pv->next_chap <= pic_out->i_pts )
+            if (pv->next_chapter_pts != AV_NOPTS_VALUE &&
+                pv->next_chapter_pts <= pic_out->i_pts)
             {
-                pv->next_chap = 0;
-                buf->s.new_chap = pv->chap_mark;
+                // we're no longer looking for this chapter
+                pv->next_chapter_pts = AV_NOPTS_VALUE;
+
+                // get the chapter index from the list
+                struct chapter_s *item = hb_list_item(pv->delayed_chapters, 0);
+                if (item != NULL)
+                {
+                    // we're done with this chapter
+                    buf->s.new_chap = item->index;
+                    hb_list_rem(pv->delayed_chapters, item);
+                    free(item);
+
+                    // we may still have another pending chapter
+                    item = hb_list_item(pv->delayed_chapters, 0);
+                    if (item != NULL)
+                    {
+                        // we're looking for this one now
+                        // we still need it, don't remove it
+                        pv->next_chapter_pts = item->start;
+                    }
+                }
             }
         }
 
@@ -547,10 +587,24 @@ static hb_buffer_t *x264_encode( hb_work_object_t *w, hb_buffer_t *in )
            when this frame finally pops out of the encoder we'll mark
            its buffer as the start of a chapter. */
         pv->pic_in.i_type = X264_TYPE_IDR;
-        if( pv->next_chap == 0 )
+        if (pv->next_chapter_pts == AV_NOPTS_VALUE)
+        {
+            pv->next_chapter_pts = in->s.start;
+        }
+        /*
+         * Chapter markers are sometimes so close we can get a new one before the
+         * previous marker has been through the encoding queue.
+         *
+         * Dropping markers can cause weird side-effects downstream, including but
+         * not limited to missing chapters in the output, so we need to save it
+         * somehow.
+         */
+        struct chapter_s *item = malloc(sizeof(struct chapter_s));
+        if (item != NULL)
         {
-            pv->next_chap = in->s.start;
-            pv->chap_mark = in->s.new_chap;
+            item->start = in->s.start;
+            item->index = in->s.new_chap;
+            hb_list_add(pv->delayed_chapters, item);
         }
         /* don't let 'work_loop' put a chapter mark on the wrong buffer */
         in->s.new_chap = 0;