[vlc-devel] new aac encoder for review

[Ilkka Ollakka]

On Sat, Oct 06, 2012 at 02:02:06PM -0400, Sergio M. Ammirata, Ph.D. wrote:
> +
> +#define  CH_ORDER_MPEG 0  /*!< MPEG channel ordering (e. g. 5.1: C, L, R, SL, SR, LFE)           */
> +#define  CH_ORDER_WAV 1   /*!< WAV fileformat channel ordering (e. g. 5.1: L, R, C, LFE, SL, SR) */
> +#define  CH_ORDER_WG4 2   /*!< WG4 fileformat channel ordering (e. g. 5.1: L, R, SL, SR, C, LFE) */
> +
> +#define PROFILE_AAC_LC 2
> +#define PROFILE_AAC_HE 5
> +#define PROFILE_AAC_HE_v2 29
> +#define PROFILE_AAC_LD 23
> +#define PROFILE_AAC_ELD 39
> +
> +#define SIGNALING_COMPATIBLE 1
> +#define SIGNALING_HIERARCHICAL 2

Hi, are these numbers from libfdk-headers?

> +
> +    add_bool( ENC_CFG_PREFIX "afterburner", true, AFTERBURNER_TEXT,
> +              AFTERBURNER_LONGTEXT, false )
> +    add_integer( ENC_CFG_PREFIX "signaling", SIGNALING_COMPATIBLE, SIGNALING_TEXT,
> +             SIGNALING_LONGTEXT, false )

Aren't afterburner and signaling more advanced options? Not a big issue,
but that false means that they are visible everytime and considered
basic options that user should/could always fiddle.

> +    if ( p_aout_buf )

This could use likely( p_aout_buf )

> +        if ((erraac = aacEncEncode(p_sys->handle, &in_buf, &out_buf, &in_args, &out_args)) != AACENC_OK) {
> +            if (erraac == AACENC_ENCODE_EOF) {
> +                msg_Info( p_enc, "Encoding final bytes (EOF)");

How does this behave on EOF? I didn't spot directly how it does break
away from this loop when it gets that NULL block and encodes EOF stuff?

> +            if (i_samples == 0)

likely/unlikely in here too? or is it common to have i_samples and not to have
i_samples?

> +        }
> +        if ( i_loop_count++ > 100 )

Does this happen easily? unlikely( ) ?
> +        {
> +            msg_Err( p_enc, "Loop count greater than 100!!!, something must be wrong with the encoder library");
> +            break;
> +        }

-- 
Ilkka Ollakka
The reason why worry kills more people than work is that more people
worry than work.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20121010/10dec1e3/attachment.sig>