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

[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 |    6 ++++--
 libavformat/srtenc.c      |    4 +++-
 3 files changed, 10 insertions(+), 5 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..abc6ddd 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1136,7 +1136,9 @@ static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
     AVIOContext *pb = s->pb;
     AVCodecContext *codec = s->streams[pkt->stream_index]->codec;
     int keyframe = !!(pkt->flags & AV_PKT_FLAG_KEY);
-    int duration = pkt->duration;
+    /* For backward compatibility, prefer convergence_duration. */
+    int duration = pkt->convergence_duration > 0 ?
+                   pkt->convergence_duration : pkt->duration;
     int ret;
     int64_t ts = mkv->tracks[pkt->stream_index].write_dts ? pkt->dts : pkt->pts;
 
@@ -1166,7 +1168,7 @@ 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;
+        duration = pkt->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..7c0119b 100644
--- a/libavformat/srtenc.c
+++ b/libavformat/srtenc.c
@@ -64,7 +64,9 @@ static int srt_write_packet(AVFormatContext *avf, AVPacket *pkt)
         int len;
 
         if (d <= 0)
-            d = pkt->convergence_duration;
+            /* For backward compatibility, prefer convergence_duration. */
+            d = pkt->convergence_duration > 0 ?
+                pkt->convergence_duration : pkt->duration;
         if (s == AV_NOPTS_VALUE || d <= 0) {
             av_log(avf, AV_LOG_ERROR, "Insufficient timestamps.\n");
             return AVERROR(EINVAL);
-- 
1.7.9.5