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

[Nachiket Tarate]

________________________________
From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> on behalf of James Almer <jamrial at gmail.com>
Sent: Tuesday, October 6, 2020 8:47 PM
To: ffmpeg-devel at ffmpeg.org <ffmpeg-devel at 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.tarate at 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 at ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".