[FFmpeg-devel] AAC decoder round 6

[Michael Niedermayer]

On Mon, Aug 11, 2008 at 12:51:35AM +0100, Robert Swain wrote:
> 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?

i do not know ...


> 
> Oops, and the attachment...

ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080811/f74a1d4f/attachment.pgp>