[FFmpeg-devel] [PATCH] avformat/avc: write the missing bits in the AVC Decoder Configuration Box

[James Almer]

On 11/26/2019 2:32 PM, Carl Eugen Hoyos wrote:
> Am Di., 26. Nov. 2019 um 14:07 Uhr schrieb James Almer <jamrial at gmail.com>:
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavformat/avc.c | 38 +++++++++++++++++++++++++++++++++-----
>>  libavformat/avc.h |  1 +
>>  2 files changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/avc.c b/libavformat/avc.c
>> index a041e84357..9bd215c07f 100644
>> --- a/libavformat/avc.c
>> +++ b/libavformat/avc.c
>> @@ -107,11 +107,11 @@ int ff_avc_parse_nal_units_buf(const uint8_t *buf_in, uint8_t **buf, int *size)
>>
>>  int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>>  {
>> -    AVIOContext *sps_pb = NULL, *pps_pb = NULL;
>> +    AVIOContext *sps_pb = NULL, *pps_pb = NULL, *sps_ext_pb = NULL;
>>      uint8_t *buf = NULL, *end, *start = NULL;
>> -    uint8_t *sps = NULL, *pps = NULL;
>> -    uint32_t sps_size = 0, pps_size = 0;
>> -    int ret, nb_sps = 0, nb_pps = 0;
>> +    uint8_t *sps = NULL, *pps = NULL, *sps_ext = NULL;
>> +    uint32_t sps_size = 0, pps_size = 0, sps_ext_size = 0;
>> +    int ret, nb_sps = 0, nb_pps = 0, nb_sps_ext = 0;
>>
>>      if (len <= 6)
>>          return AVERROR_INVALIDDATA;
>> @@ -133,6 +133,9 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>>      if (ret < 0)
>>          goto fail;
>>      ret = avio_open_dyn_buf(&pps_pb);
>> +    if (ret < 0)
>> +        goto fail;
>> +    ret = avio_open_dyn_buf(&sps_ext_pb);
>>      if (ret < 0)
>>          goto fail;
>>
>> @@ -160,12 +163,21 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>>              }
>>              avio_wb16(pps_pb, size);
>>              avio_write(pps_pb, buf, size);
>> +        } else if (nal_type == 13) { /* SPS_EXT */
>> +            nb_sps_ext++;
>> +            if (size > UINT16_MAX || nb_sps_ext >= 256) {
>> +                ret = AVERROR_INVALIDDATA;
>> +                goto fail;
>> +            }
>> +            avio_wb16(sps_ext_pb, size);
>> +            avio_write(sps_ext_pb, buf, size);
>>          }
>>
>>          buf += size;
>>      }
>>      sps_size = avio_close_dyn_buf(sps_pb, &sps);
>>      pps_size = avio_close_dyn_buf(pps_pb, &pps);
>> +    sps_ext_size = avio_close_dyn_buf(sps_ext_pb, &sps_ext);
>>
>>      if (sps_size < 6 || !pps_size) {
>>          ret = AVERROR_INVALIDDATA;
>> @@ -183,13 +195,29 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>>      avio_w8(pb, nb_pps); /* number of pps */
>>      avio_write(pb, pps, pps_size);
>>
>> +    if (sps[3] != 66 && sps[3] != 77 && sps[3] != 88) {
>> +        H264SequenceParameterSet *seq = ff_avc_decode_sps(sps, sps_size);
>> +        if (!seq)
>> +            goto fail;
>> +        avio_w8(pb, 0xfc | seq->chroma_format_idc); /* 6 bits reserved (111111) + chroma_format_idc */
>> +        avio_w8(pb, 0xf8 | seq->bit_depth_luma - 8); /* 5 bits reserved (11111) + bit_depth_luma_minus8 */
>> +        avio_w8(pb, 0xf8 | seq->bit_depth_chroma - 8); /* 5 bits reserved (11111) + bit_depth_chroma_minus8 */
>> +        avio_w8(pb, nb_sps_ext); /* number of sps ext */
>> +        if (nb_sps_ext)
>> +            avio_write(pb, sps_ext, sps_ext_size);
>> +        av_free(seq);
>> +    }
>> +
>>  fail:
>>      if (!sps)
>>          avio_close_dyn_buf(sps_pb, &sps);
>>      if (!pps)
>>          avio_close_dyn_buf(pps_pb, &pps);
>> +    if (!sps_ext)
>> +        avio_close_dyn_buf(sps_ext_pb, &sps_ext);
>>      av_free(sps);
>>      av_free(pps);
>> +    av_free(sps_ext);
>>      av_free(start);
>>
>>      return ret;
>> @@ -351,7 +379,7 @@ H264SequenceParameterSet *ff_avc_decode_sps(const uint8_t *buf, int buf_size)
>>              skip_bits1(&gb); // separate_colour_plane_flag
>>          }
>>          sps->bit_depth_luma = get_ue_golomb(&gb) + 8;
>> -        get_ue_golomb(&gb); // bit_depth_chroma_minus8
>> +        sps->bit_depth_chroma = get_ue_golomb(&gb) + 8;
>>          skip_bits1(&gb); // qpprime_y_zero_transform_bypass_flag
>>          if (get_bits1(&gb)) { // seq_scaling_matrix_present_flag
>>              for (i = 0; i < ((sps->chroma_format_idc != 3) ? 8 : 12); i++) {
>> diff --git a/libavformat/avc.h b/libavformat/avc.h
>> index a79bf9b2db..5286d19d89 100644
>> --- a/libavformat/avc.h
>> +++ b/libavformat/avc.h
>> @@ -43,6 +43,7 @@ typedef struct {
>>      uint8_t constraint_set_flags;
>>      uint8_t chroma_format_idc;
>>      uint8_t bit_depth_luma;
>> +    uint8_t bit_depth_chroma;
>>      uint8_t frame_mbs_only_flag;
>>      AVRational sar;
>>  } H264SequenceParameterSet;
> 
> Sorry if this is obvious:
> What does this patch fix?
> 
> Carl Eugen

It adds the remaining portion of the avcC box as defined in the spec.
Some parsers complain when the box ends abruptly, but most don't because
they simply stop parsing after the pps part and skip the rest, so nobody
noticed it.