[FFmpeg-devel] BUG in use of extradata and extradata_size with dvb subtitles and teletext
Marton Balint
cus at passwd.hu
Tue Jan 28 02:29:05 CET 2014
On Sun, 26 Jan 2014, Andriy Lysnevych wrote:
> On Sun, Jan 26, 2014 at 3:43 AM, Marton Balint <cus at passwd.hu> wrote:
>> I don't think that is acceptable breakage, but others may know this better.
>> To make sure things don't break too badly a better solution maybe is to put
>> the subtitling_type byte AFTER the composition and ancillary page id, even
>> if that is different from the order in the subtitling descriptor.
>>
>> Also make sure the dvbsub decoder codec still works after your patch, since
>> it is using extradata...
>>
>
> Indeed dvbsub decoder uses extradata and expects composition_id and
> ancillary_id to be the first two values of extradata. Now patches do
> not break compatibility:
>
> 1) Put subtitling_type byte AFTER the composition and ancillary page
> id, as you suggested.
> 2) DVB subtitles decoder supports both old 4 byte format and new 5
> byte (i.e. with subtitling_type) formats
>
> Regards,
> Andriy Lysnevych
>
>From 1bcb4982f1b0a03e231ba69d381147505c62e03c Mon Sep 17 00:00:00 2001
>From: mrlika <andriy.lysnevych at gmail.com>
>Date: Sun, 26 Jan 2014 16:21:33 +0200
>Subject: mpegts muxer and demuxer: DVB subtitles multiple languages support
>
>* copy multiple languages data from PMT to extradata
>* restore multiple languages data from extradata to PMT table
>* setting correctly hearing empaired subtitling type
>---
> libavformat/mpegts.c | 84 +++++++++++++++++++++++++++++++++++--------------
> libavformat/mpegtsenc.c | 44 +++++++++++++++++---------
> 2 files changed, 91 insertions(+), 37 deletions(-)
>
>diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>index d352038..47e4894 100644
>--- a/libavformat/mpegts.c
>+++ b/libavformat/mpegts.c
>@@ -1512,31 +1512,69 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
> }
> break;
> case 0x59: /* subtitling descriptor */
>- language[0] = get8(pp, desc_end);
>- language[1] = get8(pp, desc_end);
>- language[2] = get8(pp, desc_end);
>- language[3] = 0;
>- /* hearing impaired subtitles detection */
>- switch(get8(pp, desc_end)) {
>- case 0x20: /* DVB subtitles (for the hard of hearing) with no monitor aspect ratio criticality */
>- case 0x21: /* DVB subtitles (for the hard of hearing) for display on 4:3 aspect ratio monitor */
>- case 0x22: /* DVB subtitles (for the hard of hearing) for display on 16:9 aspect ratio monitor */
>- case 0x23: /* DVB subtitles (for the hard of hearing) for display on 2.21:1 aspect ratio monitor */
>- case 0x24: /* DVB subtitles (for the hard of hearing) for display on a high definition monitor */
>- case 0x25: /* DVB subtitles (for the hard of hearing) with plano-stereoscopic disparity for display on a high definition monitor */
>- st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
>- break;
>- }
>- if (st->codec->extradata) {
>- if (st->codec->extradata_size == 4 && memcmp(st->codec->extradata, *pp, 4))
>- avpriv_request_sample(fc, "DVB sub with multiple IDs");
>- } else {
>- if (!ff_alloc_extradata(st->codec, 4)) {
>- memcpy(st->codec->extradata, *pp, 4);
>+ {
>+ /* 8 bytes per DVB subtitle substream data:
>+ * ISO_639_language_code (3 bytes),
>+ * subtitling_type (1 byte),
>+ * composition_page_id (2 bytes),
>+ * ancillary_page_id (2 bytes) */
>+ int language_count = desc_len / 8;
>+
>+ if (desc_len > 0 && desc_len % 8 != 0)
>+ return AVERROR_INVALIDDATA;
>+
>+ if (language_count > 1) {
>+ avpriv_request_sample(fc, "DVB subtitles with multiple languages");
>+ }
>+
>+ if (language_count > 0) {
>+ uint8_t *extradata;
>+
>+ /* 4 bytes per language code (3 bytes) with comma or NUL byte should fit language buffer */
>+ if (language_count > sizeof(language) / 4) {
>+ language_count = sizeof(language) / 4;
>+ }
>+
>+ if (st->codec->extradata == NULL) {
>+ if (ff_alloc_extradata(st->codec, language_count * 5)) {
>+ return AVERROR(ENOMEM);
>+ }
>+ }
>+
>+ if (st->codec->extradata_size < language_count * 5)
>+ return AVERROR_INVALIDDATA;
>+
>+ extradata = st->codec->extradata;
>+
>+ for (i = 0; i < language_count; i++) {
>+ language[i * 4 + 0] = get8(pp, desc_end);
>+ language[i * 4 + 1] = get8(pp, desc_end);
>+ language[i * 4 + 2] = get8(pp, desc_end);
>+ language[i * 4 + 3] = ',';
>+
>+ /* hearing impaired subtitles detection using subtitling_type */
>+ switch(*pp[0]) {
>+ case 0x20: /* DVB subtitles (for the hard of hearing) with no monitor aspect ratio criticality */
>+ case 0x21: /* DVB subtitles (for the hard of hearing) for display on 4:3 aspect ratio monitor */
>+ case 0x22: /* DVB subtitles (for the hard of hearing) for display on 16:9 aspect ratio monitor */
>+ case 0x23: /* DVB subtitles (for the hard of hearing) for display on 2.21:1 aspect ratio monitor */
>+ case 0x24: /* DVB subtitles (for the hard of hearing) for display on a high definition monitor */
>+ case 0x25: /* DVB subtitles (for the hard of hearing) with plano-stereoscopic disparity for display on a high definition monitor */
>+ st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
>+ break;
>+ }
>+
>+ extradata[4] = get8(pp, desc_end); /* subtitling_type */
>+ memcpy(extradata, *pp, 4); /* composition_page_id and ancillary_page_id */
>+ extradata += 5;
>+
>+ *pp += 4;
>+ }
>+
>+ language[i * 4 - 1] = 0;
>+ av_dict_set(&st->metadata, "language", language, 0);
> }
> }
>- *pp += 4;
>- av_dict_set(&st->metadata, "language", language, 0);
> break;
> case 0x0a: /* ISO 639 language descriptor */
> for (i = 0; i + 4 <= desc_len; i += 4) {
>diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
>index 9b42e48..b05d05a 100644
>--- a/libavformat/mpegtsenc.c
>+++ b/libavformat/mpegtsenc.c
>@@ -376,21 +376,37 @@ static void mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
> const char default_language[] = "und";
> const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;
>
>- if (st->codec->codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
>- /* The descriptor tag. subtitling_descriptor */
>- *q++ = 0x59;
>- *q++ = 8;
>- *q++ = language[0];
>- *q++ = language[1];
>- *q++ = language[2];
>- *q++ = 0x10; /* normal subtitles (0x20 = if hearing pb) */
>- if(st->codec->extradata_size == 4) {
>- memcpy(q, st->codec->extradata, 4);
>- q += 4;
>- } else {
>- put16(&q, 1); /* page id */
>- put16(&q, 1); /* ancillary page id */
>+ if (st->codec->codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
>+ uint8_t *len_ptr;
>+ int extradata_copied = 0;
>+
>+ *q++ = 0x59; /* subtitling_descriptor */
>+ len_ptr = q++;
>+
>+ while (strlen(language) >= 3) {
>+ *q++ = *language++;
>+ *q++ = *language++;
>+ *q++ = *language++;
>+ /* Skip comma */
>+ if (*language != '\0')
>+ language++;
>+
>+ if (st->codec->extradata_size - 1 > extradata_copied) {
Since you copy 5 bytes I guess you should check for
st->codec->extradata_size - 4 > extradata_copied
>+ *q++ = st->codec->extradata[extradata_copied + 4]; /* subtitling_type */
>+ memcpy(q, st->codec->extradata + extradata_copied, 4); /* composition_page_id and ancillary_page_id */
>+ extradata_copied += 5;
>+ q += 4;
>+ } else {
>+ /* subtitling_type:
>+ * 0x10 - normal with no monitor aspect ratio criticality
>+ * 0x20 - for the hard of hearing with no monitor aspect ratio criticality */
>+ *q++ = (st->disposition & AV_DISPOSITION_HEARING_IMPAIRED) ? 0x20 : 0x10;
>+ put16(&q, 1); /* composition_page_id */
>+ put16(&q, 1); /* ancillary_page_id */
>+ }
> }
>+
>+ *len_ptr = q - len_ptr - 1;
> } else if (st->codec->codec_id == AV_CODEC_ID_DVB_TELETEXT) {
> uint8_t *len_ptr = NULL;
> int extradata_copied = 0;
>--
>1.8.3.2
>
>From e9b97d63729f465399dd40ee76a12cc4a1cb24a2 Mon Sep 17 00:00:00 2001
>From: mrlika <andriy.lysnevych at gmail.com>
>Date: Sun, 26 Jan 2014 16:38:22 +0200
>Subject: DVB subtitles decoder: support of 5 bytes extradata format
>
>---
> libavcodec/dvbsubdec.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
>index 3863a44..1b2b6c3 100644
>--- a/libavcodec/dvbsubdec.c
>+++ b/libavcodec/dvbsubdec.c
>@@ -367,7 +367,11 @@ static av_cold int dvbsub_init_decoder(AVCodecContext *avctx)
> int i, r, g, b, a = 0;
> DVBSubContext *ctx = avctx->priv_data;
>
>- if (!avctx->extradata || avctx->extradata_size != 4) {
>+ if (!avctx->extradata || avctx->extradata_size < 4) {
>+ av_log(avctx, AV_LOG_WARNING, "Invalid DVB subtitle stream extradata!\n");
>+ ctx->composition_id = -1;
>+ ctx->ancillary_id = -1;
>+ } else if (avctx->extradata_size > 5) {
In case of combined streams wouldn't it make sense to set the
composition_id and ancillary_id to the ids of the first entry?
You may also want to implement a stricter check for extradata_size (e.g.:
(extradata_size == 4 || (extradata_size && extradata_size % 5 == 0))
> av_log(avctx, AV_LOG_WARNING, "Invalid extradata, subtitle streams may be combined!\n");
> ctx->composition_id = -1;
> ctx->ancillary_id = -1;
>--
>1.8.3.2
Also please consider using git send-email for sending patches, it will
make the review process easier for us and by sending patch series you can
also define the order of the patches. (In this case the DVB part would
come first as patch 01, then the mpegts part as patch 02)
Regards,
Marton
More information about the ffmpeg-devel
mailing list