libquicktime-devel

Re: [Libquicktime-devel] AAC audio

[Derek C. <der...@mi...>]

Hi Phil,

I was just starting to have a go at adding an AAC decoder to the lqt_ffmpeg plugin as an LPGL alternative to using the GPL lqt_faad2 plugin and when I decided to do a search on the mailing list I saw this thread start by you - proposing to do exactly the same thing!

Don't suppose you got anywhere with a patch?  Any remarks to make on getting this working?

Thanks,
Derek

Re: [Libquicktime-devel] AAC audio

[Phil B. <ph...@fi...>]

Hi Derek

> I was just starting to have a go at adding an AAC decoder to the  
> lqt_ffmpeg plugin as an LPGL >alternative to using the GPL lqt_faad2  
> plugin and when I decided to do a search on the mailing list I >saw this  
> thread start by you - proposing to do exactly the same thing!
>
> Don't suppose you got anywhere with a patch?  Any remarks to make on  
> getting this working?

My colleague Erik Johansson implemented AAC decoding support via the  
FFMPEG plugin.

He doesn't think the patch would be considered appropriate for merging as  
it stands currently, largely because it's implemented by reading one part  
of the iTunes data atom (iTunSMPB) without any support for writing this,  
or for reading or writing other parts of the atom. But it works for us.

We could put together a patch for you to develop further if you liked.

Phil
-- 
Phil Barrett
FilmLight Ltd

Re: [Libquicktime-devel] AAC audio

[Derek C. <der...@mi...>]

Oh yes please, if possible I'd like to have a look.

(PS, say hi to Mark for me! :P)

________________________________
From: Phil Barrett [ph...@fi...]
Sent: 04 October 2013 15:31
To: Derek Chow
Cc: lib...@li...
Subject: Re: [Libquicktime-devel] AAC audio

Hi Derek

I was just starting to have a go at adding an AAC decoder to the lqt_ffmpeg plugin as an LPGL alternative to using the GPL lqt_faad2 plugin and when I decided to do a search on the mailing list I saw this thread start by you - proposing to do exactly the same thing!

Don't suppose you got anywhere with a patch?  Any remarks to make on getting this working?

My colleague Erik Johansson implemented AAC decoding support via the FFMPEG plugin.

He doesn't think the patch would be considered appropriate for merging as it stands currently, largely because it's implemented by reading one part of the iTunes data atom (iTunSMPB) without any support for writing this, or for reading or writing other parts of the atom. But it works for us.

We could put together a patch for you to develop further if you liked.

Phil
--
Phil Barrett
FilmLight Ltd

Re: [Libquicktime-devel] AAC audio

[Burkhard P. <bur...@ig...>]

Hi,

Am 04.10.2013 17:28, schrieb Phil Barrett:
> Hi Derek
>
> Try attached patch against current CVS tip - it's probably not quite right but it's mostly there.
>

I took a quick look and noticed the following (for the case it should go into cvs):

- If an ffmpeg decoder outputs floating point samples, they should be
   passed through and not converted to 16 bit int.

- The prime samples are completely ignored by the faad2 based decoder so ignoring them
   for the ffmpeg decoder would mean no feature regression.

- The proper method for getting the prime samples can be a separate patch. If the data
   is available in an itunes atom this can be used of course, but we shouln't write any
   itunes specific stuff unless the file type is LQT_FILE_M4A.

Burkhard

Re: [Libquicktime-devel] AAC audio

[Erik J. <eri...@gm...>]

Hi,

On Mon, Oct 7, 2013 at 2:02 PM, Burkhard Plaum
<bur...@ig...> wrote:
> - If an ffmpeg decoder outputs floating point samples, they should be
>    passed through and not converted to 16 bit int.
>
> - The prime samples are completely ignored by the faad2 based decoder so ignoring them
>    for the ffmpeg decoder would mean no feature regression.
>
> - The proper method for getting the prime samples can be a separate patch. If the data
>    is available in an itunes atom this can be used of course, but we shouln't write any
>    itunes specific stuff unless the file type is LQT_FILE_M4A.

I'll just add a few additional comments. As Phil wrote, we had an
internal discussion about whether to send our existing solution as a
patch to libquicktime. My comments at that time were:

> 1) The support for reading the iTunes atom is very limited, as we only read iTunSMPB.
> It appears most of the other supported atoms can be both read and written.
>
> 2) The parsing of the payload of the iTunSMPB is assumes the data is correctly
> formatted.

A little later, we switched to a newer version of ffmpeg, which is
where the horrible float to 16-bit audio hack enters the picture. My
unredacted notes with regards to that are as follows:

> AVCodecContext provides a field for audio sample format as follows:
>     /**
>      * audio sample format
>      * - encoding: Set by user.
>      * - decoding: Set by libavcodec.
>      */
>     enum AVSampleFormat sample_fmt;  ///< sample format
> The audio decoder in libquicktimes ffmpeg plugin sets sample_fmt to 16bit signed, and
> then relies on that being the case throughout the code. Only the latest ffmpeg resets
> sample_fmt to float as per the documentation. In addition, it appears memory
> allocated for audio decoded using avcodec_decode_audio4() is never free'd.

and

> Fixed memory leak and implemented horrible hack to get by for now. Opened bug
> 24574 for fixing properly ("broken gets fixed, shoddy lasts forever").

I realise I'm probably mixing issues on the same thread, but hopefully
it'll provide some context that may prove helpful to Derek :-)

Thanks,
Erik

Re: [Libquicktime-devel] AAC audio

[Burkhard P. <bur...@ig...>]

Hi,

Am 07.10.2013 21:48, schrieb Erik Johansson:
> Hi,
[...]
>> AVCodecContext provides a field for audio sample format as follows:
>>      /**
>>       * audio sample format
>>       * - encoding: Set by user.
>>       * - decoding: Set by libavcodec.
>>       */
>>      enum AVSampleFormat sample_fmt;  ///< sample format
>> The audio decoder in libquicktimes ffmpeg plugin sets sample_fmt to 16bit signed, and
>> then relies on that being the case throughout the code. Only the latest ffmpeg resets
>> sample_fmt to float as per the documentation.

This was written at a time when all ffmpeg audio codecs used 16 bit samples. Nowadays it's
different. The behaviour of the core is to call the decode() function with output = NULL at
the beginning. During this initialization call, the member atrack->sample_format can be set
(according to what's in ffmpeg). In most codecs it's hardwired, but in audiocodec/pcm.c
you'll find a function init_decode_lpcm(), which does runtime detection of the sample format.

Making proper sample format detection and supporting planar audio in ffmpeg would be a very
patch to start with.

Burkhard