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

ASF Duration Calculation Fix, Creation Date Fix #256

Merged
merged 1 commit into from
May 14, 2021
Merged

ASF Duration Calculation Fix, Creation Date Fix #256

merged 1 commit into from
May 14, 2021

Conversation

benrr101
Copy link
Contributor

@benrr101 benrr101 commented Mar 8, 2021

Description: Another couple bugs I found while porting ASF support in TagLib# to node 😅

The TagLib# implementation of ASF duration wasn't calculating the same duration for the sample.wma file as FFMpeg, foobar2000, VLC, which all said the file was 0:04 (or 4.153s). TagLib# was reporting the file was around 5s. Digging through the ASF specification and reading the sample with a hex editor, I found two issues with the TagLib# implementation:

  1. The ASF File Properties Object (page 14 in the linked PDF) stores the play duration before the send duration. TagLib# is reading/writing the object the other way around, which means duration was calculated based on send duration (which in this case is a hundred milliseconds less than the play duration). This also means TagLib# is potentially corrupting ASF files by writing the send/play durations backwards. Fix for this is to swap the order play/send duration are read/written in the FilePropertiesObject class.
  2. Preroll duration in the File Properties Object is stored in milliseconds. TagLib# is reading that value as 100ns ticks, so when it is subtracted from the play duration to calculate duration, it's subtracting a far smaller value than it should. Fix for this is to subtract a TimeSpan constructed from the preroll as milliseconds.

The issue described in #255 states that ASF spec says creation date in File Properties Object is 100ns ticks since 1/1/1601 instead of the C# DateTime format's 100ns ticks since 1/1/0000. I don't really know why this is the case, but it is what it is. The fix for this is to add the number of ticks for 1/1/1601 to the creation date as read from the file. ... However, after revisiting the code, I realized creation date is never used and it's impossible to get access to an instance of the class without manually re-reading it from the AsfFile, so this fix is more/less inconsequential. Nevertheless, it's bug, so I fix 😄. Unrelated, I'm planning to expose the HeaderObject in the AsfFile class in my port, and I'm thinking this would be a valuable addition to the TagLib# API.

Testing:

  • Switched the ASF format tests from using the StandardFileTests properties test to checking all the properties we know about the sample file. All cases pass!

…ies object

* Pre-roll is stored in milliseconds
* Fixing calculation of creation date to be based on 1/1/1601
* Checking more values in ASF format properties tests
Base automatically changed from master to main March 10, 2021 16:12
@decriptor
Copy link
Collaborator

@benrr101 Thanks for the fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants