[webkit-reviews] review denied: [Bug 34790] [Gtk] wrong video aspect ratio : [Attachment 48508] proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 10 09:56:07 PST 2010


Gustavo Noronha (kov) <gns at gnome.org> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 34790: [Gtk] wrong video aspect ratio
https://bugs.webkit.org/show_bug.cgi?id=34790

Attachment 48508: proposed patch
https://bugs.webkit.org/attachment.cgi?id=48508&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
 62 gint greatestCommonDivisor(gint a, gint b)

No need to use gint for these. I think xan will agree =D

 491	 int width = 0, height = 0;
 492	 GstCaps* caps = GST_PAD_CAPS(pad);
 493	 gint pixelAspectRatioNumerator, pixelAspectRatioDenominator;
 494	 gint displayWidth, displayHeight, darGcd;

Same here. Also GCD is an acronym, so it should go all upper-cased. I would
also prefer to see displayAspectRatioGCD.

I wonder if we can include a layout test with this one. We used to have this
code added to fix a layout test, if I remember correctly, so we should be able
to use it as a base, provided that we can add a video that triggers it to the
tree. r- so we can have that test, as discussed on IRC =)


More information about the webkit-reviews mailing list