The Wayback Machine - https://web.archive.org/web/20190817015654/https://github.com/Borewit/music-metadata/issues/207
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AAC parsed format is missing duration property #207

Closed
Vondry opened this issue May 2, 2019 · 5 comments

Comments

@Vondry
Copy link

commented May 2, 2019

Bug description
Parsed aac file is missing duration property. Using await parseFile(path, {duration: true});

{ format:
   { tagTypes: [],
     lossless: false,
     dataformat: 'ADTS/MPEG-4',
     encoder: 'AAC',
     sampleRate: 44100,
     codecProfile: 'AAC LC',
     bitrate: 127821.59375,
     numberOfChannels: 2 },
  native: undefined,
  common:
   { track: { no: null, of: null }, disk: { no: null, of: null } } }

Expected behavior
I would expect to have duration property.

Audio file demonstrating the problem
(https://github.com/Borewit/music-metadata/files/3139850/NIK.TENDO.DECKY.-.Most.Wanted.x.YZOMANDIAS.OFF.VIZUAL.zip)

@Vondry Vondry changed the title ACC parsed format is missing duration property AAC parsed format is missing duration property May 2, 2019

@Borewit

This comment has been minimized.

Copy link
Owner

commented May 3, 2019

@Vondry thanks for reporting this issue. Good to hear the new AAC capability is actually being used.

The duration of ADTS/AAC is a bit nasty to implement (even Foobar2000 did not calculate the duration of my sample files), so I skipped it deliberately, I was curious if someone would point it out.

If I implement it, the duration calculation will be expensive (will take a while to calculate), each and every ADTS frame has to be parsed. Therefor I will map the option to retrieve the duration to the duration flag.

@Vondry

This comment has been minimized.

Copy link
Author

commented May 3, 2019

Hi, thanks for such a quick reply.

Let's assume small sized file, which has between 5-10Mb. I have no idea how much time it may take, but if it takes less than a second, it is OK in my case. Since I parse all files before the actual app starts and then i really need duration property to be present.

Have a nice day. :)

@Borewit

This comment has been minimized.

Copy link
Owner

commented May 3, 2019

but if it takes less than a second, it is OK in my case

I don't think so, probably a few seconds, maybe even more then 10 seconds...

Sorry, these MPEG/AFTS frames which require byte-hunting are terrible inefficient to parse in JavaScript. AAC is using very small frames which makes it even worse.

@Vondry

This comment has been minimized.

Copy link
Author

commented May 3, 2019

This performance issue should be definitively highlighted in docs with some related topic, that is for sure.

My app relies on the duration. So if no duration is present, i can not properly visualize progress of current track, so it makes aac more likely unusuable format. I would welcome duration property to be present anyway.

@Borewit

This comment has been minimized.

Copy link
Owner

commented Jul 26, 2019

Because this issue is open since a long time I will give an update.
To address this issue, to get the duration out of the ADTS file, I need to improve the MPEG/ADTS parser. Parsing ADTS files, I still experience loosing sync somewhere half way the file.

So it's a lot of work, for a file format which is not very common. Once addressed it will be limited success (it will be slow) at most. Therefor it is not high on my priority list. Since there are not to many other issues, I may do fix it one day.

@Borewit Borewit added the enhancement label Jul 27, 2019

Borewit added a commit that referenced this issue Aug 12, 2019

Borewit added a commit that referenced this issue Aug 13, 2019

Borewit added a commit that referenced this issue Aug 13, 2019

@Borewit Borewit closed this in #234 Aug 13, 2019

Borewit added a commit that referenced this issue Aug 13, 2019

Merge pull request #234 from Borewit/issue-207-adts-parser
#207 Improve ADTS stream handling, calculate duration if duration fla…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.