Closed Bug 1441145 Opened 6 years ago Closed 6 years ago

Wrong video stream resolution

Categories

(Core :: WebRTC: Audio/Video, defect, P2)

Product:
Core
Component:
WebRTC: Audio/Video
Platform:
ARM
Android
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 + verified
firefox60 + fixed

People

(Reporter: ohorvath, Assigned: pehrsons)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Bug 1441145 - Swap width and height in target capability if the frame is rotated.
59 bytes, text/x-review-board-request
jib
: review+
lizzard
: approval-mozilla-release+
Details
Bug 1441145 - Hack the frame rotation through to CamerasParent and CamerasChild.
59 bytes, text/x-review-board-request
jib
: review+
Details 
Device: Huawei Nexus 6P (Android 8.0)
Build: Nightly 60.0a1 (2018-02-26)

Steps to reproduce:
1. Go to https://jsfiddle.net/d8dw0sw9/8/
2. Start video streaming.
3. Check the resolution displayed.

Expected result:
The video should be displayed in 480x640 resolution.

Actual result:
The video is displayed in a 600x480 resolution (on current Nightly & Beta 59.0b12)
*480x480 on Nightly, after Bug 1439529 will be fixed.

Regression range:
Last good known build: 2017-12-04
First bad known build: 2017-12-06

Bisection results:
Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=741eed8ed628c81dec7e8d2b038f641591c71f9b&full=1
Found commit message:
Bug 1388219 - down scale camera output frame to the target capability. r=jib
MozReview-Commit-ID: 7dlbWXndbgf
Thanks!

On Samsung Galaxy S8 with current Nightly I see the below in the log. Note that due to bug 1439529 we consider 640x480 bad and instead ask for the next larger resolution, 736x736, which fails to start:

> 02-26 14:23:00.588  3350  3350 I ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-INFO(checkPreviewSize[1715]):[setParameters]param.preview size(736x736)
> 02-26 14:23:00.588  3350  3350 I ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-INFO(checkPreviewSize[1716]):[setParameters]Adjust Preview size(736x736), ratioId(2)
> 02-26 14:23:00.588  3350  3350 I ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-INFO(checkPreviewSize[1717]):[setParameters]Calibrated HwPreview size(736x736)
> 02-26 14:23:00.588  3350  3350 D ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-DEBUG(checkPreviewSize[1730]):setRestartPreviewChecked true
> 02-26 14:23:00.588  3350  3350 D ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-DEBUG(m_setRestartPreviewChecked[1548]):setRestartPreviewChecked(during SetParameters) true
> 02-26 14:23:00.588  3350  3350 D ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-DEBUG(checkHWVdisMode[2015]):[setParameters]newHwVdis 0
> 02-26 14:23:00.589  3350  3350 D ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-DEBUG(check3dnrMode[2885]):[setParameters]new3dnrMode false
> 02-26 14:23:00.589  3350  3350 D ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-DEBUG(checkDrcMode[2940]):[setParameters]newDrcMode false
> 02-26 14:23:00.589  3350  3350 D ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-DEBUG(checkOdcMode[2978]):[setParameters]newOdcMode false
> 02-26 14:23:00.589  3350  3350 D ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-DEBUG(checkPreviewFormat[1985]):[setParameters]newPreviewFormat: yuv420sp
> 02-26 14:23:00.589  3350  3350 E ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-ERR(m_isSupportedPictureSize[2689]):Invalid picture size(736x736)
> 02-26 14:23:00.589  3350  3350 E ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-ERR(checkPictureSize[2528]):Invalid picture size(736x736)
> 02-26 14:23:00.589  3350  3350 E ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-ERR(checkPictureSize[2535]):changed picture size to MAX(3264x2448)
> 02-26 14:23:00.589  3350  3350 E ExynosCamera1Parameters: [CAM_ID(1)][ParametersFront]-ERR(setParameters[638]): checkPictureSize fail
> 02-26 14:23:00.589  3350  3350 D ExynosCamera3Interface: DEBUG:duration time(    3 msec):(HAL_camera_device_set_parameters)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: startCapture failed
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: java.lang.RuntimeException: setParameters failed
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at android.hardware.Camera.native_setParameters(Native Method)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at android.hardware.Camera.setParameters(Camera.java:1940)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at org.webrtc.videoengine.VideoCaptureAndroid.startCaptureOnCameraThread(VideoCaptureAndroid.java:236)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at org.webrtc.videoengine.VideoCaptureAndroid.access$100(VideoCaptureAndroid.java:45)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at org.webrtc.videoengine.VideoCaptureAndroid$1.run(VideoCaptureAndroid.java:123)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at android.os.Handler.handleCallback(Handler.java:751)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at android.os.Handler.dispatchMessage(Handler.java:95)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at android.os.Looper.loop(Looper.java:154)
> 02-26 14:23:00.592 23558 23735 E WEBRTC-JC: 	at org.webrtc.videoengine.VideoCaptureAndroid$CameraThread.run(VideoCaptureAndroid.java:97)
> 02-26 14:23:00.592 23558 23705 D HelpersAndroid: Detaching thread from JVM@[tid=23705]
> 02-26 14:23:00.593 23558 23702 I Gecko   : 2018-02-26 13:23:00.593389 UTC - [23558:MediaManager]: D/MediaManager StartCapture failed

Odd that it fails, because it's listed as a capability:
> 02-26 14:23:00.403 23558 23702 I Gecko   : 2018-02-26 13:23:00.403072 UTC - [23558:MediaManager]: D/MediaManager ChooseCapability: prefs: 640x480 @30fps
> 02-26 14:23:00.403 23558 23702 I Gecko   : 2018-02-26 13:23:00.403104 UTC - [23558:MediaManager]: D/MediaManager Constraints: width: { min: -2147483647, max: 2147483647 }
> 02-26 14:23:00.403 23558 23702 I Gecko   : 2018-02-26 13:23:00.403132 UTC - [23558:MediaManager]: D/MediaManager              height: { min: -2147483647, max: 2147483647 }
> 02-26 14:23:00.403 23558 23702 I Gecko   : 2018-02-26 13:23:00.403163 UTC - [23558:MediaManager]: D/MediaManager              frameRate: { min: -inf, max: inf }
> 02-26 14:23:00.412 23558 23702 I Gecko   : 2018-02-26 13:23:00.412669 UTC - [23558:MediaManager]: D/MediaManager Capability: 1920 x 1080 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.412 23558 23702 I Gecko   : 2018-02-26 13:23:00.412765 UTC - [23558:MediaManager]: D/MediaManager Capability: 1440 x 1080 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.412 23558 23702 I Gecko   : 2018-02-26 13:23:00.412884 UTC - [23558:MediaManager]: D/MediaManager Capability: 1088 x 1088 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413003 UTC - [23558:MediaManager]: D/MediaManager Capability: 1072 x 1072 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413041 UTC - [23558:MediaManager]: D/MediaManager Capability: 1280 x  720 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413074 UTC - [23558:MediaManager]: D/MediaManager Capability: 1056 x  704 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413105 UTC - [23558:MediaManager]: D/MediaManager Capability:  960 x  720 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413135 UTC - [23558:MediaManager]: D/MediaManager Capability:  800 x  450 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413164 UTC - [23558:MediaManager]: D/MediaManager Capability:  736 x  736 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413193 UTC - [23558:MediaManager]: D/MediaManager Capability:  720 x  480 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413222 UTC - [23558:MediaManager]: D/MediaManager Capability:  640 x  480 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413251 UTC - [23558:MediaManager]: D/MediaManager Capability:  352 x  288 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413279 UTC - [23558:MediaManager]: D/MediaManager Capability:  320 x  240 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413306 UTC - [23558:MediaManager]: D/MediaManager Capability:  256 x  144 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413335 UTC - [23558:MediaManager]: D/MediaManager Capability:  176 x  144 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413409 UTC - [23558:MediaManager]: D/MediaManager Chosen capability:  640 x  480 x 30 maxFps, NV21, Unknown codec. Distance = 0
> 02-26 14:23:00.413 23558 23702 I Gecko   : 2018-02-26 13:23:00.413442 UTC - [23558:MediaManager]: D/MediaManager ChooseCapability(kFitness) for mCapability (Allocate) --

Really what I think is going wrong is that in the child we don't know that the camera is mounted sideways (270 degrees) so these capabilities are actually wrong. With bug 1439529 fixed we'd ask for 640x480 but start getting frames of 480x640. Since our target capability says 640x480 we try to crop the 480x640 frame to 640x480, instead ending up at 480x480.
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Rank: 15
status-firefox-esr52: --- → unaffected
tracking-firefox59: --- → ?
Component: Audio/Video → WebRTC: Audio/Video
Keywords: regression
Priority: -- → P2
Product: Firefox for Android → Core
Ah, this also depends on device rotation. Try unlocking rotation of your device in Android and rotate it. On 58 I get 480x720 in portrait and 720x480 in landscape.

It makes sense, but makes it harder to choose correct capabilities.

To just avoid this regression I think we have to pass the rotation to content so it can know how to interpret the target capability it has already chosen.
[Tracking Requested - why for this release]: serious functionality regression
tracking-firefox60: --- → ?
See Also: → 1441585
Do we need to fix this in 59, too?
Flags: needinfo?(jib)
Comment on attachment 8954901 [details]
Bug 1441145 - Swap width and height in target capability if the frame is rotated.

https://reviewboard.mozilla.org/r/224056/#review230060

Lgtm.
Attachment #8954901 - Flags: review?(jib) → review+
Comment on attachment 8954900 [details]
Bug 1441145 - Signal the capture-to-render rotation from parent to child.

https://reviewboard.mozilla.org/r/224054/#review230064

Lgtm.
Attachment #8954900 - Flags: review?(jib) → review+
We can still get this into 59 if you can request uplift to m-r after the patch lands on central/beta.
tracking-firefox59: ? → +
tracking-firefox60: ? → +
The existing patches here are a no-go because of a threading issue. Looking at how to fix this I don't think we can get anywhere near low enough risk for a 59 uplift.

If anyone wants to steal this, feel free. I'm putting it off for a while to take care of other burning issues.
Actually, we can make a small ugly hack and avoid that threading issue altogether. Yes, let me take a look at that.
Attachment #8954900 - Attachment is obsolete: true
Comment on attachment 8955527 [details]
Bug 1441145 - Hack the frame rotation through to CamerasParent and CamerasChild.

https://reviewboard.mozilla.org/r/224682/#review230834

Lgtm.

Bug 1442841 got in the way of testing this with multiple tabs (see that bug for the fiddles I used), but that bug happens without this patch, so we should be no worse off taking this patch.
Attachment #8955527 - Flags: review?(jib) → review+
I've tested this patch on OSX and Android. Will test Windows tomorrow or Monday. If someone could test linux that camera is not pear-shaped that would be great!
Flags: needinfo?(jib)
Pushed by jbruaroey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/844dff31a286
Hack the frame rotation through to CamerasParent and CamerasChild. r=jib
https://hg.mozilla.org/integration/autoland/rev/c672cca7633a
Swap width and height in target capability if the frame is rotated. r=jib
https://hg.mozilla.org/mozilla-central/rev/844dff31a286
https://hg.mozilla.org/mozilla-central/rev/c672cca7633a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment on attachment 8954901 [details]
Bug 1441145 - Swap width and height in target capability if the frame is rotated.

This approval request applies to both patches.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1388219
[User impact if declined]: rotated camera or device (typically android) results in wrong resolution of camera stream
[Is this code covered by automated tests?]: the non-rotated case is covered, the regressed rotated case is not.
[Has the fix been verified in Nightly?]: yes, I just tested on latest nightly for android
[Needs manual test from QE? If yes, steps to reproduce]: yes. See comment 0. Also try to rotate the device during the capture.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: first, this added logic only kicks in when the camera sensor is rotated in relation to the screen. Second, it's simple and only swaps width and height as input to a scaling algorithm. Worst fallout would be that the rendered frames end up being too small.
[String changes made/needed]: none
Attachment #8954901 - Flags: approval-mozilla-release?
Blocks: 1388219
(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #16)
> I've tested this patch on OSX and Android. Will test Windows tomorrow or
> Monday. If someone could test linux that camera is not pear-shaped that
> would be great!

Linux looks good on a March 4th Nightly Ubuntu build.
> [User impact if declined]: rotated camera or device (typically android) results in wrong resolution of camera stream

To clarify: Not just incorrect resolution but visibly pear-shaped dimensions (e.g. 480x480 instead of 640x480).
Comment on attachment 8954901 [details]
Bug 1441145 - Swap width and height in target capability if the frame is rotated.

Looks like the risk is limited in scope to WebRTC, fixes a new regression in 59, so let's get this uplifted to m-r.
Attachment #8954901 - Flags: approval-mozilla-release? → approval-mozilla-release+
Verified as fixed on Beta 59b15.
Devices:
HTC 10 (Android 8.0)
Huawei P9 Lite (Android 6.0)
Samsung Galaxy S8 (Android 7.0)
Status: RESOLVED → VERIFIED
status-firefox59: fixed → verified
Also verified on RC 59.0.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: