Closed Bug 1139132 Opened 8 years ago Closed 8 years ago

WebRTC video with odd frame width not rendered properly


(Core :: WebRTC, defect)

Not set



Tracking Status
firefox36 --- wontfix
firefox37 + fixed
firefox38 + verified
firefox39 --- verified
firefox-esr31 --- unaffected
firefox-esr38 --- fixed


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



(Whiteboard: [webrtc-uplift])


(1 file)

Attached patch Patch fileSplinter Review
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
Ever confirmed: true

[Tracking Requested - why for this release]:
Simple patch, fixes a problem with general screenshared streams if the width is odd
Assignee: nobody → cesar
Closed: 8 years ago
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.
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).

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).
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.
You need to log in before you can comment on or make changes to this bug.