[FFmpeg-devel,1/4] libavcodec/adts_header: add frame_length field and avpriv function to parse AAC ADTS header

These will be used by HLS demuxer in case of sample decryption.

Signed-off-by: Nachiket Tarate <nachiket.programmer@gmail.com>
---
 libavcodec/adts_header.c |  1 +
 libavcodec/adts_header.h | 14 ++++++++++++++
 libavcodec/adts_parser.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

> 2021年8月16日 上午2:25，Nachiket Tarate <nachiket.programmer@gmail.com> 写道：
> 
> These will be used by HLS demuxer in case of sample decryption.
> 
> Signed-off-by: Nachiket Tarate <nachiket.programmer@gmail.com>
> ---
> libavcodec/adts_header.c |  1 +
> libavcodec/adts_header.h | 14 ++++++++++++++
> libavcodec/adts_parser.c | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 47 insertions(+)
> 
> diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
> index 0889820f8a..e4454529c4 100644
> --- a/libavcodec/adts_header.c
> +++ b/libavcodec/adts_header.c
> @@ -66,6 +66,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
>     hdr->sample_rate    = avpriv_mpeg4audio_sample_rates[sr];
>     hdr->samples        = (rdb + 1) * 1024;
>     hdr->bit_rate       = size * 8 * hdr->sample_rate / hdr->samples;
> +    hdr->frame_length   = size;
> 
>     return size;
> }
> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
> index f615f6a9f9..9ecd67fb5b 100644
> --- a/libavcodec/adts_header.h
> +++ b/libavcodec/adts_header.h
> @@ -34,6 +34,7 @@ typedef struct AACADTSHeaderInfo {
>     uint8_t  sampling_index;
>     uint8_t  chan_config;
>     uint8_t  num_aac_frames;
> +    uint32_t frame_length;
> } AACADTSHeaderInfo;
> 
> /**
> @@ -47,4 +48,17 @@ typedef struct AACADTSHeaderInfo {
>  */
> int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
> 
> +/**
> + * Parse the ADTS frame header contained in the buffer, which is
> + * the first 54 bits.
> + * @param[in]  buf  Pointer to buffer containing the first 54 bits of the frame.
> + * @param[in]  size Size of buffer containing the first 54 bits of the frame.
> + * @param[out] phdr Pointer to pointer to struct AACADTSHeaderInfo for which
> + * memory is allocated and header info is written into it.
> + * @return Returns 0 on success, -1 if there is a sync word mismatch,
> + * -2 if the version element is invalid, -3 if the sample rate
> + * element is invalid, or -4 if the bit rate element is invalid.
> + */
> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size);
> +
> #endif /* AVCODEC_ADTS_HEADER_H */
> diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
> index 5c9f8ff6f2..32489dadf4 100644
> --- a/libavcodec/adts_parser.c
> +++ b/libavcodec/adts_parser.c
> @@ -42,3 +42,35 @@ int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
>     return AVERROR(ENOSYS);
> #endif
> }
> +
> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size)
> +{
> +#if CONFIG_ADTS_HEADER
> +    int ret = 0;
> +    GetBitContext gb;
> +
> +    if (size < AV_AAC_ADTS_HEADER_SIZE)
> +        return AVERROR_INVALIDDATA;
> +
> +    if (!*phdr)
> +        *phdr = av_mallocz(sizeof(AACADTSHeaderInfo));
> +    if (!*phdr)
> +        return AVERROR(ENOMEM);
> +
> +    ret = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
> +    if (ret < 0) {
> +        av_free(*phdr);
> +        return ret;
> +    }
> +
> +    ret = ff_adts_header_parse(&gb, *phdr);
> +    if (ret < 0) {
> +        av_free(*phdr);
> +        return ret;
> +    }
> +
> +    return 0;
> +#else
> +    return AVERROR(ENOSYS);
> +#endif
> +}
> -- 
> 2.17.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 

Looks ok to me, will fix some code style problem locally and push one week after if there have no objections.

Thanks

Steven Liu

> On Aug 16, 2021, at 2:25 AM, Nachiket Tarate <nachiket.programmer@gmail.com> wrote:
> 
> These will be used by HLS demuxer in case of sample decryption.
> 
> Signed-off-by: Nachiket Tarate <nachiket.programmer@gmail.com>
> ---
> libavcodec/adts_header.c |  1 +
> libavcodec/adts_header.h | 14 ++++++++++++++
> libavcodec/adts_parser.c | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 47 insertions(+)
> 
> diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
> index 0889820f8a..e4454529c4 100644
> --- a/libavcodec/adts_header.c
> +++ b/libavcodec/adts_header.c
> @@ -66,6 +66,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
>     hdr->sample_rate    = avpriv_mpeg4audio_sample_rates[sr];
>     hdr->samples        = (rdb + 1) * 1024;
>     hdr->bit_rate       = size * 8 * hdr->sample_rate / hdr->samples;
> +    hdr->frame_length   = size;
> 
>     return size;
> }
> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
> index f615f6a9f9..9ecd67fb5b 100644
> --- a/libavcodec/adts_header.h
> +++ b/libavcodec/adts_header.h
> @@ -34,6 +34,7 @@ typedef struct AACADTSHeaderInfo {
>     uint8_t  sampling_index;
>     uint8_t  chan_config;
>     uint8_t  num_aac_frames;
> +    uint32_t frame_length;
> } AACADTSHeaderInfo;
> 

The patch can be separated into two patches.

> /**
> @@ -47,4 +48,17 @@ typedef struct AACADTSHeaderInfo {
>  */
> int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
> 
> +/**
> + * Parse the ADTS frame header contained in the buffer, which is
> + * the first 54 bits.
> + * @param[in]  buf  Pointer to buffer containing the first 54 bits of the frame.
> + * @param[in]  size Size of buffer containing the first 54 bits of the frame.
> + * @param[out] phdr Pointer to pointer to struct AACADTSHeaderInfo for which
> + * memory is allocated and header info is written into it.
> + * @return Returns 0 on success, -1 if there is a sync word mismatch,
> + * -2 if the version element is invalid, -3 if the sample rate
> + * element is invalid, or -4 if the bit rate element is invalid.
> + */
> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size);
> +
> #endif /* AVCODEC_ADTS_HEADER_H */
> diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
> index 5c9f8ff6f2..32489dadf4 100644
> --- a/libavcodec/adts_parser.c
> +++ b/libavcodec/adts_parser.c
> @@ -42,3 +42,35 @@ int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
>     return AVERROR(ENOSYS);
> #endif
> }
> +
> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size)
> +{
> +#if CONFIG_ADTS_HEADER
> +    int ret = 0;
> +    GetBitContext gb;
> +
> +    if (size < AV_AAC_ADTS_HEADER_SIZE)
> +        return AVERROR_INVALIDDATA;
> +
> +    if (!*phdr)
> +        *phdr = av_mallocz(sizeof(AACADTSHeaderInfo));
> +    if (!*phdr)
> +        return AVERROR(ENOMEM);

I’m not keen about such design, that
1. you can allocate AACADTSHeaderInfo directly
2. if you don’t, I will allocate it for you

I’m not sure about the rule of avpriv_ part, in my opinion, the struct should
always allocated by libavcodec and used by libavformat. libavformat shouldn’t
allocate it on stack or heap by itself for ABI compatibility.

> +
> +    ret = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
> +    if (ret < 0) {
> +        av_free(*phdr);
> +        return ret;
> +    }
> +
> +    ret = ff_adts_header_parse(&gb, *phdr);
> +    if (ret < 0) {
> +        av_free(*phdr);

av_freep(phdr)

> +        return ret;
> +    }
> +
> +    return 0;
> +#else
> +    return AVERROR(ENOSYS);
> +#endif
> +}
> -- 
> 2.17.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

On 8/16/2021 12:08 AM, "zhilizhao(赵志立)" wrote:
> 
> 
>> On Aug 16, 2021, at 2:25 AM, Nachiket Tarate <nachiket.programmer@gmail.com> wrote:
>>
>> These will be used by HLS demuxer in case of sample decryption.
>>
>> Signed-off-by: Nachiket Tarate <nachiket.programmer@gmail.com>
>> ---
>> libavcodec/adts_header.c |  1 +
>> libavcodec/adts_header.h | 14 ++++++++++++++
>> libavcodec/adts_parser.c | 32 ++++++++++++++++++++++++++++++++
>> 3 files changed, 47 insertions(+)
>>
>> diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
>> index 0889820f8a..e4454529c4 100644
>> --- a/libavcodec/adts_header.c
>> +++ b/libavcodec/adts_header.c
>> @@ -66,6 +66,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
>>      hdr->sample_rate    = avpriv_mpeg4audio_sample_rates[sr];
>>      hdr->samples        = (rdb + 1) * 1024;
>>      hdr->bit_rate       = size * 8 * hdr->sample_rate / hdr->samples;
>> +    hdr->frame_length   = size;
>>
>>      return size;
>> }
>> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
>> index f615f6a9f9..9ecd67fb5b 100644
>> --- a/libavcodec/adts_header.h
>> +++ b/libavcodec/adts_header.h
>> @@ -34,6 +34,7 @@ typedef struct AACADTSHeaderInfo {
>>      uint8_t  sampling_index;
>>      uint8_t  chan_config;
>>      uint8_t  num_aac_frames;
>> +    uint32_t frame_length;
>> } AACADTSHeaderInfo;
>>
> 
> The patch can be separated into two patches.
> 
>> /**
>> @@ -47,4 +48,17 @@ typedef struct AACADTSHeaderInfo {
>>   */
>> int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
>>
>> +/**
>> + * Parse the ADTS frame header contained in the buffer, which is
>> + * the first 54 bits.
>> + * @param[in]  buf  Pointer to buffer containing the first 54 bits of the frame.
>> + * @param[in]  size Size of buffer containing the first 54 bits of the frame.
>> + * @param[out] phdr Pointer to pointer to struct AACADTSHeaderInfo for which
>> + * memory is allocated and header info is written into it.
>> + * @return Returns 0 on success, -1 if there is a sync word mismatch,
>> + * -2 if the version element is invalid, -3 if the sample rate
>> + * element is invalid, or -4 if the bit rate element is invalid.
>> + */
>> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size);
>> +
>> #endif /* AVCODEC_ADTS_HEADER_H */
>> diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
>> index 5c9f8ff6f2..32489dadf4 100644
>> --- a/libavcodec/adts_parser.c
>> +++ b/libavcodec/adts_parser.c
>> @@ -42,3 +42,35 @@ int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
>>      return AVERROR(ENOSYS);
>> #endif
>> }
>> +
>> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size)
>> +{
>> +#if CONFIG_ADTS_HEADER
>> +    int ret = 0;
>> +    GetBitContext gb;
>> +
>> +    if (size < AV_AAC_ADTS_HEADER_SIZE)
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    if (!*phdr)
>> +        *phdr = av_mallocz(sizeof(AACADTSHeaderInfo));
>> +    if (!*phdr)
>> +        return AVERROR(ENOMEM);
> 
> I’m not keen about such design, that
> 1. you can allocate AACADTSHeaderInfo directly
> 2. if you don’t, I will allocate it for you
> 
> I’m not sure about the rule of avpriv_ part, in my opinion, the struct should
> always allocated by libavcodec and used by libavformat. libavformat shouldn’t
> allocate it on stack or heap by itself for ABI compatibility.

It's copying the behavior of avpriv_ac3_parse_header(), which may be a 
remnant from a time where it was called from within libavcodec itself.
I agree it should not be allowed to pass a pre-allocated struct by 
another library for ABI compatibility reasons, but the only user 
Nachiket is adding is letting lavc allocate it, so it's not that important.

> 
>> +
>> +    ret = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
>> +    if (ret < 0) {
>> +        av_free(*phdr);
>> +        return ret;
>> +    }
>> +
>> +    ret = ff_adts_header_parse(&gb, *phdr);
>> +    if (ret < 0) {
>> +        av_free(*phdr);
> 
> av_freep(phdr)
> 
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +#else
>> +    return AVERROR(ENOSYS);
>> +#endif
>> +}
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

> On Aug 16, 2021, at 11:19 AM, James Almer <jamrial@gmail.com> wrote:
> 
> On 8/16/2021 12:08 AM, "zhilizhao(赵志立)" wrote:
>>> On Aug 16, 2021, at 2:25 AM, Nachiket Tarate <nachiket.programmer@gmail.com> wrote:
>>> 
>>> These will be used by HLS demuxer in case of sample decryption.
>>> 
>>> Signed-off-by: Nachiket Tarate <nachiket.programmer@gmail.com>
>>> ---
>>> libavcodec/adts_header.c |  1 +
>>> libavcodec/adts_header.h | 14 ++++++++++++++
>>> libavcodec/adts_parser.c | 32 ++++++++++++++++++++++++++++++++
>>> 3 files changed, 47 insertions(+)
>>> 
>>> diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
>>> index 0889820f8a..e4454529c4 100644
>>> --- a/libavcodec/adts_header.c
>>> +++ b/libavcodec/adts_header.c
>>> @@ -66,6 +66,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
>>>     hdr->sample_rate    = avpriv_mpeg4audio_sample_rates[sr];
>>>     hdr->samples        = (rdb + 1) * 1024;
>>>     hdr->bit_rate       = size * 8 * hdr->sample_rate / hdr->samples;
>>> +    hdr->frame_length   = size;
>>> 
>>>     return size;
>>> }
>>> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
>>> index f615f6a9f9..9ecd67fb5b 100644
>>> --- a/libavcodec/adts_header.h
>>> +++ b/libavcodec/adts_header.h
>>> @@ -34,6 +34,7 @@ typedef struct AACADTSHeaderInfo {
>>>     uint8_t  sampling_index;
>>>     uint8_t  chan_config;
>>>     uint8_t  num_aac_frames;
>>> +    uint32_t frame_length;
>>> } AACADTSHeaderInfo;
>>> 
>> The patch can be separated into two patches.
>>> /**
>>> @@ -47,4 +48,17 @@ typedef struct AACADTSHeaderInfo {
>>>  */
>>> int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
>>> 
>>> +/**
>>> + * Parse the ADTS frame header contained in the buffer, which is
>>> + * the first 54 bits.
>>> + * @param[in]  buf  Pointer to buffer containing the first 54 bits of the frame.
>>> + * @param[in]  size Size of buffer containing the first 54 bits of the frame.
>>> + * @param[out] phdr Pointer to pointer to struct AACADTSHeaderInfo for which
>>> + * memory is allocated and header info is written into it.
>>> + * @return Returns 0 on success, -1 if there is a sync word mismatch,
>>> + * -2 if the version element is invalid, -3 if the sample rate
>>> + * element is invalid, or -4 if the bit rate element is invalid.
>>> + */
>>> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size);
>>> +
>>> #endif /* AVCODEC_ADTS_HEADER_H */
>>> diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
>>> index 5c9f8ff6f2..32489dadf4 100644
>>> --- a/libavcodec/adts_parser.c
>>> +++ b/libavcodec/adts_parser.c
>>> @@ -42,3 +42,35 @@ int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
>>>     return AVERROR(ENOSYS);
>>> #endif
>>> }
>>> +
>>> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size)
>>> +{
>>> +#if CONFIG_ADTS_HEADER
>>> +    int ret = 0;
>>> +    GetBitContext gb;
>>> +
>>> +    if (size < AV_AAC_ADTS_HEADER_SIZE)
>>> +        return AVERROR_INVALIDDATA;
>>> +
>>> +    if (!*phdr)
>>> +        *phdr = av_mallocz(sizeof(AACADTSHeaderInfo));
>>> +    if (!*phdr)
>>> +        return AVERROR(ENOMEM);
>> I’m not keen about such design, that
>> 1. you can allocate AACADTSHeaderInfo directly
>> 2. if you don’t, I will allocate it for you
>> I’m not sure about the rule of avpriv_ part, in my opinion, the struct should
>> always allocated by libavcodec and used by libavformat. libavformat shouldn’t
>> allocate it on stack or heap by itself for ABI compatibility.
> 
> It's copying the behavior of avpriv_ac3_parse_header(), which may be a remnant from a time where it was called from within libavcodec itself.
> I agree it should not be allowed to pass a pre-allocated struct by another library for ABI compatibility reasons, but the only user Nachiket is adding is letting lavc allocate it, so it's not that important.

The commit message says: "These will be used by HLS demuxer in case of sample decryption.” I thought it will be used by libavformat.

> 
>>> +
>>> +    ret = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
>>> +    if (ret < 0) {
>>> +        av_free(*phdr);
>>> +        return ret;
>>> +    }
>>> +
>>> +    ret = ff_adts_header_parse(&gb, *phdr);
>>> +    if (ret < 0) {
>>> +        av_free(*phdr);
>> av_freep(phdr)
>>> +        return ret;
>>> +    }
>>> +
>>> +    return 0;
>>> +#else
>>> +    return AVERROR(ENOSYS);
>>> +#endif
>>> +}
>>> -- 
>>> 2.17.1
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

"zhilizhao(赵志立)":
> 
> 
>> On Aug 16, 2021, at 11:19 AM, James Almer <jamrial@gmail.com> wrote:
>>
>> On 8/16/2021 12:08 AM, "zhilizhao(赵志立)" wrote:
>>>> On Aug 16, 2021, at 2:25 AM, Nachiket Tarate <nachiket.programmer@gmail.com> wrote:
>>>>
>>>> These will be used by HLS demuxer in case of sample decryption.
>>>>
>>>> Signed-off-by: Nachiket Tarate <nachiket.programmer@gmail.com>
>>>> ---
>>>> libavcodec/adts_header.c |  1 +
>>>> libavcodec/adts_header.h | 14 ++++++++++++++
>>>> libavcodec/adts_parser.c | 32 ++++++++++++++++++++++++++++++++
>>>> 3 files changed, 47 insertions(+)
>>>>
>>>> diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
>>>> index 0889820f8a..e4454529c4 100644
>>>> --- a/libavcodec/adts_header.c
>>>> +++ b/libavcodec/adts_header.c
>>>> @@ -66,6 +66,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
>>>>     hdr->sample_rate    = avpriv_mpeg4audio_sample_rates[sr];
>>>>     hdr->samples        = (rdb + 1) * 1024;
>>>>     hdr->bit_rate       = size * 8 * hdr->sample_rate / hdr->samples;
>>>> +    hdr->frame_length   = size;
>>>>
>>>>     return size;
>>>> }
>>>> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
>>>> index f615f6a9f9..9ecd67fb5b 100644
>>>> --- a/libavcodec/adts_header.h
>>>> +++ b/libavcodec/adts_header.h
>>>> @@ -34,6 +34,7 @@ typedef struct AACADTSHeaderInfo {
>>>>     uint8_t  sampling_index;
>>>>     uint8_t  chan_config;
>>>>     uint8_t  num_aac_frames;
>>>> +    uint32_t frame_length;
>>>> } AACADTSHeaderInfo;
>>>>
>>> The patch can be separated into two patches.
>>>> /**
>>>> @@ -47,4 +48,17 @@ typedef struct AACADTSHeaderInfo {
>>>>  */
>>>> int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
>>>>
>>>> +/**
>>>> + * Parse the ADTS frame header contained in the buffer, which is
>>>> + * the first 54 bits.
>>>> + * @param[in]  buf  Pointer to buffer containing the first 54 bits of the frame.
>>>> + * @param[in]  size Size of buffer containing the first 54 bits of the frame.
>>>> + * @param[out] phdr Pointer to pointer to struct AACADTSHeaderInfo for which
>>>> + * memory is allocated and header info is written into it.
>>>> + * @return Returns 0 on success, -1 if there is a sync word mismatch,
>>>> + * -2 if the version element is invalid, -3 if the sample rate
>>>> + * element is invalid, or -4 if the bit rate element is invalid.
>>>> + */
>>>> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size);
>>>> +
>>>> #endif /* AVCODEC_ADTS_HEADER_H */
>>>> diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
>>>> index 5c9f8ff6f2..32489dadf4 100644
>>>> --- a/libavcodec/adts_parser.c
>>>> +++ b/libavcodec/adts_parser.c
>>>> @@ -42,3 +42,35 @@ int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
>>>>     return AVERROR(ENOSYS);
>>>> #endif
>>>> }
>>>> +
>>>> +int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size)
>>>> +{
>>>> +#if CONFIG_ADTS_HEADER
>>>> +    int ret = 0;
>>>> +    GetBitContext gb;
>>>> +
>>>> +    if (size < AV_AAC_ADTS_HEADER_SIZE)
>>>> +        return AVERROR_INVALIDDATA;
>>>> +
>>>> +    if (!*phdr)
>>>> +        *phdr = av_mallocz(sizeof(AACADTSHeaderInfo));
>>>> +    if (!*phdr)
>>>> +        return AVERROR(ENOMEM);
>>> I’m not keen about such design, that
>>> 1. you can allocate AACADTSHeaderInfo directly
>>> 2. if you don’t, I will allocate it for you
>>> I’m not sure about the rule of avpriv_ part, in my opinion, the struct should
>>> always allocated by libavcodec and used by libavformat. libavformat shouldn’t
>>> allocate it on stack or heap by itself for ABI compatibility.
>>
>> It's copying the behavior of avpriv_ac3_parse_header(), which may be a remnant from a time where it was called from within libavcodec itself.
>> I agree it should not be allowed to pass a pre-allocated struct by another library for ABI compatibility reasons, but the only user Nachiket is adding is letting lavc allocate it, so it's not that important.
> 
> The commit message says: "These will be used by HLS demuxer in case of sample decryption.” I thought it will be used by libavformat.
> 
Yes, it will be used by libavformat. But libavformat will let libavcodec
allocate it.
(IMO a better approach would be to not allocate it at all and equip
avpriv_adts_header_parse() with a parameter for
sizeof(AACADTSHeaderInfo) as known to the caller.
avpriv_adts_header_parse() will then copy the beginning of its possibly
larger context into the structure provided by the caller.)

- Andreas