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

Fix segments #1495

Merged
merged 1 commit into from May 15, 2021
Merged

Fix segments #1495

merged 1 commit into from May 15, 2021

Conversation

raymondjacobson
Copy link
Member

@raymondjacobson raymondjacobson commented May 15, 2021

Description

What is the purpose of this PR? What is the current behavior? New behavior? Relevant links (e.g. Trello) and/or information pertaining to PR?

Spent forever playing with aac encoders, etc. etc. etc. and turns out this is the issue.
Tracks > 1:30 will fail at 10:06 (the 101st segment) because of this line
https://github.com/AudiusProject/audius-protocol/blob/master/creator-node/src/ffmpeg.js#L42
which reads segments from the dir in alphabetical order -- those files will be
000.ts
001.ts
...
100.ts
1000.ts
1001.ts
...
1009.ts
101.ts
...

This as you imagine can break * a lot * of things

We have a few ways we can handle this client side for old tracks, but let's get this in ASAP.

  • Manually reorder segments > 100 in length uploaded before this is released
  • Stream mp3 variants before this is released
    Either way we should be able to patch this over as soon as we can take this out to prod

Tests

List the manual tests and repro instructions to verify that this PR works as anticipated. Include log analysis if possible.
❗ If this change impacts clients, make sure that you have tested the clients ❗

  1. Units
  2. On stage cn3 (tested with a track that is messed up on prod)
    ...

❗ Reminder 💡❗:
If this PR touches a critical flow (such as Indexing, Uploads, Gateway or the Filesystem), make sure to add the requires-special-attention label. Add relevant labels as necessary.

@raymondjacobson raymondjacobson force-pushed the rj-fix-segments branch 5 times, most recently from 8a6cc18 to 2e910ab Compare May 15, 2021 06:05
Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯 this bug is insane. so theoretically every track that overflows past 3 digit segments experiences this because of the character sort issue?

@raymondjacobson
Copy link
Member Author

🤯 this bug is insane. so theoretically every track that overflows past 3 digit segments experiences this because of the character sort issue?

yep...:(

@raymondjacobson
Copy link
Member Author

🤯 this bug is insane. so theoretically every track that overflows past 3 digit segments experiences this because of the character sort issue?

yep...:(

we could also do a global migration to fix existing tracks on discprov. that could be cool.

@raymondjacobson raymondjacobson merged commit 27576f5 into master May 15, 2021
@raymondjacobson raymondjacobson deleted the rj-fix-segments branch May 15, 2021 17:35
dmanjunath pushed a commit that referenced this pull request May 18, 2021
dmanjunath added a commit that referenced this pull request May 18, 2021
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.

None yet

2 participants