[FFmpeg-devel] [PATCH 1/3] avformat/rtpdec_mpeg4: reassemble fragmented AAC frames

[Thomas Volkert]

On 02/13/2015 10:51 PM, Gilles Chanteperdrix wrote:
> Signed-off-by: Gilles Chanteperdrix <gilles.chanteperdrix at xenomai.org>
> ---
>   libavformat/rtpdec_mpeg4.c | 84 +++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 76 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 9655d30..f2814b5 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -33,6 +33,8 @@
>   #include "libavutil/avstring.h"
>   #include "libavcodec/get_bits.h"
>   
> +#define MAX_AAC_HBR_FRAME_SIZE 8191
> +
>   /** Structure listing useful vars to parse RTP packet payload */
>   struct PayloadContext {
>       int sizelength;
> @@ -59,8 +61,9 @@ struct PayloadContext {
>       int au_headers_length_bytes;
>       int cur_au_index;
>   
> -    uint8_t buf[RTP_MAX_PACKET_LENGTH];
> +    uint8_t buf[FFMAX(RTP_MAX_PACKET_LENGTH, MAX_AAC_HBR_FRAME_SIZE)];
>       int buf_pos, buf_size;
> +    uint32_t timestamp;
>   };
>   
>   typedef struct {
> @@ -168,30 +171,95 @@ static int aac_parse_packet(AVFormatContext *ctx, PayloadContext *data,
>   {
>       int ret;
>   
> +
>       if (!buf) {
> -        if (data->cur_au_index > data->nb_au_headers)
> +        if (data->cur_au_index > data->nb_au_headers) {
> +            av_log(ctx, AV_LOG_ERROR, "Invalid parser state\n");
>               return AVERROR_INVALIDDATA;
> -        if (data->buf_size - data->buf_pos < data->au_headers[data->cur_au_index].size)
> +        }
> +        if (data->buf_size - data->buf_pos < data->au_headers[data->cur_au_index].size) {
> +            av_log(ctx, AV_LOG_ERROR, "Invalid AU size\n");
>               return AVERROR_INVALIDDATA;
> -        if ((ret = av_new_packet(pkt, data->au_headers[data->cur_au_index].size)) < 0)
> +        }
> +        if ((ret = av_new_packet(pkt, data->au_headers[data->cur_au_index].size)) < 0) {
> +            av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
>               return ret;
> +        }
>           memcpy(pkt->data, &data->buf[data->buf_pos], data->au_headers[data->cur_au_index].size);
>           data->buf_pos += data->au_headers[data->cur_au_index].size;
>           pkt->stream_index = st->index;
>           data->cur_au_index++;
> -        return data->cur_au_index < data->nb_au_headers;
> +
> +        if (data->cur_au_index == data->nb_au_headers) {
> +            data->buf_pos = 0;
> +            return 0;
> +        }
> +
> +        return 1;
>       }
>   
> -    if (rtp_parse_mp4_au(data, buf, len))
> +    if (rtp_parse_mp4_au(data, buf, len)) {
> +        av_log(ctx, AV_LOG_ERROR, "Error parsing AU headers\n");
>           return -1;
> +    }
>   
>       buf += data->au_headers_length_bytes + 2;
>       len -= data->au_headers_length_bytes + 2;
> +    if (data->nb_au_headers == 1 && len < data->au_headers[0].size) {
> +        /* Packet is fragmented */
> +
> +        if (!data->buf_pos) {
> +            if (data->au_headers[0].size > MAX_AAC_HBR_FRAME_SIZE) {
> +                av_log(ctx, AV_LOG_ERROR, "Invalid AU size\n");
> +                return AVERROR_INVALIDDATA;
> +            }
> +
> +            data->buf_size = data->au_headers[0].size;
> +            data->timestamp = *timestamp;
> +        }
> +
> +        if (data->timestamp != *timestamp
> +            || data->au_headers[0].size != data->buf_size
> +            || data->buf_pos + len > MAX_AAC_HBR_FRAME_SIZE) {
> +            data->buf_pos = 0;
> +            data->buf_size = 0;
> +            av_log(ctx, AV_LOG_ERROR, "Invalid packet received\n");
> +            return AVERROR_INVALIDDATA;
> +        }
>   
> -    if (len < data->au_headers[0].size)
> +        memcpy(&data->buf[data->buf_pos], buf, len);
> +        data->buf_pos += len;
> +
> +        if (!(flags & RTP_FLAG_MARKER))
> +            return AVERROR(EAGAIN);
> +
> +        if (data->buf_pos != data->buf_size) {
> +            data->buf_pos = 0;
> +            av_log(ctx, AV_LOG_ERROR, "Missed some packets, discarding frame\n");
> +            return AVERROR_INVALIDDATA;
> +        }
> +
> +        data->buf_pos = 0;
> +        ret = av_new_packet(pkt, data->buf_size);
> +        if (ret < 0) {
> +            av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
> +            return ret;
> +        }
> +        pkt->stream_index = st->index;
> +
> +        memcpy(pkt->data, data->buf, data->buf_size);
> +
> +        return 0;
> +    }
> +

Okay.

> +    if (len < data->au_headers[0].size) {
> +        av_log(ctx, AV_LOG_ERROR, "First AU larger than packet size\n");

Can this really happen?
In the lines above you already check for the case that data is missing 
while you have already received the last packet of the frame.


>           return AVERROR_INVALIDDATA;
> -    if ((ret = av_new_packet(pkt, data->au_headers[0].size)) < 0)
> +    }
> +    if ((ret = av_new_packet(pkt, data->au_headers[0].size)) < 0) {
> +        av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
>           return ret;
> +    }
>       memcpy(pkt->data, buf, data->au_headers[0].size);
>       len -= data->au_headers[0].size;
>       buf += data->au_headers[0].size;
Okay.

Best regards,
Thomas.