[FFmpeg-devel] [PATCH v4 0/2 ] avformat/mpegts: opus muxing & demuxing for mapping family 255

[pkv.stream]

>>   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,56 @@ 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;
>> +                        size_t extradata_size;
>> +                        extradata_size = (22 + channels) * sizeof(uint8_t);
>> +                        uint8_t *extradata;
> this produces warnings:
> libavformat/mpegts.c:1906:25: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
>

Thanks a lot Michael for your review.
I have cleaned this warning and others issued by gcc.

>> +                        extradata = av_malloc(extradata_size);
>> +                        if (!extradata)
>> +                            return AVERROR(ENOMEM);
>> +                        for (i = 0; i <= (22+channels); i++) {
> the extradata_size expression is repeated 3 times, code duplication
> should be avoided
>
removed
>> +                            if (i < 9) {
>> +                                extradata[i] = opus_default_extradata[i];
>> +                            }
>> +                            else {
>> +                                extradata[i] = 0;
>> +                            }
>> +                        }
>> +                        memcpy(st->codecpar->extradata, extradata, sizeof(extradata));
> this looks wrong
>
right, fixed (removed)

>> +                        av_free(extradata);
>> +                        st->codecpar->extradata[9] = channels;
>> +                        st->codecpar->extradata[18] = 255;
>> +                        st->codecpar->extradata[19] = channels;
>> +                        st->codecpar->extradata[20] = 0;
>> +                        size_t channel_map_size = channels * sizeof(uint8_t);
>> +                        uint8_t *opus_channel_map255;
>> +                        opus_channel_map255 = av_malloc(channel_map_size);
>> +                        if (!opus_channel_map255)
>> +                            return AVERROR(ENOMEM);
>> +                        uint8_t j;
>> +                        for (j = 0; j < channels; j++) {
>> +                            opus_channel_map255[j] = j;
>> +                        }
>> +                        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);
> the channels value is not checked before use in memcpy

fixed;
I have added other checks for all the data read from header;
>
>> +                        }
>> +                        else {
>> +                            avpriv_request_sample(fc, "Opus in MPEG-TS - channel_config_code %i", 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
> muxer and demuxer changes should be in seperate patches

done; see next posts.

My previous code was not following strictly the draft spec here:
https://people.xiph.org/~tterribe/opus/ETSI_TS_opus-v0.1.3-draft.doc
So I have rewritten it to have strict adherence regarding the bits 
allocated in header.

I have also run fate + patcheck ; no issues (just some unused variables 
warnings for variables antedating this patch).

Regards