On Tue, Feb 20, 2018 at 3:37 PM, Dale Curtis <dalecur...@chromium.org> wrote:
> From 97380752ef12337d9b9a01801a9a84df3b4b9c0a Mon Sep 17 00:00:00 2001
> From: Dale Curtis <dalecur...@chromium.org>
> Date: Thu, 15 Feb 2018 16:22:55 -0800
> Subject: [PATCH] [aacdec] Parse and drop gain control data, so that SSR
> packets decode.
>
> This will result in poor quality audio for SSR streams, but they
> will at least demux and decode without error; partially fixing
> ticket #1693.
>
> This just pulls in the decode_gain_control() function from the
> ffmpeg summer-of-code repo (original author Robert Swain) at
> svn://svn.mplayerhq.hu/soc/aac/aac.c and adds AOT_AAC_SSR to
> decode_audio_specific_config_gb().
>
> Signed-off-by: Dale Curtis <dalecur...@chromium.org>
> Co-authored-by: Robert Swain <robert.sw...@gmail.com>
> ---
> libavcodec/aacdec_template.c | 49 +++++++++++++++++++++++++++++++++---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
> index c2d9802023..c3ec3eefe8 100644
> --- a/libavcodec/aacdec_template.c
> +++ b/libavcodec/aacdec_template.c
> @@ -997,6 +997,7 @@ static int decode_audio_specific_config_gb(AACContext *ac,
> switch (m4ac->object_type) {
> case AOT_AAC_MAIN:
> case AOT_AAC_LC:
> + case AOT_AAC_SSR:
> case AOT_AAC_LTP:
> case AOT_ER_AAC_LC:
> case AOT_ER_AAC_LD:
> @@ -1967,6 +1968,44 @@ static void apply_prediction(AACContext *ac,
> SingleChannelElement *sce)
> reset_all_predictors(sce->predictor_state);
> }
>
> +static int decode_gain_control(SingleChannelElement * sce, GetBitContext *
> gb)
> +{
> + // wd_num, wd_test, aloc_size
> + static const int gain_mode[4][3] = {
I would make this uint8_t
> + {1, 0, 5}, // ONLY_LONG_SEQUENCE = 0,
> + {2, 1, 2}, // LONG_START_SEQUENCE,
> + {8, 0, 2}, // EIGHT_SHORT_SEQUENCE,
> + {2, 1, 5}, // LONG_STOP_SEQUENCE
> + };
> +
> + // per-element gain control for SSR
> + struct {
> + int max_band;
> + int adjust_num[4][8];
> + int alev[4][8][8];
> + int aloc[4][8][8];
> + } ssr;
I would drop this stack struck and replace the leaf get_bits() with
skip bits. It makes the code that much harder to exploit and there is
no point in storing data we don't plan on decoding
> +
> + const int mode = sce->ics.window_sequence[0];
> + int bd, wd, ad;
> +
> + // FIXME: Store the gain control data on |sce| and do something with it.
> + ssr.max_band = get_bits(gb, 2);
> + for (bd = 0; bd < ssr.max_band; bd++) {
> + for (wd = 0; wd < gain_mode[mode][0]; wd++) {
> + ssr.adjust_num[bd][wd] = get_bits(gb, 3);
> + for (ad = 0; ad < ssr.adjust_num[bd][wd]; ad++) {
> + ssr.alev[bd][wd][ad] = get_bits(gb, 4);
> + if (wd == 0 && gain_mode[mode][1])
> + ssr.aloc[bd][wd][ad] = get_bits(gb, 4);
> + else
> + ssr.aloc[bd][wd][ad] = get_bits(gb, gain_mode[mode][2]);
> + }
> + }
> + }
> + return 0;
> +}
> +
> /**
> * Decode an individual_channel_stream payload; reference: table 4.44.
> *
> @@ -2034,9 +2073,13 @@ static int decode_ics(AACContext *ac,
> SingleChannelElement *sce,
> goto fail;
> }
> if (!eld_syntax && get_bits1(gb)) {
> - avpriv_request_sample(ac->avctx, "SSR");
> - ret = AVERROR_PATCHWELCOME;
> - goto fail;
> + // FIXME: Do something with the gain control data...
> + ret = decode_gain_control(sce, gb);
> + if (ret < 0) {
> + ret = AVERROR_PATCHWELCOME;
> + avpriv_request_sample(ac->avctx, "SSR");
> + goto fail;
> + }
This block seems funny. decode_gain_control() always returns zero.
Maybe make this warn once per stream when present like some of the
other AAC warn
cases.
> }
> // I see no textual basis in the spec for this occurring after SSR
> gain
> // control, but this is what both reference and real implmentations
> do
> --
> 2.16.1.291.g4437f3f132-goog
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
