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

[Michael Niedermayer]

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 at 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.

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170831/631fbb2d/attachment.sig>