knik0 / faac

Freeware Advanced Audio Coder faac mirror
https://sourceforge.net/projects/faac/
Other
180 stars 60 forks source link

Introduction of PNS coding breaks Joint Stereo coding #11

Closed enzo1982 closed 6 years ago

enzo1982 commented 6 years ago

The recent introduction PNS in FAAC 1.29.9 broke Joint Stereo coding (jointmode == JOINT_MS) due to a bad branch.

In quantize.c the else branch in the following code is now executed if book == HCB_ZERO, but it should only be executed for book == HCB_ESC:

    if ((book == HCB_INTENSITY) || (book == HCB_INTENSITY2))
    {
        [...]
    }
    else if (book != HCB_PNS)
    {
        [...]
    }

Here is a sample which shows exceptionally audible artifacts in the right channel due to this: sample.zip

I'll prepare a pull request with a fix.

knik0 commented 6 years ago

What a bug, thanks for catching it. I believe it should be fixed by skipping HCB_ZERO.

knik0 commented 6 years ago

BTW. M/S coding is not very useful, left mainly for reference. It would probably be better removed. When you use M/S it just means you have less efficient encoding than IS. Let me know if you see any problems with removing it.

enzo1982 commented 6 years ago

Is every AAC decoder able to decode streams encoded with IS? If so, I have no objections to removing MS.

I think FAAC should then do IS encoding when jointmode is set to either 1 or 2.

knik0 commented 6 years ago

IS is like a standard AAC thing, nothing special, all decoders I tried support it.

I think FAAC should then do IS encoding when jointmode is set to either 1 or 2.

Yes, exactly.

enzo1982 commented 6 years ago

OK, great!