[FFmpeg-devel] [PATCH] matroskadec, matroskadec, srtenc: Read/Write duration for subtitles. (v2)

[Philip Langdale]

Ok, let's get this sorted out once and for all.

I can't see any reason to manipulate the convergence duration instead.
These subtitle packets are the moral equivalent of keyframes, and
while the documentation says it should be used for 'some types of
subtitles', I don't see how Matroska or srt text subtitles fall into
that category.

The original claim was that convergence_duration was needed to avoid
overflow on long duration subtitles. This claim seems questionable.
If we consider the typical timebase of 1/1000, that still allows
for a duration of 8 years. For a 1/1000000 timebase, you still get
a 71 minute maximum duration, so I'm not sure where this claim
originated from. Only if you go down to a 1ns timebase do you end
up with a short max duration of ~4 seconds. Am I missing something?

This change is backward compatible by reading and writing
convergence_duration. I don't know if that's really necessary.

Signed-off-by: Philip Langdale <philipl at overt.org>
---
 libavformat/matroskadec.c |    5 +++--
 libavformat/matroskaenc.c |    5 ++++-
 libavformat/srtenc.c      |    1 +
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 2c954af..d5bdad4 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2088,10 +2088,11 @@ static int matroska_parse_block(MatroskaDemuxContext *matroska, uint8_t *data,
                 else
                     pkt->pts = timecode;
                 pkt->pos = pos;
+                pkt->duration = duration;
+
                 if (st->codec->codec_id == CODEC_ID_TEXT)
+                    /* For backward compatibility. */
                     pkt->convergence_duration = duration;
-                else if (track->type != MATROSKA_TRACK_TYPE_SUBTITLE)
-                    pkt->duration = duration;
 
                 if (st->codec->codec_id == CODEC_ID_SSA)
                     matroska_fix_ass_packet(matroska, pkt, duration);
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index f5fdaae..444b881 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1166,7 +1166,10 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
         duration = mkv_write_srt_blocks(s, pb, pkt);
     } else {
         ebml_master blockgroup = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP, mkv_blockgroup_size(pkt->size));
-        duration = pkt->convergence_duration;
+        /* For backward compatibility, prefer convergence_duration. */
+        if (pkt->convergence_duration > 0) {
+          duration = pkt->convergence_duration;
+        }
         mkv_write_block(s, pb, MATROSKA_ID_BLOCK, pkt, 0);
         put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, duration);
         end_ebml_master(pb, blockgroup);
diff --git a/libavformat/srtenc.c b/libavformat/srtenc.c
index 171c45b..0640c95 100644
--- a/libavformat/srtenc.c
+++ b/libavformat/srtenc.c
@@ -64,6 +64,7 @@ static int srt_write_packet(AVFormatContext *avf, AVPacket *pkt)
         int len;
 
         if (d <= 0)
+            /* For backward compatibility, fallback to convergence_duration. */
             d = pkt->convergence_duration;
         if (s == AV_NOPTS_VALUE || d <= 0) {
             av_log(avf, AV_LOG_ERROR, "Insufficient timestamps.\n");
-- 
1.7.9.5