[FFmpeg-devel] [PATCH] MLP/TrueHD Decoder - 2nd try

[Ramiro Polla]

Diego Biurrun wrote:
> On Tue, Jul 01, 2008 at 06:24:55PM +0100, Ramiro Polla wrote:
>>
>> /** Maximum number of matrices used in decoding. Most streams have one matrix
>>  *  per output channel, but some rematrix a channel (usually 0) more than once.
>>  */
>>
>> #define MAX_MATRICES        15
> 
> Note that while "matrix" can be pluralized as both "matrices" and
> "matrixes", the latter is more common in FFmpeg so please use that form.

I'm not interested in the Ministry of English Composition Supreme Court 
Case Matrices Vs. Matrixes. I still have an encoder to get working in a 
week. Whatever the verdict is at the end of the trial feel free to make 
the necessary changes. Somehow I feel the trial will take longer. There 
are still many more witnesses to be called, and I've heard rumours that 
the defense bribed some specialist to testify in their favour.

(funny thing: thunderbird puts a red squiggly line under matrixes)

>> /** The maximum number of taps in either the IIR or FIR filter.
>>  *  I believe MLP actually specifies the maximum order for IIR filters is four,
> 
> as four / to be four
> 
> Why "I believe" anyway?
> 
>> typedef struct SubStream {
>>     //! For each substream, whether a restart header has been read
> 
> .
> 
>>     /** Restart header data */
> 
> lowercase

Hmm... What's the logic here, again? I'm not sure I got all the 
lowercase or end-in-a-periods right.

>>     //! The sync word used at the start of the last restart header
> 
> .
> 
>>     //! The index of the first channel coded in this substream
> 
> .
> 
>>     //! The index of the last channel coded in this substream
> 
> .
> 
>>     //! The number of channels input into the rematrix stage
> 
> .
> 
>>     //! The left shift applied to random noise in 0x31ea substreams
> 
> .
> 
> .. and more below ..
> 
>>     //! Output channel of matrix
> 
> matrix output channel
> 
>>     //! Number of taps in filter
> 
> lowercase
> 
>>     //! Right shift to apply to output of filter
> 
> lowercase
> 
> .. more below ..
> 
>> /** Tables defining the huffman codes.
>>  *  There are three entropy coding methods used in MLP (four if you count "none"
>>  *  as a method). These use the same sequences for codes starting 00... or 01...
>>  *  but have different codes starting 1....
> 
> starting with/at
> 
>>  *  We can implement this behaviour using a standard av_crc on all but the
> 
> behavior
> 
>>         av_log(m->avctx, AV_LOG_ERROR,
>>                "Channel group 2 cannot have more bits per sample than group 1\n");
> 
> .
> 
> .. more below ..
> 
>> /** Read parameters for one of the prediction filters.
>>  */
> 
> nit: You could put the closing comment marks on the same line, same in
> other places.
> 
>>             /* Both filters must have the same precision, so the filtering
>>              * code always use the FIR precision. If only IIR is used, we copy
>>              * its precision to FIR. */
> 
> useS
> 
> Isn't this the comment we discussed and changed before?
> 
>> /** Read a block of PCM residual (or actual if no filtering active) data.
> 
> Read a block of PCM residual data (or actual data if no filtering is
> active).
> 
>>             av_log(m->avctx, AV_LOG_ERROR,
>>                    "Substream %d data indicated length goes off end of packet.\n",
> 
> I assume this means
> 
> "Indicated length of substream %d data goes off the end of the packet.\n",
> 
>>             av_log(avctx, AV_LOG_ERROR,
>>                    "Substream %d data indicated end offset "
>>                    "is before calculated start offset.\n",
> 
> same here

I think I took care of most.

Ramiro Polla