[FFmpeg-devel] AAC decoder round 6

[Robert Swain]

2008/8/11 Robert Swain <robert.swain at gmail.com>:
> 2008/8/10 Michael Niedermayer <michaelni at gmx.at>:
>> On Sun, Aug 10, 2008 at 11:11:21PM +0100, Robert Swain wrote:
>>> 2008/8/10 Michael Niedermayer <michaelni at gmx.at>:
>> [...]
>>> >> > [...]
>>> >> >> >> +    }
>>> >> >> >> +    domain = get_bits1(gb);
>>> >> >> >> +
>>> >> >> >> +    if (is_indep_coup) {
>>> >> >> >> +        coup->coupling_point = AFTER_IMDCT;
>>> >> >> >> +    } else if(domain) {
>>> >> >> >> +        coup->coupling_point = BETWEEN_TNS_AND_IMDCT;
>>> >> >> >> +    } else
>>> >> >> >> +        coup->coupling_point = BEFORE_TNS;
>>> >> >> >
>>> >> >> > :/ i thought the bits were at least stored together, what morons designed
>>> >> >> > this!?
>>> >> >> >
>>> >> >> > coup->coupling_point= get_bits1(gb);
>>> >> >> > ...
>>> >> >> > coup->coupling_point+= 2*get_bits1(gb);
>>> >> >> >
>>> >> >> > still is a possibility though that is cleaner
>>> >> >> > btw, what is the 4th possibility for?
>>> >> >>
>>> >> >> is_indep_coup = 1 overrides whatever domain is so the 4th case is
>>> >> >> redundant. Though theoretically, domain must be 1 if is_indep_coup is
>>> >> >> 1. is_indep_coup = 1 and domain = 0 is theoretically invalid.
>>> >> >>
>>> >> >> coup->coupling_point = 2*get_bits1(gb);
>>> >> >> ...
>>> >> >> coup->coupling_point += get_bits1(gb) && !coup->coupling_point;
>>> >> >> ?
>>> >> >
>>> >> > hmm, this seems unneccesarily complex
>>> >> > and the invalid value should be checked for with av_log&return -1
>>> >>
>>> >> It's not invalid in that it should be checked and cause an error. It's
>>> >> just that the value of domain is ignored when is_indep_coup is set. I
>>> >
>>> > Where does the spec say this?
>>> > If the spec says that one bit switches between doing it before and after A
>>> > and says the second switches before and after B then the order of A and B
>>> > implicate which combination is invalid.
>>>
>>> ind_sw_cce_flag
>>> one bit indicating whether the coupled target syntax element is an
>>> independently switched (1) or a dependently switched (0).
>>>
>>> cc_domain
>>> one bit indicating whether the coupling is performed before (0) or after (1)
>>> the TNS decoding of the coupled target channels
>>>
>>> In 4.6.8.3.3 Decoding process (4.6.8.3 is Coupling channel) it states that:
>>>
>>> "the independently switched CCE must be decoded all the way to the
>>> time domain (i.e. including the synthesis filterbank) before it is
>>> scaled and added onto the various SCE and CPE channels that it is
>>> coupled to in the case that window state does not match."
>>>
>>> "A dependently switched CCE, on the other hand, must have a window
>>> state that matches all of the target SCE and CPE channels that it is
>>> coupled onto as determined by the list of cc_l and cc_r elements. In
>>> this case, the CCE only needs to be decoded as far as the frequency
>>> domain and then scaled as directed by the gain list before it is added
>>> to the target SCE or CPE channels."
>>>
>>> So, the spec seems to suggest that one can couple an independently
>>> switched CCE in the frequency domain in the case that the window state
>>> matches. In these cases, I suppose the cc_domain variable would be of
>>> consequence but otherwise, when coupling in the time domain for
>>> independently switched CCEs, cc_domain should really always be 1 as
>>> coupling would always be conducted after TNS decoding.
>>
>>> Can all
>>> encoders be trusted to write the correct value? :)
>>
>> who cares? its a bug that is VERY easy to debug as long as the "illegal"
>> value is checked for. Its much better than silently doing something with
>> it that may or may not be what the encoder expects. (the artifacts would
>> be much harder to debug)
>
> Fair enough. See attached. I added another memset 0 to be safe and I
> think the same should be added in the decode_ics() return case. Do you
> agree?

Oops, and the attachment...

Regards,
Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080811-0051-coupling_point_simplification.diff
Type: text/x-diff
Size: 1372 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080811/91bc8b74/attachment.diff>