diff mbox

[FFmpeg-devel] avformat/mpegts: opus muxing & demuxing expanded

Message ID ae657c02-2cfc-ef71-56cf-6d829775ad91@gmail.com
State Superseded
Headers show

Commit Message

pkv.stream Aug. 29, 2017, 9:53 p.m. UTC
Hello again (patch attached this time),

this patch expands support for mapping_family 255 (and part of 1) for 
mpegts muxer and demuxer.

(Ambisonics family 2 & 3 not covered).

See commit commentary.

My case use (as an example) is that I needed to broadcast in mpeg-ts 
with 16 audio channels coming from SDI, corresponding to 16 different 
languages. Opus supports up to 255 channels but the mpegts muxer and 
demuxer allowed only up to 8 channels.

Thanks for all reviews and opinion.
From e0e8edb1173539deb653081209d6b7e4bf94b1c1 Mon Sep 17 00:00:00 2001
From: pkviet <pkv.stream@gmail.com>
Date: Tue, 29 Aug 2017 01:24:12 +0200
Subject: [PATCH] avformat/mpegts: opus muxing & demuxing expanded

Support for opus in mpegts demuxer was brought by commit
9cfa68c560bdec82d2d5ec079f9c5b0f9ca37af0 (Kieran Kunhya).
Support for opus in mpegts muxer was then added by commit
01509cdf9287b975eced1fd609a8201fbd1438e3 (S. Droge).
Later commit 37941878f193a2316c514bd5ba55bfe9d2dfdfcf by Michael Graczyk
added support of mapping_family encoder parameter which allows for up to 255
audio channels (for family 255).
While matroska muxer & demuxer readily accepts mapping_family, it is not the
case for mpegts muxer & demuxer for all the range of the parameter (family 255
and also part of family 1 with channel_config_code > 0x81 unsupported).
This commit brings such a support.
---
 libavcodec/libopusenc.c |  2 +-
 libavformat/mpegts.c    | 54 ++++++++++++++++++++++++++++++++++++++++++++++---
 libavformat/mpegtsenc.c | 19 +++++++++++------
 3 files changed, 65 insertions(+), 10 deletions(-)

Comments

Michael Niedermayer Aug. 31, 2017, 7:47 p.m. UTC | #1
On Tue, Aug 29, 2017 at 11:53:57PM +0200, pkv.stream wrote:
> Hello again (patch attached this time),
> 
> this patch expands support for mapping_family 255 (and part of 1)
> for mpegts muxer and demuxer.
> 
> (Ambisonics family 2 & 3 not covered).
> 
> See commit commentary.
> 
> My case use (as an example) is that I needed to broadcast in mpeg-ts
> with 16 audio channels coming from SDI, corresponding to 16
> different languages. Opus supports up to 255 channels but the mpegts
> muxer and demuxer allowed only up to 8 channels.
> 
> Thanks for all reviews and opinion.
> 

>  libavcodec/libopusenc.c |    2 -
>  libavformat/mpegts.c    |   54 +++++++++++++++++++++++++++++++++++++++++++++---
>  libavformat/mpegtsenc.c |   19 +++++++++++-----
>  3 files changed, 65 insertions(+), 10 deletions(-)
> ad6985f34e120426fb3d34f916d7984b75d32fd3  0001-avformat-mpegts-opus-muxing-demuxing-expanded.patch
> From e0e8edb1173539deb653081209d6b7e4bf94b1c1 Mon Sep 17 00:00:00 2001
> From: pkviet <pkv.stream@gmail.com>
> Date: Tue, 29 Aug 2017 01:24:12 +0200
> Subject: [PATCH] avformat/mpegts: opus muxing & demuxing expanded
> 
> Support for opus in mpegts demuxer was brought by commit
> 9cfa68c560bdec82d2d5ec079f9c5b0f9ca37af0 (Kieran Kunhya).
> Support for opus in mpegts muxer was then added by commit
> 01509cdf9287b975eced1fd609a8201fbd1438e3 (S. Droge).
> Later commit 37941878f193a2316c514bd5ba55bfe9d2dfdfcf by Michael Graczyk
> added support of mapping_family encoder parameter which allows for up to 255
> audio channels (for family 255).
> While matroska muxer & demuxer readily accepts mapping_family, it is not the
> case for mpegts muxer & demuxer for all the range of the parameter (family 255
> and also part of family 1 with channel_config_code > 0x81 unsupported).
> This commit brings such a support.
> ---
>  libavcodec/libopusenc.c |  2 +-
>  libavformat/mpegts.c    | 54 ++++++++++++++++++++++++++++++++++++++++++++++---
>  libavformat/mpegtsenc.c | 19 +++++++++++------
>  3 files changed, 65 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
> index c40fcde..77d8310 100644
> --- a/libavcodec/libopusenc.c
> +++ b/libavcodec/libopusenc.c
> @@ -368,7 +368,7 @@ static av_cold int libopus_encode_init(AVCodecContext *avctx)
>          goto fail;
>      }
>  
> -    /* Header includes channel mapping table if and only if mapping family is 0 */
> +    /* Header includes channel mapping table if and only if mapping family is NOT 0 */
>      header_size = 19 + (mapping_family == 0 ? 0 : 2 + avctx->channels);
>      avctx->extradata = av_malloc(header_size + AV_INPUT_BUFFER_PADDING_SIZE);
>      if (!avctx->extradata) {

This apears unrelated and should thus be in a seperate patch



> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 4d2f5c6..8c2349b 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -1633,7 +1633,7 @@ static const uint8_t opus_stream_cnt[9] = {
>      1, 1, 1, 2, 2, 3, 4, 4, 5,
>  };
>  
> -static const uint8_t opus_channel_map[8][8] = {
> +static const uint8_t opus_channel_map_a[8][8] = {
>      { 0 },
>      { 0,1 },
>      { 0,2,1 },
> @@ -1644,6 +1644,17 @@ static const uint8_t opus_channel_map[8][8] = {
>      { 0,6,1,2,3,4,5,7 },
>  };
>  
> +static const uint8_t opus_channel_map_b[8][8] = {
> +    { 0 },
> +    { 0,1 },
> +    { 0,1,2 },
> +    { 0,1,2,3 },
> +    { 0,1,2,3,4 },
> +    { 0,1,2,3,4,5 },
> +    { 0,1,2,3,4,5,6 },
> +    { 0,1,2,3,4,5,6,7 },
> +};
> +
>  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type,
>                                const uint8_t **pp, const uint8_t *desc_list_end,
>                                Mp4Descr *mp4_descr, int mp4_descr_count, int pid,
> @@ -1887,9 +1898,46 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
>                      st->codecpar->extradata[18] = channel_config_code ? (channels > 2) : /* Dual Mono */ 255;
>                      st->codecpar->extradata[19] = opus_stream_cnt[channel_config_code];
>                      st->codecpar->extradata[20] = opus_coupled_stream_cnt[channel_config_code];
> -                    memcpy(&st->codecpar->extradata[21], opus_channel_map[channels - 1], channels);
> +                    memcpy(&st->codecpar->extradata[21], opus_channel_map_a[channels - 1], channels);
>                  } else {
> -                    avpriv_request_sample(fc, "Opus in MPEG-TS - channel_config_code > 0x8");
> +                    if (channel_config_code == 0x81) {
> +                        channels = get8(pp, desc_end);
> +                        st->codecpar->extradata_size = 22 + channels;
> +                        int *extradata = av_malloc((22 + channels) * sizeof(int));
> +                        for (int i = 0; i <= (22+channels); i++) {
> +                            if (i < 9) {
> +                                extradata[i] = opus_default_extradata[i];
> +                            }
> +                            else {
> +                                extradata[i] = 0;
> +                            }
> +                        }

This is missing malloc failure checks, also the "for (int i" syntax is
something we avoid due to some at least past portability issues.

[...]
pkv.stream Aug. 31, 2017, 8:19 p.m. UTC | #2
> This is missing malloc failure checks, also the "for (int i" syntax is
> something we avoid due to some at least past portability issues.

thanks a lot. Will fix.
diff mbox

Patch

diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
index c40fcde..77d8310 100644
--- a/libavcodec/libopusenc.c
+++ b/libavcodec/libopusenc.c
@@ -368,7 +368,7 @@  static av_cold int libopus_encode_init(AVCodecContext *avctx)
         goto fail;
     }
 
-    /* Header includes channel mapping table if and only if mapping family is 0 */
+    /* Header includes channel mapping table if and only if mapping family is NOT 0 */
     header_size = 19 + (mapping_family == 0 ? 0 : 2 + avctx->channels);
     avctx->extradata = av_malloc(header_size + AV_INPUT_BUFFER_PADDING_SIZE);
     if (!avctx->extradata) {
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 4d2f5c6..8c2349b 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -1633,7 +1633,7 @@  static const uint8_t opus_stream_cnt[9] = {
     1, 1, 1, 2, 2, 3, 4, 4, 5,
 };
 
-static const uint8_t opus_channel_map[8][8] = {
+static const uint8_t opus_channel_map_a[8][8] = {
     { 0 },
     { 0,1 },
     { 0,2,1 },
@@ -1644,6 +1644,17 @@  static const uint8_t opus_channel_map[8][8] = {
     { 0,6,1,2,3,4,5,7 },
 };
 
+static const uint8_t opus_channel_map_b[8][8] = {
+    { 0 },
+    { 0,1 },
+    { 0,1,2 },
+    { 0,1,2,3 },
+    { 0,1,2,3,4 },
+    { 0,1,2,3,4,5 },
+    { 0,1,2,3,4,5,6 },
+    { 0,1,2,3,4,5,6,7 },
+};
+
 int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type,
                               const uint8_t **pp, const uint8_t *desc_list_end,
                               Mp4Descr *mp4_descr, int mp4_descr_count, int pid,
@@ -1887,9 +1898,46 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
                     st->codecpar->extradata[18] = channel_config_code ? (channels > 2) : /* Dual Mono */ 255;
                     st->codecpar->extradata[19] = opus_stream_cnt[channel_config_code];
                     st->codecpar->extradata[20] = opus_coupled_stream_cnt[channel_config_code];
-                    memcpy(&st->codecpar->extradata[21], opus_channel_map[channels - 1], channels);
+                    memcpy(&st->codecpar->extradata[21], opus_channel_map_a[channels - 1], channels);
                 } else {
-                    avpriv_request_sample(fc, "Opus in MPEG-TS - channel_config_code > 0x8");
+                    if (channel_config_code == 0x81) {
+                        channels = get8(pp, desc_end);
+                        st->codecpar->extradata_size = 22 + channels;
+                        int *extradata = av_malloc((22 + channels) * sizeof(int));
+                        for (int i = 0; i <= (22+channels); i++) {
+                            if (i < 9) {
+                                extradata[i] = opus_default_extradata[i];
+                            }
+                            else {
+                                extradata[i] = 0;
+                            }
+                        }
+                        memcpy(st->codecpar->extradata, extradata, sizeof(extradata));
+                        av_free(extradata);
+                        st->codecpar->extradata[9] = channels;
+                        st->codecpar->extradata[18] = 255;
+                        st->codecpar->extradata[19] = channels;
+                        st->codecpar->extradata[20] = 0;
+                        int *opus_channel_map255 = av_malloc(channels * sizeof(int));
+                        for (uint8_t i = 0; i < channels; i++) {
+                            opus_channel_map255[i] = i;
+                            }
+                        memcpy(&st->codecpar->extradata[21], opus_channel_map255, channels);
+                        av_free(opus_channel_map255);
+                    } else {
+                        if ((channel_config_code >= 0x82) && (channel_config_code <= 0x88)) {
+                            channels = get8(pp, desc_end);
+                            st->codecpar->extradata[9] = channels;
+                            st->codecpar->extradata[18] = 1;
+                            st->codecpar->extradata[19] = channels;
+                            st->codecpar->extradata[20] = 0;
+                            memcpy(&st->codecpar->extradata[21], opus_channel_map_b[channels - 1], channels);
+                        }
+                        else {
+                            avpriv_request_sample(fc, "Opus in MPEG-TS - channel_config_code");
+                        }
+
+                    }
                 }
                 st->need_parsing = AVSTREAM_PARSE_FULL;
                 st->internal->need_context_update = 1;
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index fdfa544..af1dfc6 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -421,8 +421,8 @@  static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
                 *q++ = 'D';
             }
             if (st->codecpar->codec_id==AV_CODEC_ID_OPUS) {
-                /* 6 bytes registration descriptor, 4 bytes Opus audio descriptor */
-                if (q - data > SECTION_LENGTH - 6 - 4) {
+                /* 6 bytes registration descriptor, 6 bytes Opus audio descriptor */
+                if (q - data > SECTION_LENGTH - 6 - 6) {
                     err = 1;
                     break;
                 }
@@ -435,7 +435,7 @@  static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
                 *q++ = 's';
 
                 *q++ = 0x7f; /* DVB extension descriptor */
-                *q++ = 2;
+                *q++ = 4;
                 *q++ = 0x80;
 
                 if (st->codecpar->extradata && st->codecpar->extradata_size >= 19) {
@@ -483,9 +483,14 @@  static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
                             *q++ = 0xff;
                         }
                     } else {
-                        /* Unsupported */
-                        av_log(s, AV_LOG_ERROR, "Unsupported Opus channel mapping for family %d", st->codecpar->extradata[18]);
-                        *q++ = 0xff;
+                        /* mapping family 255 , set channel_config_code to 0x81 */
+                        if (st->codecpar->extradata[18] == 255) {
+                            *q++ = 0x81;
+                        } else {
+                            /* Unsupported */
+                            av_log(s, AV_LOG_ERROR, "Unsupported Opus channel mapping for family %d", st->codecpar->extradata[18]);
+                            *q++ = 0xff;
+                        }
                     }
                 } else if (st->codecpar->channels <= 2) {
                     /* Assume RTP mapping family */
@@ -495,6 +500,8 @@  static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
                     av_log(s, AV_LOG_ERROR, "Unsupported Opus channel mapping");
                     *q++ = 0xff;
                 }
+                *q++ = st->codecpar->extradata[9];
+                *q++ = st->codecpar->extradata[18];
             }
 
             if (lang) {