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

Improve the QuickTime spec #684

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

marcin-osowski
Copy link

I plan to extend it more, but I'd like to send out this chunk of work first, to check that I'm doing this right.

Changes:

  1. Updated URLs to Apple documentation.
  2. Added docs
  3. Implemented the matrix field
  4. Added duration in seconds
  5. Added two new atoms (there's a lot more in the spec)
    • Media header (mdhd)
    • Handler reference (hdlr)

I plan to extend it more, but I'd like to send out this chunk of work
first, to check that I'm doing this right.

Changes:
  1) Updated URLs to Apple documentation.
  2) Added docs
  3) Implemented the matrix field
  4) Added duration in seconds
  5) Added two new atoms (there's a lot more in the spec)
      - Media header (mdhd)
      - Handler reference (hdlr)
@marcin-osowski
Copy link
Author

Friendly ping.

@marcin-osowski
Copy link
Author

Thanks for the improvements, they look good to me.

I don't fully understand the syntax in d8fce6b1fceb0695b1bb0106764e9d2927dd335c, though, but I guess it's all good.

@generalmimon
Copy link
Member

I don't fully understand the syntax in d8fce6b1fceb0695b1bb0106764e9d2927dd335c, though, but I guess it's all good.

Most of it (the doc key) is just human-readable documentation with some Markdown formatting (more precisely CommonMark) - the syntax there is not important, the content is.

The value instances are described in the User Guide, but I guess you aren't asking about them, given that you used them too.

@marcin-osowski
Copy link
Author

I see, I was puzzled by for example -orig-id, but I see that those are just keys which names start with -: https://doc.kaitai.io/user_guide.html.

Looks good to me!

Copy link
Member

@generalmimon generalmimon left a comment

Choose a reason for hiding this comment

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

I have a few small suggestions:

media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
media/quicktime_mov.ksy Outdated Show resolved Hide resolved
@marcin-osowski
Copy link
Author

Thanks for the suggestions, and the very detailed research! I updated the pull request, I hope I didn't miss any of your proposals.

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