[FFmpeg-cvslog] srt: make the demuxer output SubRip packets.

[Clément Bœsch]

ffmpeg | branch: master | Clément Bœsch <ubitux at gmail.com> | Thu Oct 25 00:01:42 2012 +0200| [c27b3816e4a7bf2047820ba47664ed1c874216c1] | committer: Clément Bœsch

srt: make the demuxer output SubRip packets.

The SRT format should never have outputted CODEC_ID_SRT packets in the
first place: SRT is a subtitle format containing SubRip text markup
events. The timing information is part of the format, not the codec, and
thus CODEC_ID_SRT should not exist.

Creating packets with the timing information within the payload only
leads to problem (such as remuxing with timing alteration not working),
especially when the SubRip markup is being used in container like
Matroska in addition to this standalone SRT format.

The main reason the timing line was included in those CODEC_ID_SRT
packets is likely because it contained extra information (the event
position) the codec actually needs. This issue is solved by using the
AV_PKT_DATA_SUBTITLE_POSITION side data type.

> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=c27b3816e4a7bf2047820ba47664ed1c874216c1
---

 libavcodec/srtdec.c  |   11 +++++++++++
 libavformat/srtdec.c |   44 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/libavcodec/srtdec.c b/libavcodec/srtdec.c
index 5824091..ddeabd2 100644
--- a/libavcodec/srtdec.c
+++ b/libavcodec/srtdec.c
@@ -21,6 +21,7 @@
 
 #include "libavutil/avstring.h"
 #include "libavutil/common.h"
+#include "libavutil/intreadwrite.h"
 #include "libavutil/parseutils.h"
 #include "avcodec.h"
 #include "ass.h"
@@ -219,6 +220,15 @@ static int srt_decode_frame(AVCodecContext *avctx,
     char buffer[2048];
     const char *ptr = avpkt->data;
     const char *end = avpkt->data + avpkt->size;
+    int size;
+    const uint8_t *p = av_packet_get_side_data(avpkt, AV_PKT_DATA_SUBTITLE_POSITION, &size);
+
+    if (p && size == 16) {
+        x1 = AV_RL32(p     );
+        y1 = AV_RL32(p +  4);
+        x2 = AV_RL32(p +  8);
+        y2 = AV_RL32(p + 12);
+    }
 
     if (avpkt->size <= 0)
         return avpkt->size;
@@ -247,6 +257,7 @@ static int srt_decode_frame(AVCodecContext *avctx,
 }
 
 #if CONFIG_SRT_DECODER
+/* deprecated decoder */
 AVCodec ff_srt_decoder = {
     .name         = "srt",
     .long_name    = NULL_IF_CONFIG_SMALL("SubRip subtitle with embedded timing"),
diff --git a/libavformat/srtdec.c b/libavformat/srtdec.c
index 056165e..4f5f095 100644
--- a/libavformat/srtdec.c
+++ b/libavformat/srtdec.c
@@ -47,26 +47,30 @@ static int srt_read_header(AVFormatContext *s)
         return AVERROR(ENOMEM);
     avpriv_set_pts_info(st, 64, 1, 1000);
     st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
-    st->codec->codec_id   = AV_CODEC_ID_SRT;
+    st->codec->codec_id   = AV_CODEC_ID_SUBRIP;
     return 0;
 }
 
-static int64_t get_pts(const char *buf, int *duration)
+static int64_t get_pts(char **buf, int *duration,
+                       int32_t *x1, int32_t *y1, int32_t *x2, int32_t *y2)
 {
     int i;
 
     for (i=0; i<2; i++) {
         int hh1, mm1, ss1, ms1;
         int hh2, mm2, ss2, ms2;
-        if (sscanf(buf, "%d:%2d:%2d%*1[,.]%3d --> %d:%2d:%2d%*1[,.]%3d",
+        if (sscanf(*buf, "%d:%2d:%2d%*1[,.]%3d --> %d:%2d:%2d%*1[,.]%3d"
+                   "%*[ ]X1:%u X2:%u Y1:%u Y2:%u",
                    &hh1, &mm1, &ss1, &ms1,
-                   &hh2, &mm2, &ss2, &ms2) == 8) {
+                   &hh2, &mm2, &ss2, &ms2,
+                   x1, x2, y1, y2) >= 8) {
             int64_t start = (hh1*3600LL + mm1*60LL + ss1) * 1000LL + ms1;
             int64_t end   = (hh2*3600LL + mm2*60LL + ss2) * 1000LL + ms2;
             *duration = end - start;
+            *buf += strcspn(*buf, "\n") + 1;
             return start;
         }
-        buf += strcspn(buf, "\n") + 1;
+        *buf += strcspn(*buf, "\n") + 1;
     }
     return AV_NOPTS_VALUE;
 }
@@ -87,11 +91,31 @@ static int srt_read_packet(AVFormatContext *s, AVPacket *pkt)
         ptr += ff_get_line(s->pb, ptr, sizeof(buffer)+buffer-ptr);
     } while (!is_eol(*ptr2) && !url_feof(s->pb) && ptr-buffer<sizeof(buffer)-1);
 
-    if (buffer[0] && !(res = av_new_packet(pkt, ptr-buffer))) {
-        memcpy(pkt->data, buffer, pkt->size);
-        pkt->flags |= AV_PKT_FLAG_KEY;
-        pkt->pos = pos;
-        pkt->pts = pkt->dts = get_pts(pkt->data, &(pkt->duration));
+    if (buffer[0]) {
+        int64_t pts;
+        int duration;
+        const char *end = ptr;
+        int32_t x1 = -1, y1 = -1, x2 = -1, y2 = -1;
+
+        ptr = buffer;
+        pts = get_pts(&ptr, &duration, &x1, &y1, &x2, &y2);
+        if (pts != AV_NOPTS_VALUE &&
+            !(res = av_new_packet(pkt, end - ptr))) {
+            memcpy(pkt->data, ptr, pkt->size);
+            pkt->flags |= AV_PKT_FLAG_KEY;
+            pkt->pos = pos;
+            pkt->pts = pkt->dts = pts;
+            pkt->duration = duration;
+            if (x1 != -1) {
+                uint8_t *p = av_packet_new_side_data(pkt, AV_PKT_DATA_SUBTITLE_POSITION, 16);
+                if (p) {
+                    AV_WL32(p,      x1);
+                    AV_WL32(p +  4, y1);
+                    AV_WL32(p +  8, x2);
+                    AV_WL32(p + 12, y2);
+                }
+            }
+        }
     }
     return res;
 }