Bug 172908

Summary: Fix MediaPlayerPrivateMediaFoundation::naturalSize
Product: WebKit Reporter: Konstantin Tokarev <annulen>
Component: WebCore Misc.Assignee: Konstantin Tokarev <annulen>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, pvollan
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=198555
https://bugs.webkit.org/show_bug.cgi?id=198640
Attachments:
Description Flags
Patch achristensen: review+

Konstantin Tokarev
Reported 2017-06-04 13:45:22 PDT
naturalSize should return size of video frame corrected by PAR, not size of video element.
Attachments
Patch (5.19 KB, patch)
2017-06-04 13:49 PDT, Konstantin Tokarev 		achristensen: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Konstantin Tokarev
Comment 1 2017-06-04 13:49:40 PDT
Created attachment 311972 [details] Patch
Alex Christensen
Comment 2 2017-06-05 07:10:23 PDT
Comment on attachment 311972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311972&action=review > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:1563 > + FloatSize size(width, height); > + size.setWidth(size.width() * parNumerator / parDenominator); > + return size; Can't we just return FloatSize(width * parNumerator / parDenominator, height)? Also, parNumerator and parDenominator could have better names.
Konstantin Tokarev
Comment 3 2017-06-05 07:15:13 PDT
Comment on attachment 311972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311972&action=review >> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:1563 >> + return size; > > Can't we just return FloatSize(width * parNumerator / parDenominator, height)? > Also, parNumerator and parDenominator could have better names. I can change it to return FloatSize(float(width) * parNumerator / parDenominator, height) What kind of better names do you have in mind? Expand "par" to "pixelAspectRatio"?
Alex Christensen
Comment 4 2017-06-05 13:09:58 PDT
Sure. I initially thought it was an abbreviation of parameter.
Konstantin Tokarev
Comment 5 2017-06-05 14:45:31 PDT
Committed r217795: <http://trac.webkit.org/changeset/217795>
Note You need to log in before you can comment on or make changes to this bug.