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

Humble suggestion. #333

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

Humble suggestion. #333

wants to merge 3 commits into from

Conversation

cabelo
Copy link

@cabelo cabelo commented Sep 15, 2022

No description provided.

@peterderivaz
Copy link
Collaborator

These all look like sensible edits, thanks!

Based on previous issues, I suspect AOMedia will be reluctant to make any purely cosmetic changes to the AV1 spec (even when they are clear improvements), but very interested in making the corresponding improvements to the AV2 specification.

@alexismt73
Copy link

These seem to be all editorial. I see no issue in accepting them since they do not impact any implementation.

@imoccaga
Copy link

I see three sets of suggested editorial changes:

  1. 0e347d7
  2. 2fd82d1
  3. ca995e1

All of them are editorial, so I do not see a problem in making these changes in the next edition of AV1 spec, and porting the same to AV2 spec.

@peterderivaz
Copy link
Collaborator

Rereading the changes I am a bit concerned about the following text:

TotalDecodedLumaSampleRate is defined as the sum of the UpscaledWidth *
FrameHeight of all frames with show_existing_frame equal to 0 that belongs
to the temporal unit that belongs to the operating point, divided by the...

The verb "belong" has been changed to "belongs" in the suggested change. To my eyes the subject is "all frames". This is plural so in my opinion the verb should be conjugated as "belong" (as in the original text).

@alexismt73
Copy link

I agree with Peter. Also some of the suggestions do not appear to be the best (briefly checked on my phone and instead of saying “sizes of the matrix”, “matrix sizes” sounds way better to me. These need to be more carefully checked.

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.

4 participants