[FFmpeg-devel] BUG in use of extradata and extradata_size with dvb subtitles and teletext
Marton Balint
cus at passwd.hu
Sat Jan 18 13:19:12 CET 2014
On Mon, 13 Jan 2014, Andriy Lysnevych wrote:
> Hi,
>
> Can you please also review next patch that fixes DVB teletext issues,
> particularly ticket #2223.
>
> In the patch we use extradata for DVB teletext in the same way as it
> was for DVB subtitles: copy PMT data from input stream and restore it
> in output stream.
>
> After applying the patch DVB teletext and DVB teletext subtitles copy
> from input TS stream to output works for us.
>
> Regards,
> Andriy Lysnevych
>
> From 5877de1bb5ccbc6d151401aab205004495ad3283 Mon Sep 17 00:00:00 2001
> From: Serhii Marchuk <serhii.marchuk at gmail.com>
> Date: Mon, 13 Jan 2014 11:45:29 +0200
> Subject: mpegts muxer and demuxer: correct DVB teletext processing
>
> * Using extradata by TS demuxer to store values from PMT
> * Using extradata by TS muxer to correctly restore PMT table
> * PES_header_data_length should be always 0x24 for DVB teletext,
> according to DVB standard
> * Support of multiple languages in one DVB teletext stream:
> comma separated language codes in metadata "language" field
>
> The patch fixes #2223 ticket.
> ---
> libavformat/mpegts.c | 30 ++++++++++++++---
> libavformat/mpegtsenc.c | 85 ++++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 91 insertions(+), 24 deletions(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index db380ca..91279b1 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -1470,11 +1470,31 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
> }
> break;
> case 0x56: /* DVB teletext descriptor */
> - language[0] = get8(pp, desc_end);
> - language[1] = get8(pp, desc_end);
> - language[2] = get8(pp, desc_end);
> - language[3] = 0;
> - av_dict_set(&st->metadata, "language", language, 0);
> + {
> + int language_count = desc_len / 5;
> + uint8_t *extradata = 0;
nit: NULL
> +
> + if (!st->codec->extradata)
> + if (!ff_alloc_extradata(st->codec, language_count * 2))
> + extradata = st->codec->extradata;
else return AVERROR(ENOMEM)?
> +
> + 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] = ',';
> +
> + if (extradata) {
> + memcpy(extradata, *pp, 2);
> + extradata += 2;
> + }
> +
> + *pp += 2;
> + }
> +
> + language[i * 4 - 1] = 0;
That seems broken if language_count == 0.
> + av_dict_set(&st->metadata, "language", language, 0);
Only set this is language_count > 0
> + }
You may consider using a similar logic that is in case 0x0a for
constructing the language metadata, but this here can also have its
merits and can be made right.
> break;
> case 0x59: /* subtitling descriptor */
> language[0] = get8(pp, desc_end);
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index 1e2d5cc..243654f 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -373,21 +373,56 @@ static void mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
> break;
> case AVMEDIA_TYPE_SUBTITLE:
> {
> - const char *language;
> - language = lang && strlen(lang->value)==3 ? lang->value : "eng";
> - *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 */
> - }
> + char default_language[] ="eng";
nit: =" missing space
Default should be "und" for undetermined.
> + 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 */
> + }
> + } else if (st->codec->codec_id == AV_CODEC_ID_DVB_TELETEXT) {
> + uint8_t *len_ptr = NULL;
> + int extradata_copied = 0;
> +
> + /* The descriptor tag. teletext_descriptor */
> + *q++ = 0x56;
> + len_ptr = q++;
> +
> + while (*language != '\0') {
strlen(language) >= 3
> + *q++ = *language++;
> + *q++ = *language++;
> + *q++ = *language++;
> + /* Skip comma */
> + if (*language != '\0')
> + language++;
> +
> + if (st->codec->extradata_size > extradata_copied) {
I'd rather check for extradata_size - 1 > extradata_copied because you
access two bytes...
> + memcpy(q, st->codec->extradata + extradata_copied, 2);
> + extradata_copied += 2;
> + q += 2;
> + } else {
> + /* The Teletext descriptor:
> + * teletext_type: This 5-bit field indicates the type of Teletext page indicated. (0x01 Initial Teletext page)
> + * teletext_magazine_number: This is a 3-bit field which identifies the magazine number.
> + * teletext_page_number: This is an 8-bit field giving two 4-bit hex digits identifying the page number. */
> + *q++ = 0x08;
> + *q++ = 0x00;
> + }
> + }
> +
> + *len_ptr = q - len_ptr - 1;
> + }
> }
> break;
> case AVMEDIA_TYPE_VIDEO:
> @@ -859,7 +894,7 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
> MpegTSWrite *ts = s->priv_data;
> uint8_t buf[TS_PACKET_SIZE];
> uint8_t *q;
> - int val, is_start, len, header_len, write_pcr, is_dvb_subtitle, flags;
> + int val, is_start, len, header_len, write_pcr, is_dvb_subtitle, is_dvb_teletext, flags;
> int afc_len, stuffing_len;
> int64_t pcr = -1; /* avoid warning */
> int64_t delay = av_rescale(s->max_delay, 90000, AV_TIME_BASE);
> @@ -923,11 +958,13 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
> }
> if (is_start) {
> int pes_extension = 0;
> + int pes_header_stuffing_bytes = 0;
> /* write PES header */
> *q++ = 0x00;
> *q++ = 0x00;
> *q++ = 0x01;
> is_dvb_subtitle = 0;
> + is_dvb_teletext = 0;
> if (st->codec->codec_type == AVMEDIA_TYPE_VIDEO) {
> if (st->codec->codec_id == AV_CODEC_ID_DIRAC) {
> *q++ = 0xfd;
> @@ -944,9 +981,12 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
> *q++ = 0xfd;
> } else {
> *q++ = 0xbd;
> - if (st->codec->codec_type == AVMEDIA_TYPE_SUBTITLE &&
> - st->codec->codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
> - is_dvb_subtitle = 1;
> + if(st->codec->codec_type == AVMEDIA_TYPE_SUBTITLE) {
> + if (st->codec->codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
> + is_dvb_subtitle = 1;
> + } else if (st->codec->codec_id == AV_CODEC_ID_DVB_TELETEXT) {
> + is_dvb_teletext = 1;
> + }
> }
> }
> header_len = 0;
> @@ -983,6 +1023,10 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
> flags |= 0x01;
> header_len += 3;
> }
> + if (is_dvb_teletext) {
> + pes_header_stuffing_bytes = 0x24 - header_len;
> + header_len = 0x24;
> + }
> len = payload_size + header_len + 3;
> /* 3 extra bytes should be added to DVB subtitle payload: 0x20 0x00 at the beginning and trailing 0xff */
> if (is_dvb_subtitle) {
> @@ -1036,6 +1080,9 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
> *q++ = 0x20;
> *q++ = 0x00;
> }
> + if (is_dvb_teletext)
> + for (int i = 0; i < pes_header_stuffing_bytes; i++)
I think inline declarations should not be used in ffmpeg.
> + *q++ = 0xff;
> is_start = 0;
> }
> /* header size */
> --
> 1.8.3.2
Regards,
Marton
More information about the ffmpeg-devel
mailing list