________________________________
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> on behalf of James Almer
<jamr...@gmail.com>
Sent: Tuesday, October 6, 2020 8:47 PM
To: ffmpeg-devel@ffmpeg.org <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/adts_header: add frame_length
field and avpriv function to parse AAC ADTS header
On 10/6/2020 11:30 AM, Nachiket Tarate wrote:
> These will be used by HLS demuxer for SAMPLE-AES decryption.
>
> Signed-off-by: Nachiket Tarate <nachiket.tar...@outlook.com>
> ---
> libavcodec/adts_header.c | 18 ++++++++++++++++++
> libavcodec/adts_header.h | 4 ++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
> index 0889820f8a..006455b110 100644
> --- a/libavcodec/adts_header.c
> +++ b/libavcodec/adts_header.c
> @@ -3,6 +3,7 @@
> * Copyright (c) 2003 Fabrice Bellard
> * Copyright (c) 2003 Michael Niedermayer
> * Copyright (c) 2009 Alex Converse
> + * Copyright (c) 2020 Nachiket Tarate
> *
> * This file is part of FFmpeg.
> *
> @@ -66,6 +67,23 @@ 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;
> }
> +
> +int avpriv_adts_header_parse (AACADTSHeaderInfo *hdr, const uint8_t *buf,
> size_t size)
You're making sizeof(AACADTSHeaderInfo) part of the ABI this way, which
is not desirable. If you look at avpriv_ac3_parse_header(), it takes a
pointer to pointer to AC3HeaderInfo, and allocates it if needed.
Also, you should instead add this function to adts_parser.c and make it
return AVERROR(ENOSYS) when CONFIG_ADTS_HEADER is 0. This is done so the
symbol is always present even when the functionality is not.
Nachiket> I will do these changes and resubmit the patch.
I assume you're adding this function because you need AACADTSHeaderInfo
fields that av_adts_header_parse() alone does not provide, like
frame_length?
Nachiket > Yes. Your understanding is correct.
> +{
> + int ret = 0;
> + GetBitContext gb;
> + if (size < AV_AAC_ADTS_HEADER_SIZE)
> + return AVERROR_INVALIDDATA;
> + ret = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
> + if (ret < 0)
> + return ret;
> + ret = ff_adts_header_parse(&gb, hdr);
> + if (ret < 0)
> + return ret;
> + return 0;
> +}
> +
> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
> index f615f6a9f9..d33bd61818 100644
> --- a/libavcodec/adts_header.h
> +++ b/libavcodec/adts_header.h
> @@ -2,6 +2,7 @@
> * AAC ADTS header decoding prototypes and structures
> * Copyright (c) 2003 Fabrice Bellard
> * Copyright (c) 2003 Michael Niedermayer
> + * Copyright (c) 2020 Nachiket Tarate
> *
> * This file is part of FFmpeg.
> *
> @@ -34,6 +35,7 @@ typedef struct AACADTSHeaderInfo {
> uint8_t sampling_index;
> uint8_t chan_config;
> uint8_t num_aac_frames;
> + uint32_t frame_length;
> } AACADTSHeaderInfo;
>
> /**
> @@ -47,4 +49,6 @@ typedef struct AACADTSHeaderInfo {
> */
> int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
>
> +int avpriv_adts_header_parse (AACADTSHeaderInfo *hdr, const uint8_t *buf,
> size_t size);
> +
> #endif /* AVCODEC_ADTS_HEADER_H */
>
_______________________________________________
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".
_______________________________________________
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".
