This repository has been archived by the owner. It is now read-only.

Support of AAC with 960 samples/frame #2031

Closed
basicmaster opened this Issue Jul 2, 2017 · 6 comments

Comments

2 participants
@basicmaster

basicmaster commented Jul 2, 2017

AAC allows to store either 1024 or 960 samples per frame. Usually 1024 is used, but DAB+ uses 960. This setting is stored in the frameLengthFlag of the GASpecificConfig.

When I convert an AAC file (in my case a LATM stream including the AudioSpecificConfig) to MKV with mkvmerge -o test.mkv test.aac, this setting seems to get lost: VLC plays the file as if 1024 had been used (which is the default for the common AAC decoders when the 960 flag is ignored somehow) - it is played slower and sounds lower pitched. Looking into the code, this flag is currently ignored in /src/common/aac.cpp.

I uploaded an affected file as aac_latm_960.aac which comes from a DAB+ service.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 2, 2017

Owner

Thanks. I'll look into it.

Owner

mbunkus commented Jul 2, 2017

Thanks. I'll look into it.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 3, 2017

Owner

Hmm, the problem is that the information from GASpecificConfig is currently not stored in Matroska; meaning mkvmerge doesn't have a way of including that information in the Matroska file. A spec modification will be required as well as modifications to all player software reading AAC from Matroska.

At the moment I'm not willing to invest the time here. Therefore I'll close this as won't fix.

Owner

mbunkus commented Jul 3, 2017

Hmm, the problem is that the information from GASpecificConfig is currently not stored in Matroska; meaning mkvmerge doesn't have a way of including that information in the Matroska file. A spec modification will be required as well as modifications to all player software reading AAC from Matroska.

At the moment I'm not willing to invest the time here. Therefore I'll close this as won't fix.

@mbunkus mbunkus closed this Jul 3, 2017

@mbunkus mbunkus added the res:wontfix label Jul 3, 2017

@basicmaster

This comment has been minimized.

Show comment
Hide comment
@basicmaster

basicmaster Jul 3, 2017

Meanwhile I tested to convert a DAB+ recording that is contained in an ISOBMFF container to MKV - this time interestingly, the converted MKV is played correctly. To me it seems that there must be already some means to keep the needed codec init data (at least) from ISOBMFF to MKV.

I uploaded the file (AAC with SBR from a DAB+ service) as aac_isobmff_960.m4a.

basicmaster commented Jul 3, 2017

Meanwhile I tested to convert a DAB+ recording that is contained in an ISOBMFF container to MKV - this time interestingly, the converted MKV is played correctly. To me it seems that there must be already some means to keep the needed codec init data (at least) from ISOBMFF to MKV.

I uploaded the file (AAC with SBR from a DAB+ service) as aac_isobmff_960.m4a.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 3, 2017

Owner

Oh? Is it stored in CodecPrivate? Hmm… Maybe I mis-read my own code. I'll re-open.

Owner

mbunkus commented Jul 3, 2017

Oh? Is it stored in CodecPrivate? Hmm… Maybe I mis-read my own code. I'll re-open.

@mbunkus mbunkus reopened this Jul 3, 2017

@mbunkus mbunkus removed the res:wontfix label Jul 3, 2017

@mbunkus mbunkus added type:enhancement and removed type:bug labels Jul 20, 2017

mbunkus added a commit that referenced this issue Jul 20, 2017

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 20, 2017

Owner

New pre-builds for Windows that include the functionality are available (build numbers 01532 and higher). They work with both samples you've provided.

Owner

mbunkus commented Jul 20, 2017

New pre-builds for Windows that include the functionality are available (build numbers 01532 and higher). They work with both samples you've provided.

@basicmaster

This comment has been minimized.

Show comment
Hide comment
@basicmaster

basicmaster commented Jul 21, 2017

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.