"zhilizhao(赵志立)":
>
>
>> On Aug 16, 2021, at 11:19 AM, James Almer <jamr...@gmail.com> wrote:
>>
>> On 8/16/2021 12:08 AM, "zhilizhao(赵志立)" wrote:
>>>> On Aug 16, 2021, at 2:25 AM, Nachiket Tarate
>>>> <nachiket.program...@gmail.com> wrote:
>>>>
>>>> These will be used by HLS demuxer in case of sample decryption.
>>>>
>>>> Signed-off-by: Nachiket Tarate <nachiket.program...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
