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.

Sign up for GitHub

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

Jump to bottom

Rename initial presentation delay fields. #108

Closed
wants to merge 2 commits into from
Closed

Rename initial presentation delay fields. #108

wants to merge 2 commits into from

Conversation

tomfinegan
Copy link
Collaborator

@tomfinegan tomfinegan commented Oct 10, 2018 •
edited
Loading

This editorial change renames the initial presentation
delay fields to avoid confusion between the ISOBMFF value,
which is in samples, and the AV1 bitstream value, which is
in frames.

  • Rename initial_presentation_delay_present to
    initial_display_delay_present.
  • Rename initial_presentation_delay_minus_one to
    initial_display_delay_in_samples_minus_1.

Closes #104

@tomfinegan tomfinegan requested a review from cconcolato October 10, 2018 17:16
@tomfinegan
Copy link
Collaborator Author

rawgit is shutting down; this will work:

https://htmlpreview.github.io/?https://github.com/AOMediaCodec/av1-isobmff/blob/a126a8c9e88f69ca49a18f720cb25e215eaa6cac/index.html

@cconcolato
Copy link
Collaborator

I'm not sure removing 'presentation' removes the confusion. #104 actually suggested renaming it to initial_display_delay_in_samples_minus_1. If we have to rename, I think I would prefer that.

@rbouqueau
Copy link

rbouqueau commented Oct 12, 2018 •
edited
Loading

I also think the new name is more confusing than the previous one. initial_display_delay_in_samples_minus_1 is better ; couldn't we re-use the show_frame e.g. initial_show_frame_delay_in_samples_minus_1 ?

@tomfinegan
Rename initial presentation delay fields.
d9786f1 
This editorial change renames the initial presentation
delay fields to avoid confusion between the ISOBMFF value,
which is in samples, and the AV1 bitstream value, which is
in frames.

- Rename initial_presentation_delay_present to
  initial_delay_present.
- Rename initial_presentation_delay_minus_one to
  initial_delay_samples_minus_one.

Closes #104
@tomfinegan
Longer names
d62b085
@tomfinegan
Copy link
Collaborator Author

tomfinegan commented Oct 12, 2018 •
edited
Loading

I also think the new name is more confusing than the previous one. initial_display_delay_in_samples_minus_1 is better ; couldn't we re-use the show_frame e.g. initial_show_frame_delay_in_samples_minus_1 ?

I think show_frame makes things more confusing than using display. I think this is more clear because the field carries the value of initial_display_delay_minus_1 in samples.

edit: add preview link, https://htmlpreview.github.io/?https://github.com/AOMediaCodec/av1-isobmff/blob/d62b085c6d190165d18059fc483e47b2573f9a32/index.html

@tomfinegan
Copy link
Collaborator Author

Dropping this as it doesn't really seem necessary.

@tomfinegan tomfinegan closed this May 28, 2019
@cconcolato cconcolato deleted the init_delay_rename branch October 9, 2021 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cconcolato cconcolato Awaiting requested review from cconcolato

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rename initial_presentation_delay to avoid further confusion
3 participants
@tomfinegan @cconcolato @rbouqueau