Bug#1032972: handbrake: debian version of handbrake does not handle subtitles correctly

[michael spreng <debian@m.spreng.ch>]

I stumbled on this issue as well. It looks to me like this patch: 
https://github.com/HandBrake/HandBrake/blob/master/contrib/ffmpeg/A07-dvdsubdec-use-pts-of-initial-packet.patch
is particularly important for how handbrake handles dvd sub titles.

My understanding is: dvd subtitles are large (being images) and will 
usually exceed a packet in the stream. ffmpeg only returns the pts with 
the first invocation (packet), where no complete subtitle results. The 
second invocation (packet), which usually completes the subtitle and 
results in a complete subtitle returned, does not contain the pts anymore.

The above mentioned patch to ffmpeg changes ffmpeg to remember the pts. 
But handbrake can remember the pts just as well. So see the attached 
patch which does exactly that: if the subtitle is incomplete, it saves 
the pts to the handbrake subtitle context, and retrieves it if there is 
no pts on a completed subtitle ready for output.

I am unsure how to proceed from here. Is that fix acceptable? Where 
would I submit it?
From: michael spreng <debian@m.spreng.ch>
Date: Wed, 22 Nov 2023 15:36:17 +0100
Subject: [PATCH] Save pts of incomplete subtitle

Referring to bug "debian version of handbrake does NOT handle subtitles
correctly"
handbrake patches ffmpeg to save the pts of multi-packet subtitles
https://github.com/HandBrake/HandBrake/blob/master/contrib/ffmpeg/A07-dvdsubdec-use-pts-of-initial-packet.patch
As debian uses the system libraries, this patch implements the saving of
the pts in handbrake to fix subtitles for debian
---
 libhb/decavsub.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/libhb/decavsub.c b/libhb/decavsub.c
index aad32beb2..5738b1976 100644
--- a/libhb/decavsub.c
+++ b/libhb/decavsub.c
@@ -28,6 +28,12 @@ struct hb_avsub_context_s
     //      while this should really get fixed elsewhere,
     //      dropping subtitles should be avoided as much as possible
     int64_t last_pts;
+    // large subtitles (dvd sub, image based for example) can be split
+    // across several packets. avcodec_decode_subtitle2 can return the
+    // pts in a non complete packet (got_sub_ptr == 0). In that case
+    // save the pts and reuse it once all packets were processed and
+    // the subtitle is complete
+    int64_t unused_pts;
     // For PGS subs, we need to pass 'empty' subtitles through (they clear the
     // display) - when doing forced-only extraction, only pass empty subtitles
     // through if we've seen a forced sub since the last empty sub
@@ -49,6 +55,7 @@ hb_avsub_context_t * decavsubInit( hb_work_object_t * w, hb_job_t * job )
     }
     ctx->seen_forced_sub       = 0;
     ctx->last_pts              = AV_NOPTS_VALUE;
+    ctx->unused_pts            = AV_NOPTS_VALUE;
     ctx->job                   = job;
     ctx->subtitle              = w->subtitle;
 
@@ -360,6 +367,11 @@ int decavsubWork( hb_avsub_context_t * ctx,
 
         if (!has_subtitle)
         {
+            if (subtitle.pts != AV_NOPTS_VALUE)
+            {
+                ctx->unused_pts = av_rescale(subtitle.pts, 90000, AV_TIME_BASE) +
+                      av_rescale(subtitle.start_display_time, 90000, 1000);
+            }
             continue;
         }
 
@@ -436,7 +448,11 @@ int decavsubWork( hb_avsub_context_t * ctx,
         }
         else
         {
-            if (in_s.start >= 0)
+            if (ctx->unused_pts != AV_NOPTS_VALUE)
+            {
+                pts = ctx->unused_pts;
+            }
+            else if (in_s.start >= 0)
             {
                 pts = in_s.start;
             }
@@ -475,6 +491,7 @@ int decavsubWork( hb_avsub_context_t * ctx,
             pts = ctx->last_pts + 1 * 90000LL;
         }
         ctx->last_pts = pts;
+        ctx->unused_pts = AV_NOPTS_VALUE;
 
         if (ctx->subtitle->format == TEXTSUB)
         {
-- 
2.39.2