Created attachment 8572170 [details] [diff] [review] Patch file User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.115 Safari/537.36 Steps to reproduce: Subscribe to a WebRTC video stream with an odd frame width (e.g. 641) Actual results: Video not rendered properly. The colors of the video are not correct because the chroma is displaced. Expected results: Video rendered as expected.
Comment on attachment 8572170 [details] [diff] [review] Patch file Pushing to Randell to take a look at as I think he's been looking at things in this area.
Comment on attachment 8572170 [details] [diff] [review] Patch file Review of attachment 8572170 [details] [diff] [review]: ----------------------------------------------------------------- That looks like it! Cool. Explains why I didn't see it in gum_test
Attachment #8572170 - Flags: review?(rjesup) → review+
And why I didn't see it when looking for /2's
Status: UNCONFIRMED → NEW
Ever confirmed: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/4930072a9b28 [Tracking Requested - why for this release]: Simple patch, fixes a problem with general screenshared streams if the width is odd
status-firefox36: --- → wontfix
status-firefox37: --- → affected
status-firefox38: --- → affected
status-firefox39: --- → affected
status-firefox-esr31: --- → unaffected
status-firefox-esr38: --- → affected
tracking-firefox37: --- → ?
tracking-firefox38: --- → ?
Assignee: nobody → cesar
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox39: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8572170 [details] [diff] [review] Patch file Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: Sharing odd-sized sources may result in torn color rendering (or perhaps no rendering in some cases). [Describe test coverage new/current, TreeHerder]: Currently requires manual testing as specially-sizes sources are needed. [Risks and why]: Very low risk. Basically adds one to width/height before dividing by 2 to get the right chroma size. [String/UUID change made/needed]: none
This seems bad for WebRTC screen sharing. We definitely need this in 38 (when Hello will ship screen sharing) but I think we really should take the fix in 37 as well if we can. Tracking.
tracking-firefox37: ? → +
tracking-firefox38: ? → +
Comment on attachment 8572170 [details] [diff] [review] Patch file Approved for Beta and Aurora. Note to sheriffs: jesup will handle uplifts himself.
As requested by mreavy, verified on Nightly and Aurora In the process I figured out why it's hard to repro: In VideoConduit.cpp, we sanitize the width and height to be even (which hides this bug). However, load/bitrate adaptation can force the new width/height to be odd, triggering what Mark saw occasionally. We'll uplift this, and leave the force-to-even code in place a while longer to avoid issues with people on beta/aurora sharing to someone on Release and triggering the bug. Once enough people have the reception fix, we can loosen up the sending side. There's little downside to forcing the sender to be even for now (a slight blurring due to scaling when sharing an odd-sized window, and we may be doing that anyways).
https://hg.mozilla.org/releases/mozilla-aurora/rev/646122234d90 https://hg.mozilla.org/releases/mozilla-beta/rev/19ac18d33c28 One other note: without this patch, a debug build receiving an odd size may assert (rendering the image) instead of rendering incorrectly as seen by Mark (he was using non-debug builds I assume when he saw it).
status-firefox37: affected → fixed
status-firefox38: affected → fixed
status-firefox-esr38: affected → fixed
Verified on 38 and 39 based on comment 10. I don't think this needs manual verification on 38.0.5 or ESR 38.0... please correct me if I'm wrong.
Status: RESOLVED → VERIFIED
status-firefox38: fixed → verified
status-firefox39: fixed → verified
You need to log in before you can comment on or make changes to this bug.