sync: fix merging of multiple SSA to tx3g

When more than 2 subtitles overlapped in time, they were not merged
properly and could result in cases where the subtitle time went
backwards

5 files changed, 252 insertions, 124 deletions

diff --git a/libhb/deccc608sub.c b/libhb/deccc608sub.c
index 344c17a26..8ac6642da 100644
--- a/libhb/deccc608sub.c
+++ b/libhb/deccc608sub.c
@@ -953,11 +953,11 @@ static int write_cc_buffer_as_ssa(struct eia608_screen *data,
     }
     else if (wb->clear_sub_needed)
     {
-        hb_buffer_t *buffer = hb_buffer_init(1);
+        hb_buffer_t *buffer = hb_buffer_init(0);
         buffer->s.frametype = HB_FRAME_SUBTITLE;
-        buffer->s.start = ms_start;
-        buffer->s.stop = ms_start;
-        buffer->data[0] = 0;
+        buffer->s.flags     = HB_BUF_FLAG_EOS;
+        buffer->s.start     = ms_start;
+        buffer->s.stop      = ms_start;
         hb_buffer_list_append(&wb->list, buffer);
         wb->clear_sub_needed = 0;
     }
diff --git a/libhb/decpgssub.c b/libhb/decpgssub.c
index 9a8445a93..2ddd76f45 100644
--- a/libhb/decpgssub.c
+++ b/libhb/decpgssub.c
@@ -466,16 +466,18 @@ static int decsubWork( hb_work_object_t * w, hb_buffer_t ** buf_in,
                 }
                 else
                 {
-                    out = hb_buffer_init( 1 );
-
-                    out->s.frametype = HB_FRAME_SUBTITLE;
-                    out->s.id     = in->s.id;
-                    out->s.start  = pts;
-                    out->s.stop   = pts;
-                    out->f.x      = 0;
-                    out->f.y      = 0;
-                    out->f.width  = 0;
-                    out->f.height = 0;
+                    out = hb_buffer_init( 0 );
+
+                    out->s.frametype    = HB_FRAME_SUBTITLE;
+                    out->s.flags        = HB_BUF_FLAG_EOS;
+                    out->s.id           = in->s.id;
+                    out->s.start        = pts;
+                    out->s.stop         = pts;
+                    out->s.renderOffset = AV_NOPTS_VALUE;
+                    out->f.x            = 0;
+                    out->f.y            = 0;
+                    out->f.width        = 0;
+                    out->f.height       = 0;
                 }
             }
             hb_buffer_list_append(&pv->list, out);
diff --git a/libhb/internal.h b/libhb/internal.h
index b082a7097..f4ad2b3c9 100644
--- a/libhb/internal.h
+++ b/libhb/internal.h
@@ -93,6 +93,7 @@ struct hb_buffer_settings_s
 #endif
 #define PIC_FLAG_REPEAT_FRAME       0x0200
 #define HB_BUF_FLAG_EOF             0x0400
+#define HB_BUF_FLAG_EOS             0x0800
     uint16_t      flags;
 
 #define HB_COMB_NONE  0
diff --git a/libhb/rendersub.c b/libhb/rendersub.c
index 74b1428d0..4ecb726d8 100644
--- a/libhb/rendersub.c
+++ b/libhb/rendersub.c
@@ -785,9 +785,9 @@ static int textsub_work(hb_filter_object_t * filter,
             process_sub(pv, pv->current_sub);
             hb_buffer_close(&pv->current_sub);
         }
-        if (sub->s.start == sub->s.stop)
+        if (sub->s.flags & HB_BUF_FLAG_EOS)
         {
-            // Zero duration sub used to "clear" previous sub that had
+            // marker used to "clear" previous sub that had
             // an unknown duration
             hb_buffer_close(&sub);
         }
diff --git a/libhb/sync.c b/libhb/sync.c
index 2ee82e7a3..c9ba52196 100644
--- a/libhb/sync.c
+++ b/libhb/sync.c
@@ -1615,7 +1615,22 @@ static hb_buffer_t * merge_ssa(hb_buffer_t *a, hb_buffer_t *b)
 {
     int len, ii;
     char *text;
-    hb_buffer_t *buf = hb_buffer_init(a->size + b->size);
+    hb_buffer_t *buf;
+
+    if (a == NULL && b == NULL)
+    {
+        return NULL;
+    }
+    if (a == NULL)
+    {
+        return hb_buffer_dup(b);
+    }
+    if (b == NULL)
+    {
+        return hb_buffer_dup(a);
+    }
+
+    buf = hb_buffer_init(a->size + b->size);
     buf->s = a->s;
 
     // Find the text in the second SSA sub
@@ -1642,135 +1657,237 @@ static hb_buffer_t * merge_ssa(hb_buffer_t *a, hb_buffer_t *b)
     return buf;
 }
 
-static void setSubDuration(hb_buffer_t * buf)
+static hb_buffer_t * setSubDuration(sync_stream_t * stream, hb_buffer_t * sub)
 {
-    if (buf->s.stop != AV_NOPTS_VALUE)
-        buf->s.duration = buf->s.stop - buf->s.start;
-    else
-        buf->s.duration = AV_NOPTS_VALUE;
+    if (sub->s.flags & HB_BUF_FLAG_EOS)
+    {
+        return sub;
+    }
+    if (sub->s.stop != AV_NOPTS_VALUE)
+    {
+        sub->s.duration = sub->s.stop - sub->s.start;
+        if (sub->s.duration <= 0)
+        {
+            hb_log("sync: subtitle 0x%x duration <= 0, PTS %"PRId64"",
+                   stream->subtitle.subtitle->id, sub->s.start);
+            hb_buffer_close(&sub);
+        }
+    }
+    else if (stream->subtitle.sanitizer.link)
+    {
+        hb_log("sync: subtitle 0x%x duration not set, PTS %"PRId64"",
+               stream->subtitle.subtitle->id, sub->s.start);
+        sub->s.duration = (int64_t)AV_NOPTS_VALUE;
+    }
+    return sub;
 }
 
-static hb_buffer_t * mergeSubtitles(subtitle_sanitizer_t *sanitizer)
+// Create a list of buffers that overlap the given start time.
+// Returns a list of buffers and the smallest stop time of those
+// buffers.
+static hb_buffer_t * findOverlap(subtitle_sanitizer_t *sanitizer,
+                                 int64_t start, int64_t *stop_out)
 {
-    hb_buffer_t *a, *b, *buf;
-    hb_buffer_list_t list;
+    hb_buffer_list_t   list;
+    hb_buffer_t      * buf;
+    int64_t            stop;
 
+    stop = INT64_MAX;
     hb_buffer_list_clear(&list);
-
-    if (!sanitizer->merge)
+    buf = hb_buffer_list_head(&sanitizer->list_current);
+    while (buf != NULL)
     {
-        // Handle all but the last buffer
-        while (hb_buffer_list_count(&sanitizer->list_current) > 1)
+        if (buf->s.flags & HB_BUF_FLAG_EOF)
         {
-            buf = hb_buffer_list_rem_head(&sanitizer->list_current);
-            setSubDuration(buf);
-            hb_buffer_list_append(&list, buf);
+            break;
         }
-        // Check last buffer
-        buf = hb_buffer_list_head(&sanitizer->list_current);
-        if (buf != NULL)
+        if (buf->s.start > start)
         {
-            if (buf->s.flags & HB_BUF_FLAG_EOF)
+            if (stop > buf->s.start)
             {
-                buf = hb_buffer_list_rem_head(&sanitizer->list_current);
-                hb_buffer_list_append(&list, buf);
+                *stop_out = buf->s.start;
             }
-            else if (buf->s.stop != AV_NOPTS_VALUE)
+            break;
+        }
+        if (buf->s.start <= start && start < buf->s.stop)
+        {
+            hb_buffer_t * tmp = hb_buffer_dup(buf);
+            tmp->s.start = start;
+            hb_buffer_list_append(&list, tmp);
+            if (stop > buf->s.stop)
             {
-                buf = hb_buffer_list_rem_head(&sanitizer->list_current);
-                setSubDuration(buf);
-                hb_buffer_list_append(&list, buf);
+                stop = buf->s.stop;
+                *stop_out = stop;
             }
         }
-        return hb_buffer_list_clear(&list);
+        buf = buf->next;
     }
 
-    // We only reach here if we are merging subtitles
-    while (hb_buffer_list_count(&sanitizer->list_current) > 0)
+    return hb_buffer_list_clear(&list);
+}
+
+// Find all subtitles in the list that start "now" and overlap for
+// some period of time.  Create a new subtitle buffer that is the
+// merged results of the overlapping parts and update start times
+// of non-overlapping parts.
+static int mergeSubtitleOverlaps(subtitle_sanitizer_t *sanitizer)
+{
+    hb_buffer_t  * merged_buf   = NULL;
+    hb_buffer_t  * a, * b;
+
+    a = hb_buffer_list_head(&sanitizer->list_current);
+    if (a != NULL && (a->s.flags & HB_BUF_FLAG_EOF))
     {
-        a = hb_buffer_list_head(&sanitizer->list_current);
-        if (a->s.flags & HB_BUF_FLAG_EOF)
+        // EOF
+        return 0;
+    }
+    if (a == NULL ||
+        a->s.start == AV_NOPTS_VALUE || a->s.stop == AV_NOPTS_VALUE)
+    {
+        // Not enough information to resolve an overlap
+        return -1;
+    }
+    b = a->next;
+    if (b != NULL && a->s.stop <= b->s.start)
+    {
+        // No overlap
+        return 0;
+    }
+
+    // Check that we have 2 non-overlapping buffers in the list
+    // and that all timestamps are valid up to the non-overlap.
+    // This ensures that multiple overlapping subtitles have been
+    // completely merged.
+    while (b != NULL &&
+           b->s.start < a->s.stop && !(b->s.flags & HB_BUF_FLAG_EOF))
+    {
+        if (b->s.start == AV_NOPTS_VALUE || b->s.stop == AV_NOPTS_VALUE)
         {
-            buf = hb_buffer_list_rem_head(&sanitizer->list_current);
-            hb_buffer_list_append(&list, buf);
-            break;
+            // Not enough information to resolve an overlap
+            return -1;
         }
-        b = a->next;
-        if (b == NULL)
+        b = b->next;
+    }
+    if (b == NULL)
+    {
+        // Not enough information to resolve an overlap
+        return -1;
+    }
+
+    hb_buffer_list_t   merged_list;
+    int64_t            start, stop, last;
+
+    if (b->s.flags & HB_BUF_FLAG_EOF)
+    {
+        last = INT64_MAX;
+    }
+    else
+    {
+        last = b->s.start;
+    }
+
+    hb_buffer_list_clear(&merged_list);
+    a = hb_buffer_list_head(&sanitizer->list_current);
+    start = a->s.start;
+    while (start < last)
+    {
+        hb_buffer_t * merge = findOverlap(sanitizer, start, &stop);
+        if (merge == NULL)
         {
             break;
         }
-        if (b->s.flags & HB_BUF_FLAG_EOF)
+        a = merge;
+        merged_buf = NULL;
+        while (a != NULL)
         {
-            buf = hb_buffer_list_rem_head(&sanitizer->list_current);
-            setSubDuration(buf);
-            hb_buffer_list_append(&list, buf);
-            buf = hb_buffer_list_rem_head(&sanitizer->list_current);
-            hb_buffer_list_append(&list, buf);
-            break;
-        }
+            hb_buffer_t * tmp;
 
-        if (a->s.stop == AV_NOPTS_VALUE)
-        {
-            // To evaluate overlaps, we need the stop times
-            // This should really never happen...
-            break;
+            tmp = merge_ssa(merged_buf, a);
+            hb_buffer_close(&merged_buf);
+            merged_buf = tmp;
+
+            a = a->next;
         }
+        merged_buf->s.stop = stop;
+        hb_buffer_close(&merge);
+        hb_buffer_list_append(&merged_list, merged_buf);
+        start = stop;
 
-        buf = NULL;
-        if (a->s.stop > b->s.start)
+        // Remove merged buffers
+        a = hb_buffer_list_head(&sanitizer->list_current);
+        while (a != NULL && a->s.start < stop &&
+               !(a->s.flags & HB_BUF_FLAG_EOF))
         {
-            // Overlap
-            if (ABS(a->s.start - b->s.start) <= 18000)
+            hb_buffer_t * next = a->next;
+            if (a->s.stop <= stop)
             {
-                if (b->s.stop == AV_NOPTS_VALUE)
-                {
-                    // To evaluate overlaps, we need the stop times
-                    // for a and b
-                    break;
-                }
-                a = hb_buffer_list_rem_head(&sanitizer->list_current);
+                // Buffer consumed
+                hb_buffer_list_rem(&sanitizer->list_current, a);
+                hb_buffer_close(&a);
+            }
+            else
+            {
+                a->s.start = stop;
+            }
+            a = next;
+        }
+    }
+    merged_buf = hb_buffer_list_clear(&merged_list);
+    hb_buffer_list_prepend(&sanitizer->list_current, merged_buf);
 
-                // subtitles start within 1/5 second of eachother, merge
-                if (a->s.stop > b->s.stop && b->s.stop != AV_NOPTS_VALUE)
-                {
-                    // a continues after b, reorder the list and swap
-                    b = a;
-                    a = hb_buffer_list_rem_head(&sanitizer->list_current);
-                    hb_buffer_list_prepend(&sanitizer->list_current, b);
-                }
+    return 0;
+}
 
-                b->s.start = a->s.stop;
+static hb_buffer_t * mergeSubtitles(sync_stream_t * stream)
+{
+    hb_buffer_t          * buf;
+    hb_buffer_list_t       list;
+    subtitle_sanitizer_t * sanitizer = &stream->subtitle.sanitizer;
 
-                buf = merge_ssa(a, b);
-                hb_buffer_close(&a);
-                a = buf;
+    hb_buffer_list_clear(&list);
 
-                if (b->s.stop != AV_NOPTS_VALUE &&
-                    ABS(b->s.stop - b->s.start) <= 18000)
-                {
-                    // b and a completely overlap, remove b
-                    b = hb_buffer_list_rem_head(&sanitizer->list_current);
-                    hb_buffer_close(&b);
-                }
-            }
-            else
+    if (!sanitizer->merge)
+    {
+        int limit = sanitizer->link ? 1 : 0;
+
+        // Handle all but the last buffer
+        // The last buffer may not have been "linked" yet
+        while (hb_buffer_list_count(&sanitizer->list_current) > limit)
+        {
+            buf = hb_buffer_list_rem_head(&sanitizer->list_current);
+            if (!(buf->s.flags & HB_BUF_FLAG_EOF))
             {
-                // a starts before b, output copy of a and update a start
-                buf = hb_buffer_dup(a);
-                buf->s.stop = b->s.start;
-                a->s.start = b->s.start;
+                buf = setSubDuration(stream, buf);
+                hb_buffer_list_append(&list, buf);
             }
         }
-        else if (a->s.stop <= b->s.start)
+        return hb_buffer_list_clear(&list);
+    }
+
+    // We only reach here if we are merging subtitles
+    while (hb_buffer_list_count(&sanitizer->list_current) > 0)
+    {
+        buf = hb_buffer_list_head(&sanitizer->list_current);
+        if (buf->s.flags & HB_BUF_FLAG_EOF)
         {
-            // a starts and ends before b
+            // remove EOF from list, add to output
             buf = hb_buffer_list_rem_head(&sanitizer->list_current);
+            hb_buffer_list_append(&list, buf);
+            break;
         }
 
-        if (buf != NULL)
+        int result = mergeSubtitleOverlaps(sanitizer);
+        if (result < 0)
+        {
+            // not enough information available yet to resolve the overlap
+            break;
+        }
+
+        // Overlap resolved, output a buffer
+        buf = hb_buffer_list_rem_head(&sanitizer->list_current);
+        if (buf != NULL && !(buf->s.flags & HB_BUF_FLAG_EOF))
         {
-            setSubDuration(buf);
+            buf = setSubDuration(stream, buf);
             hb_buffer_list_append(&list, buf);
         }
     }
@@ -1779,10 +1896,11 @@ static hb_buffer_t * mergeSubtitles(subtitle_sanitizer_t *sanitizer)
 }
 
 static hb_buffer_t * sanitizeSubtitle(
-    subtitle_sanitizer_t * sanitizer,
+    sync_stream_t        * stream,
     hb_buffer_t          * sub)
 {
-    hb_buffer_list_t list;
+    hb_buffer_list_t       list;
+    subtitle_sanitizer_t * sanitizer = &stream->subtitle.sanitizer;
 
     if (sub == NULL)
     {
@@ -1792,16 +1910,17 @@ static hb_buffer_t * sanitizeSubtitle(
     hb_buffer_list_set(&list, sub);
     if (!sanitizer->link && !sanitizer->merge)
     {
-        sub = hb_buffer_list_head(&list);
-        while (sub != NULL)
+        hb_buffer_list_t out_list;
+        hb_buffer_list_clear(&out_list);
+
+        sub = hb_buffer_list_rem_head(&list);
+        while (sub != NULL && !(sub->s.flags & HB_BUF_FLAG_EOF))
         {
-            if (sub->s.stop != AV_NOPTS_VALUE)
-                sub->s.duration = sub->s.stop - sub->s.start;
-            else
-                sub->s.duration = AV_NOPTS_VALUE;
-            sub = sub->next;
+            sub = setSubDuration(stream, sub);
+            hb_buffer_list_append(&out_list, sub);
+            sub = hb_buffer_list_rem_head(&list);
         }
-        return hb_buffer_list_clear(&list);
+        return hb_buffer_list_clear(&out_list);
     }
 
     sub = hb_buffer_list_rem_head(&list);
@@ -1812,26 +1931,32 @@ static hb_buffer_t * sanitizeSubtitle(
             hb_buffer_list_append(&sanitizer->list_current, sub);
             break;
         }
+
         hb_buffer_t *last = hb_buffer_list_tail(&sanitizer->list_current);
         if (last != NULL && last->s.stop == AV_NOPTS_VALUE)
         {
             last->s.stop = sub->s.start;
+            if (last->s.stop <= last->s.start)
+            {
+                // Subtitle duration <= 0.  Drop it.
+                hb_log("sync: subtitle 0x%x has no duration, PTS %"PRId64"",
+                       stream->subtitle.subtitle->id, sub->s.start);
+                hb_buffer_list_rem_tail(&sanitizer->list_current);
+                hb_buffer_close(&last);
+            }
         }
 
-        if (sub->s.start == sub->s.stop)
+        if (sub->s.flags & HB_BUF_FLAG_EOS)
         {
             // Used to indicate "clear" subtitles when the duration
             // of subtitles is not encoded in the stream
             hb_buffer_close(&sub);
         }
-        else
-        {
-            hb_buffer_list_append(&sanitizer->list_current, sub);
-        }
+        hb_buffer_list_append(&sanitizer->list_current, sub);
         sub = hb_buffer_list_rem_head(&list);
     }
 
-    return mergeSubtitles(sanitizer);
+    return mergeSubtitles(stream);
 }
 
 /***********************************************************************
@@ -2239,14 +2364,14 @@ static int syncSubtitleWork( hb_work_object_t * w, hb_buffer_t ** buf_in,
     {
         hb_buffer_list_append(&pv->stream->out_queue, hb_buffer_eof_init());
         in = hb_buffer_list_clear(&pv->stream->out_queue);
-        *buf_out = sanitizeSubtitle(&pv->stream->subtitle.sanitizer, in);
+        *buf_out = sanitizeSubtitle(pv->stream, in);
         return HB_WORK_DONE;
     }
     if (in->s.flags & HB_BUF_FLAG_EOF)
     {
         streamFlush(pv->stream);
         in = hb_buffer_list_clear(&pv->stream->out_queue);
-        *buf_out = sanitizeSubtitle(&pv->stream->subtitle.sanitizer, in);
+        *buf_out = sanitizeSubtitle(pv->stream, in);
         if (pv->common->job->indepth_scan)
         {
             // When doing subtitle indepth scan, the pipeline ends at sync.
@@ -2262,7 +2387,7 @@ static int syncSubtitleWork( hb_work_object_t * w, hb_buffer_t ** buf_in,
     in = hb_buffer_list_clear(&pv->stream->out_queue);
     if (in != NULL)
     {
-        *buf_out = sanitizeSubtitle(&pv->stream->subtitle.sanitizer, in);
+        *buf_out = sanitizeSubtitle(pv->stream, in);
     }
 
     return HB_WORK_OK;