@@ -2404,15 +2404,6 @@ static int mkv_write_header(AVFormatContext *s)
return 0;
}
-static int mkv_blockgroup_size(int pkt_size, int track_num_size)
-{
- int size = pkt_size + track_num_size + 3;
- size += ebml_length_size(size);
- size += 2; // EBML ID for block and block duration
- size += 9; // max size of block duration incl. length field
- return size;
-}
-
#if CONFIG_MATROSKA_MUXER
static int mkv_reformat_h2645(MatroskaMuxContext *mkv, AVIOContext *pb,
const AVPacket *pkt, int *size)
@@ -2481,10 +2472,38 @@ static int mkv_reformat_av1(MatroskaMuxContext *mkv, AVIOContext *pb,
return 0;
}
+static int webm_reformat_vtt(MatroskaMuxContext *mkv, AVIOContext *pb,
+ const AVPacket *pkt, int *size)
+{
+ const uint8_t *id, *settings;
+ size_t id_size, settings_size;
+ unsigned total = pkt->size + 2U;
+
+ if (total > INT_MAX)
+ return AVERROR(ERANGE);
+
+ id = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_IDENTIFIER,
+ &id_size);
+ settings = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_SETTINGS,
+ &settings_size);
+ if (id_size > INT_MAX - total || settings_size > INT_MAX - (total += id_size))
+ return AVERROR(ERANGE);
+ *size = total += settings_size;
+ if (pb) {
+ avio_write(pb, id, id_size);
+ avio_w8(pb, '\n');
+ avio_write(pb, settings, settings_size);
+ avio_w8(pb, '\n');
+ avio_write(pb, pkt->data, pkt->size);
+ }
+ return 0;
+}
+
static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
AVIOContext *pb, const AVCodecParameters *par,
mkv_track *track, const AVPacket *pkt,
- int keyframe, int64_t ts, uint64_t duration)
+ int keyframe, int64_t ts, uint64_t duration,
+ int force_blockgroup)
{
uint8_t *side_data;
size_t side_data_size;
@@ -2506,8 +2525,6 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
if (duration)
ebml_writer_add_uint(&writer, MATROSKA_ID_BLOCKDURATION, duration);
- /* The following string is identical to the one in mkv_write_vtt_blocks
- * so that only one copy needs to exist in binaries. */
av_log(logctx, AV_LOG_DEBUG,
"Writing block of size %d with pts %" PRId64 ", dts %" PRId64 ", "
"duration %" PRId64 " at relative offset %" PRId64 " in cluster "
@@ -2542,7 +2559,7 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
ebml_writer_close_master(&writer);
}
- if (writer.nb_elements == 2) {
+ if (!force_blockgroup && writer.nb_elements == 2) {
/* Nothing except the BlockGroup + Block. Can use a SimpleBlock. */
writer.elements++; // Skip the BlockGroup.
writer.nb_elements--;
@@ -2557,59 +2574,6 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
return ebml_writer_write(&writer, pb);
}
-static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, const AVPacket *pkt)
-{
- MatroskaMuxContext *mkv = s->priv_data;
- mkv_track *track = &mkv->tracks[pkt->stream_index];
- ebml_master blockgroup;
- size_t id_size, settings_size;
- int size, id_size_int, settings_size_int;
- const char *id, *settings;
- int64_t ts = track->write_dts ? pkt->dts : pkt->pts;
- const int flags = 0;
-
- id = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_IDENTIFIER,
- &id_size);
- id = id ? id : "";
-
- settings = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_SETTINGS,
- &settings_size);
- settings = settings ? settings : "";
-
- if (id_size > INT_MAX - 2 || settings_size > INT_MAX - id_size - 2 ||
- pkt->size > INT_MAX - settings_size - id_size - 2)
- return AVERROR(EINVAL);
-
- size = id_size + 1 + settings_size + 1 + pkt->size;
-
- /* The following string is identical to the one in mkv_write_block so that
- * only one copy needs to exist in binaries. */
- av_log(s, AV_LOG_DEBUG,
- "Writing block of size %d with pts %" PRId64 ", dts %" PRId64 ", "
- "duration %" PRId64 " at relative offset %" PRId64 " in cluster "
- "at offset %" PRId64 ". TrackNumber %u, keyframe %d\n",
- size, pkt->pts, pkt->dts, pkt->duration, avio_tell(pb),
- mkv->cluster_pos, track->track_num, 1);
-
- blockgroup = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP,
- mkv_blockgroup_size(size, track->track_num_size));
-
- put_ebml_id(pb, MATROSKA_ID_BLOCK);
- put_ebml_length(pb, size + track->track_num_size + 3, 0);
- put_ebml_num(pb, track->track_num, track->track_num_size);
- avio_wb16(pb, ts - mkv->cluster_pts);
- avio_w8(pb, flags);
-
- id_size_int = id_size;
- settings_size_int = settings_size;
- avio_printf(pb, "%.*s\n%.*s\n%.*s", id_size_int, id, settings_size_int, settings, pkt->size, pkt->data);
-
- put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, pkt->duration);
- end_ebml_master(pb, blockgroup);
-
- return 0;
-}
-
static int mkv_end_cluster(AVFormatContext *s)
{
MatroskaMuxContext *mkv = s->priv_data;
@@ -2769,9 +2733,11 @@ static int mkv_write_packet_internal(AVFormatContext *s, const AVPacket *pkt)
relative_packet_pos = avio_tell(pb);
- if (par->codec_id != AV_CODEC_ID_WEBVTT) {
+ /* The WebM spec requires WebVTT to be muxed in BlockGroups;
+ * so we force it even for packets without duration. */
ret = mkv_write_block(s, mkv, pb, par, track, pkt,
- keyframe, ts, write_duration);
+ keyframe, ts, write_duration,
+ par->codec_id == AV_CODEC_ID_WEBVTT);
if (ret < 0)
return ret;
if (keyframe && IS_SEEKABLE(s->pb, mkv) &&
@@ -2785,18 +2751,6 @@ static int mkv_write_packet_internal(AVFormatContext *s, const AVPacket *pkt)
return ret;
track->has_cue = 1;
}
- } else {
- ret = mkv_write_vtt_blocks(s, pb, pkt);
- if (ret < 0)
- return ret;
-
- if (IS_SEEKABLE(s->pb, mkv)) {
- ret = mkv_add_cuepoint(mkv, pkt->stream_index, ts,
- mkv->cluster_pos, relative_packet_pos, duration);
- if (ret < 0)
- return ret;
- }
- }
track->last_timestamp = ts;
mkv->duration = FFMAX(mkv->duration, ts + duration);
@@ -3162,6 +3116,9 @@ static int mkv_init(struct AVFormatContext *s)
case AV_CODEC_ID_AV1:
track->reformat = mkv_reformat_av1;
break;
+ case AV_CODEC_ID_WEBVTT:
+ track->reformat = webm_reformat_vtt;
+ break;
}
if (s->flags & AVFMT_FLAG_BITEXACT) {
@@ -1,5 +1,5 @@
-c5625f28e6968e12d91f125edef5f16c *tests/data/fate/webm-webvtt-remux.webm
-6560 tests/data/fate/webm-webvtt-remux.webm
+8620a6614f149fc49ab7f4552373943e *tests/data/fate/webm-webvtt-remux.webm
+6556 tests/data/fate/webm-webvtt-remux.webm
#tb 0: 1/1000
#media_type 0: subtitle
#codec_id 0: webvtt
Up until now, the WebM variant of WebVTT subtitles has been handled specially: It had its own function to write it, because the data had to be reformatted before writing. But given that other codecs also need reformatting, this is no good reason to also duplicate the generic stuff for writing Block(Group)s. This commit therefore uses an ordinary reformatting function for this task; writing WebVTT subtitles now uses the generic code and therefore automatically uses the least amount of bytes for its BlockGroup length fields whereas the earlier code used an overestimation for the length of the Duration element. This is the reason for the changes to the webm-webvtt-remux FATE-test. (This commit does not implement support for Matroska's way of muxing WebVTT; it also does not add checks to ensure that WebM-style subtitles don't get muxed in Matroska. But the function for reformatting gets a webm prefix to indicate that this is for WebM.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Finally! Removing this has been on my list for ages; see https://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/243012.html libavformat/matroskaenc.c | 117 ++++++++++--------------------- tests/ref/fate/webm-webvtt-remux | 4 +- 2 files changed, 39 insertions(+), 82 deletions(-)