WebRTC video with odd frame width not rendered properly

VERIFIED FIXED in Firefox 37

Status

()

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: cesar.guirao, Assigned: cesar.guirao)

Tracking

Trunk
mozilla39
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox36 wontfix, firefox37+ fixed, firefox38+ verified, firefox39 verified, firefox-esr31 unaffected, firefox-esr38 fixed)

Details

(Whiteboard: [webrtc-uplift])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
Component: Untriaged → WebRTC
Product: Firefox → Core
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.
Attachment #8572170 - Attachment is patch: true
Attachment #8572170 - Attachment mime type: text/x-patch → text/plain
Attachment #8572170 - Flags: review?(rjesup)
Blocks: 1132891
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
Duplicate of this bug: 1132891
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: --- → ?
Whiteboard: [webrtc-uplift]
https://hg.mozilla.org/mozilla-central/rev/4930072a9b28
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
Attachment #8572170 - Flags: approval-mozilla-beta?
Attachment #8572170 - Flags: approval-mozilla-aurora?
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.
Attachment #8572170 - Flags: approval-mozilla-beta?
Attachment #8572170 - Flags: approval-mozilla-beta+
Attachment #8572170 - Flags: approval-mozilla-aurora?
Attachment #8572170 - Flags: approval-mozilla-aurora+
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.