[vlc-devel] new aac encoder for review

[Sergio M. Ammirata, Ph.D.]

Answers below ...

On 10/10/12 3:32 AM, "Ilkka Ollakka" <ileoo at videolan.org> wrote:

>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?


Not in the public ones. The code expects you to use the numbers but I
found it too confusing to follow so I created these defs.

>
>> +
>> +    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.

Yes, they should be changed to true. I did not know what that last value
meant.

>
>> +    if ( p_aout_buf )
>
>This could use likely( p_aout_buf )

It is not unlikely, it actually happens at the end of every playlist item
or when you shutdown VLC while playing a stream.

>
>> +        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?

The VLC encoder function receives a NULL block and then triggers the EOF
behavior in the lib encoder by passing "in_args.numInSamples = -1" to it.
What surprised me was the fact the the aacEncEncode function needs to be
called on a loop with that argument a few times to completely empty the
library buffer (typically less than 5 and it depends on the framesize).
When the buffer is empty, aacEncEncode will come back with
"out_args.numOutBytes = 0" and with the error code "AACENC_ENCODE_EOF".

>
>> +            if (i_samples == 0)
>
>likely/unlikely in here too? or is it common to have i_samples and not to
>have
>i_samples?

Same as "if ( p_aout_buf )", it happens at EOF scenario ..

>
>> +        }
>> +        if ( i_loop_count++ > 100 )
>
>Does this happen easily? unlikely( ) ?

It happened to me a couple of times during development because of my own
mistakes Š I left it there as a sanity check. Unlikely is appropriate here.

>> +        {
>> +            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.
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>http://mailman.videolan.org/listinfo/vlc-devel

Sergio