[FFmpeg-devel] [PATCH] Reduce default FDK decoder delay by one frame

[Derek Buitenhuis]

On 2/26/2014 3:48 PM, Omer Osman wrote:
> The default concealment method in the FDK decoder is Energy
> Interpolation. This method requires one frame delay to the output. To
> reduce the default output delay, use Noise Substitution as the default
> concealment method.
> 
> Signed-off-by: Omer Osman <omer.osman at iis.fraunhofer.de>
> ---
>  libavcodec/libfdk-aacdec.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

IMO, needs an minor API version bump, since it changes default behavior which
some (me included) may rely on downstream.

That said, I am not convinced this is a good idea. Why should we stray from the
library's default? I am not big on changing defaults to suit someone's personal
use-case.

>   * This file is part of FFmpeg.
> + * Do not use non-GPL-compatible AAC decoders if you also use FFmpeg
> + * together with GPL-licensed modules, such as the FFmpeg GPL modules.

Drop this. It is handled in configure. Scare tactics not welcome.

>   *
>   * FFmpeg is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2.1 of the License, or (at your option) any later version.
> + * License, version 2.1 (only) as published by the Free Software Foundation.

Why? I am inclined to reject such changes to the standard LGPL header.

>   *
>   * FFmpeg is distributed in the hope that it will be useful,
>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
>   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> + * Lesser General Public License, version 2.1, for more details.

Ditto.

Moreover, you are adding these changes in an unrelated patch. Not cool.

> +    if (decode_flag != AACDEC_CONCEAL){
>      err = aacDecoder_Fill(s->handle, &avpkt->data, &avpkt->size, &valid);
>      if (err != AAC_DEC_OK) {
>          av_log(avctx, AV_LOG_ERROR, "aacDecoder_Fill() failed: %x\n", err);
>          return AVERROR_INVALIDDATA;
>      }
> +    }

You haven't documented these changes in the commit message.

nit: Also needs a re-indent after, in a 2nd patch.

> -    err = aacDecoder_DecodeFrame(s->handle, (INT_PCM *) buf, buf_size, 0);
> +    err = aacDecoder_DecodeFrame(s->handle, (INT_PCM *) buf, buf_size, decode_flag);
>      if (err == AAC_DEC_NOT_ENOUGH_BITS) {
>          ret = avpkt->size - valid;
>          goto end;

[...]

- Derek