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

[James Almer]

On 11/27/2019 9:26 AM, James Almer wrote:
> On 11/26/2019 10:57 AM, Andreas Rheinhardt wrote:
>> On Tue, Nov 26, 2019 at 2:07 PM James Almer <jamrial at gmail.com> wrote:
>>
>>> 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);
>>>
>>
>> I don't like how these buffers are freed. How about the following approach:
>> You don't close the dynamic buffer above; instead you just call
>> avio_get_dyn_buf(). And here on fail you use ffio_free_dyn_buf()
>> unconditionally instead of these combinations of avio_close_dyn_buf() and
>> av_free().
> 
> Ok, will change that and apply this soon.

Changed and applied.