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

Switch to webm-iterable for parsing raw file data #188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

austinleroy
Copy link

Hello! I saw the post for Symphonia on Reddit a little while back and it immediately piqued my interest. I love writing code in Rust, and I've actually done a fair amount of work around parsing and writing EBML (basis for mkv/webm). I decided to look into the project to see if there was anything I could do to help on the Matroska front.

I put together this PR in an attempt to:

  • Reduce code in the project
  • Make the mkv demuxer logic operate at a higher level (deal with metadata packets rather than need to know binary encodings)
  • Improve stability/compatibility
  • Improve demuxing speed

Overall, I'm satisfied with the outcome. This PR does remove over 1000 lines of code (822 additions and 2078 deletions), it delegates raw EBML packet parsing to the webm-iterable library (some bias here since I wrote it), and the resulting symphonia-play binary can play all of the matroska test files. I unfortunately wasn't able to improve the demuxing speed like I had hoped, but the speed also isn't any worse, so a neutral result there.

Hopefully you think this is a helpful change! I'm more than happy to talk through any questions or feedback.

Some notes on the matroska test files
This branch plays them all perfectly except for file 7 (the damaged/corrupted file). It plays, but not smoothly - it skips around a little more than mpv does. Just a note that there could still be improvement there.
As far as the previous version of the mkv demuxer - it failed to play test file 3 entirely, and test file 7 ended after 1 second of playback. The other test files were fine, although some other mkv/mka files I found in the wild were also unplayable on the old version (the new version plays these files fine). I've uploaded 2 examples in another branch in my forked repo: https://github.com/austinleroy/Symphonia/tree/AdditionalTestFiles/symphonia-play/mkvtestfiles

Some notes on my performance testing
I used hyperfine and the --decode-only flag like was described elsewhere in this repo. All of my runs seemed to bounce between the new version and the old version being "1.00 +- 0.02 times faster" than the other. I took that as an indication that performance was identical, although it would be good to confirm this on other machines.

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.

1 participant