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

frame: plane: Fix Downsampling of odd sized frames #2095

Merged
merged 3 commits into from
Feb 6, 2020

Conversation

vibhoothi
Copy link
Collaborator

Hi,

Currently, the hres and qres are talking half(1/2) and one-fourth(1/4) of the
width and height which is rounded down, when we have an odd-sized frame this becomes
a problem so to solve that we are rounding one up for both width and height.

Earlier: src.cfg.width / 2
Now : (src.cfg.width + 1) / 2,

Fixes #2055

I am not 100% if doing this causes any other harm, but it would be great, to hear the comments with this

@vibhoothi
Copy link
Collaborator Author

vibhoothi commented Jan 17, 2020

https://beta.arewecompressedyet.com/?job=%402020-01-17T15%3A01%3A19.232Z&job=master-caef43f3fcadcde053ddd6c49caef188724209ea

Did AWCY and noticed there is a very small encoding time improving by less than a second or second which is neglible,

Good news is nothing is broken.

Copy link
Collaborator

@rzumer rzumer left a comment

Choose a reason for hiding this comment

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

LGTM

@tdaede
Copy link
Collaborator

tdaede commented Jan 17, 2020

Could you make a downsampling test that shows this? See the existing downsample test - make an odd sized ones.

src/frame/plane.rs Outdated Show resolved Hide resolved
src/frame/plane.rs Outdated Show resolved Hide resolved
Currently, the hres and qres are talking half(1/2) and one-fourth(1/4) of the
width and height which is rounded down, when we have an odd sized frame this
becomes a problem so to solve that we are rounding one up for both width and
height.

Earlier: src.cfg.width / 2
Now    : (src.cfg.width + 1) / 2,

Fixes xiph#2055
Copy link
Collaborator

@barrbrain barrbrain left a comment

Choose a reason for hiding this comment

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

The test case describes the expectation that padding pixels be used when down-sampling planes of odd dimensions. This requires that padding pixels are available, which is true of width if both stride and alignment are even. Odd heights are less constrained, so some care may be required. In practice, we substantially pad the frames to simplify motion estimation.

src/frame/plane.rs Show resolved Hide resolved
vibhoothi and others added 2 commits February 6, 2020 18:38
Here we have modified the test case since we are considering odd heights,
the exceptional case is where the right or bottom padding are zero causing
additional pixels to be read from the next row or past the end of the data

Co-authored-by: David Michael Barr <b@rr-dav.id.au>
Copy link
Collaborator

@barrbrain barrbrain left a comment

Choose a reason for hiding this comment

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

This sufficiently describes the expectations and limitations of downsampling planes with odd dimensions. We ought to pad the input frames before downsampling - I have not yet traced through all the frame creation code to determine whether we already do that.

@takehirokj takehirokj merged commit 5be4720 into xiph:master Feb 6, 2020
barrbrain added a commit to barrbrain/rav1e that referenced this pull request Oct 5, 2022
* frame: plane: Fix Downsampling of odd sized frames

Currently, the hres and qres are talking half(1/2) and one-fourth(1/4) of the
width and height which is rounded down, when we have an odd sized frame this
becomes a problem so to solve that we are rounding one up for both width and
height.

Earlier: src.cfg.width / 2
Now    : (src.cfg.width + 1) / 2,

Fixes xiph#2055

* frame: plane: Introduce new test case for down_sampling

Here we have modified the test case since we are considering odd heights,
the exceptional case is where the right or bottom padding are zero causing
additional pixels to be read from the next row or past the end of the data

Co-authored-by: David Michael Barr <b@rr-dav.id.au>

* benches: plane: Introduce new bench with odd frames

Co-authored-by: David Michael Barr <b@rr-dav.id.au>
barrbrain added a commit to barrbrain/rav1e that referenced this pull request Oct 5, 2022
* frame: plane: Fix Downsampling of odd sized frames

Currently, the hres and qres are talking half(1/2) and one-fourth(1/4) of the
width and height which is rounded down, when we have an odd sized frame this
becomes a problem so to solve that we are rounding one up for both width and
height.

Earlier: src.cfg.width / 2
Now    : (src.cfg.width + 1) / 2,

Fixes xiph#2055

* frame: plane: Introduce new test case for down_sampling

Here we have modified the test case since we are considering odd heights,
the exceptional case is where the right or bottom padding are zero causing
additional pixels to be read from the next row or past the end of the data

Co-authored-by: David Michael Barr <b@rr-dav.id.au>

* benches: plane: Introduce new bench with odd frames

Co-authored-by: David Michael Barr <b@rr-dav.id.au>
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.

Fix downsampling on odd sized frames
5 participants