pion / mediadevices

Go implementation of the MediaDevices API.
https://pion.ly/
MIT License
528 stars 121 forks source link

Fix scaling functions #385

Closed f-fl0 closed 2 years ago

f-fl0 commented 2 years ago

Description

This PR corrects two small errors that causes the improper results reported in the referenced issue:

Reference issue

Fixes https://github.com/pion/mediadevices/issues/312

codecov[bot] commented 2 years ago

Codecov Report

Merging #385 (466d14b) into master (1f92ea4) will increase coverage by 0.74%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #385      +/-   ##
==========================================
+ Coverage   46.42%   47.16%   +0.74%     
==========================================
  Files          67       67              
  Lines        4373     4380       +7     
==========================================
+ Hits         2030     2066      +36     
+ Misses       2220     2189      -31     
- Partials      123      125       +2     
Impacted Files Coverage Δ
pkg/io/video/scale.go 66.12% <100.00%> (ø)
pkg/io/video/scaleycrcb.go 92.30% <100.00%> (+2.83%) :arrow_up:
pkg/io/video/scaler_cgo.go 60.55% <0.00%> (+26.60%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1f92ea4...466d14b. Read the comment docs.

f-fl0 commented 2 years ago

I found another problem when using the nearest neighbor scaling function when the same size as the input image: image

It is demonstrated by the test added in d56d4fb42127acf4b34b821c81aca55dc453f99b. It is fixed by 89d120e98551fb54d218c8d1935910ef69e6b921.

f-fl0 commented 2 years ago

The need for the fix introduced in ee67dee4b812bb68e026cfb0b57115d9fb8a1b45 is demonstrated by the test added in 5f2216ace1313f2e521e9fe4b5ee0b82bff0f590.

f-fl0 commented 2 years ago

The need for the fix introduced in ad6b0ad5e1c7878befe0dca92d82e09b3dae345d is demonstrated by the test added in edef5f5dee11be34eb8c12b719064b2764b6fd4d

f-fl0 commented 2 years ago

@at-wat I added some tests and tried to demonstrate they would fail on master by reverting each change and let CI fail.