rcombs:
> Fixes ticket #5641
> ---
> libavformat/matroska.c | 11 ++--
> libavformat/matroskadec.c | 111 ++++++++++++++++++++++++--------------
> 2 files changed, 77 insertions(+), 45 deletions(-)
>
> diff --git a/libavformat/matroska.c b/libavformat/matroska.c
> index 7c56aba403..962fa496f4 100644
> --- a/libavformat/matroska.c
> +++ b/libavformat/matroska.c
> @@ -60,16 +60,12 @@ const CodecTags ff_mkv_codec_tags[]={
> {"A_VORBIS" , AV_CODEC_ID_VORBIS},
> {"A_WAVPACK4" , AV_CODEC_ID_WAVPACK},
>
> - {"D_WEBVTT/SUBTITLES" , AV_CODEC_ID_WEBVTT},
> - {"D_WEBVTT/CAPTIONS" , AV_CODEC_ID_WEBVTT},
> - {"D_WEBVTT/DESCRIPTIONS", AV_CODEC_ID_WEBVTT},
> - {"D_WEBVTT/METADATA" , AV_CODEC_ID_WEBVTT},
> -
> {"S_TEXT/UTF8" , AV_CODEC_ID_SUBRIP},
> {"S_TEXT/UTF8" , AV_CODEC_ID_TEXT},
> {"S_TEXT/ASCII" , AV_CODEC_ID_TEXT},
> {"S_TEXT/ASS" , AV_CODEC_ID_ASS},
> {"S_TEXT/SSA" , AV_CODEC_ID_ASS},
> + {"S_TEXT/WEBVTT" , AV_CODEC_ID_WEBVTT},
> {"S_ASS" , AV_CODEC_ID_ASS},
> {"S_SSA" , AV_CODEC_ID_ASS},
> {"S_VOBSUB" , AV_CODEC_ID_DVD_SUBTITLE},
> @@ -77,6 +73,11 @@ const CodecTags ff_mkv_codec_tags[]={
> {"S_HDMV/PGS" , AV_CODEC_ID_HDMV_PGS_SUBTITLE},
> {"S_HDMV/TEXTST" , AV_CODEC_ID_HDMV_TEXT_SUBTITLE},
>
> + {"D_WEBVTT/SUBTITLES" , AV_CODEC_ID_WEBVTT},
> + {"D_WEBVTT/CAPTIONS" , AV_CODEC_ID_WEBVTT},
> + {"D_WEBVTT/DESCRIPTIONS", AV_CODEC_ID_WEBVTT},
> + {"D_WEBVTT/METADATA" , AV_CODEC_ID_WEBVTT},
> +
> {"V_AV1" , AV_CODEC_ID_AV1},
> {"V_DIRAC" , AV_CODEC_ID_DIRAC},
> {"V_FFV1" , AV_CODEC_ID_FFV1},
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index cff7f0cb54..1e4947e209 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -3305,62 +3305,81 @@ static int matroska_parse_webvtt(MatroskaDemuxContext
> *matroska,
> uint8_t *data, int data_len,
> uint64_t timecode,
> uint64_t duration,
> - int64_t pos)
> + int64_t pos,
> + uint8_t *additional, uint64_t
> additional_id, int additional_size)
> {
> AVPacket pktl, *pkt = &pktl;
> - uint8_t *id, *settings, *text, *buf;
> - int id_len, settings_len, text_len;
> + uint8_t *id, *settings, *comment, *text, *buf;
> + int id_len = 0, settings_len = 0, comment_len = 0, text_len;
> uint8_t *p, *q;
> int err;
> + int webm_style = !strncmp(track->codec_id, "D_WEBVTT/", 9);
>
> if (data_len <= 0)
> return AVERROR_INVALIDDATA;
>
> - p = data;
> - q = data + data_len;
> -
> - id = p;
> - id_len = -1;
> - while (p < q) {
> - if (*p == '\r' || *p == '\n') {
> - id_len = p - id;
> - if (*p == '\r')
> - p++;
> - break;
> + p = webm_style ? data : additional;
> + q = webm_style ? (data + data_len) : (additional + additional_size);
> +
Unfortunately all pointer arithmetic involving NULL is undefined
behaviour in C, even NULL + 0. So the above is ub when not using
webm-style and when there is no BlockAddition.
> + if (p) {
> + id = p;
> + id_len = -1;
This is unnecessary. Initializing to zero (as is already done above)
works just fine.
> + while (p < q) {
> + if (*p == '\r' || *p == '\n') {
> + id_len = p - id;
> + if (*p == '\r')
> + p++;
> + break;
> + }
> + p++;
> }
> +
> + if (p >= q || *p != '\n')
> + return AVERROR_INVALIDDATA;
> p++;
> - }
>
> - if (p >= q || *p != '\n')
> - return AVERROR_INVALIDDATA;
> - p++;
> -
> - settings = p;
> - settings_len = -1;
> - while (p < q) {
> - if (*p == '\r' || *p == '\n') {
> - settings_len = p - settings;
> - if (*p == '\r')
> - p++;
> - break;
> + settings = p;
> + settings_len = -1;
> + while (p < q) {
> + if (*p == '\r' || *p == '\n') {
> + settings_len = p - settings;
> + if (*p == '\r')
> + p++;
> + break;
> + }
> + p++;
> }
> +
> + if (p >= q || *p != '\n')
> + return AVERROR_INVALIDDATA;
You're repeating yourself.
> p++;
> +
> + if (!webm_style && p < q) {
> + if (q[-1] != '\r' && q[-1] != '\n')
The current Matroska codec mapping indeed requires the terminating
WebVTT line terminator to be retained, yet mkvmerge doesn't do so, so we
should not be so picky with this.
> + return AVERROR_INVALIDDATA;
> +
> + comment = p;
> + comment_len = q - p;
> + }
> }
>
> - if (p >= q || *p != '\n')
> - return AVERROR_INVALIDDATA;
> - p++;
> -
> - text = p;
> - text_len = q - p;
> - while (text_len > 0) {
> - const int len = text_len - 1;
> - const uint8_t c = p[len];
> - if (c != '\r' && c != '\n')
> - break;
> - text_len = len;
> + if (webm_style) {
> + text = p;
> + text_len = q - p;
> +
> + while (text_len > 0) {
> + const int len = text_len - 1;
> + const uint8_t c = p[len];
> + if (c != '\r' && c != '\n')
> + break;
> + text_len = len;
> + }
> + } else {
> + text = data;
> + text_len = data_len;
> }
>
> +
> if (text_len <= 0)
> return AVERROR_INVALIDDATA;
>
> @@ -3393,6 +3412,17 @@ static int matroska_parse_webvtt(MatroskaDemuxContext
> *matroska,
> memcpy(buf, settings, settings_len);
> }
>
> + if (comment_len > 0) {
> + buf = av_packet_new_side_data(pkt,
> + AV_PKT_DATA_WEBVTT_COMMENT,
> + comment_len);
> + if (!buf) {
> + av_packet_unref(pkt);
> + return AVERROR(ENOMEM);
> + }
> + memcpy(buf, comment, comment_len);
> + }
> +
> // Do we need this for subtitles?
> // pkt->flags = AV_PKT_FLAG_KEY;
>
> @@ -3656,7 +3686,8 @@ static int matroska_parse_block(MatroskaDemuxContext
> *matroska, AVBufferRef *buf
> res = matroska_parse_webvtt(matroska, track, st,
> out_data, out_size,
> timecode, lace_duration,
> - pos);
> + pos,
> + additional, additional_id,
> additional_size);
> if (!buf)
> av_free(out_data);
> if (res)
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
