[B2G getUserMedia] Workaround MediaRecorder-can't-handle-rotation for PeerConnection calls

RESOLVED FIXED in Firefox 32, Firefox OS v2.0

Status

()

Core
WebRTC: Audio/Video
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla33
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [ft:webrtc])

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
+++ This bug was initially created as a clone of Bug #970183 +++

We disabled dynamic screen rotation in bug 970183 until we can fix MediaRecorder.  In the meantime, we may be able to workaround the issue for PeerConnection (webrtc calls) by keying off the direct-realtime-consumer state of the track (i.e. allow rotation if there's a direct consumer in a peerconnection).  Note that this *might* (for a small number of apps) cause confusion for the user if they rotate after getUserMedia and before attaching to the peerconnection - the image won't dynamic rotate until the stream is hooked into a peerconnection.  (I.e. if the app provides "hair-check mode" before starting a call).
(Assignee)

Updated

3 years ago
Assignee: nobody → rjesup
(Assignee)

Comment 1

3 years ago
Created attachment 8454914 [details] [diff] [review]
patch 1 - Add notification of DirectListeners and generalize Notification of events
(Assignee)

Comment 2

3 years ago
Created attachment 8454915 [details] [diff] [review]
patch 2 - Enable dynamic rotation in gUM when we're hooked to a PeerConnection
(Assignee)

Comment 3

3 years ago
Comment on attachment 8454914 [details] [diff] [review]
patch 1 - Add notification of DirectListeners and generalize Notification of events

I decided to genericize Notification of simple events instead of adding more virtual entries to every subclass.  With the proliferation of Notification consumers of MSG, adding new virtual entries each time means a lot of files to modify that don't care about the new notification (like I had to do in this patch).  Alternatively, we could just add NotifyHasDirectListeners(bool) (or two, but that would be really silly), or we could make it have an optional argument and move all the state-notifications into one callback and reduce the number of NotifyFoo()'s.

It's relatively easy either way.

Note this is used to implement the workaround here (block dynamic rotation except when we're hooked directly to a PeerConnection) to avoid confusing MediaRecorder.
Attachment #8454914 - Flags: review?(roc)
(Assignee)

Comment 4

3 years ago
Comment on attachment 8454915 [details] [diff] [review]
patch 2 - Enable dynamic rotation in gUM when we're hooked to a PeerConnection

This blocks dynamic rotation except when hooked to a PeerConnection.  The only theoretical hole here would be if you tried to both record a gum stream at the same time you're sending it on a PeerConnection.  We can relnote that, since that's at best a very minor usecase.

Note that self-images shown before or after a call will NOT dynamically rotate - they'll rotate only while in a call.

Tested on Flame.
Attachment #8454915 - Flags: review?(jolin)
Comment on attachment 8454914 [details] [diff] [review]
patch 1 - Add notification of DirectListeners and generalize Notification of events

Review of attachment 8454914 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/MediaStreamGraph.cpp
@@ +2366,5 @@
>  
>  void
>  SourceMediaStream::AddDirectListener(MediaStreamDirectListener* aListener)
>  {
> +  bool was_empty;

wasEmpty, isEmpty
Attachment #8454914 - Flags: review?(roc) → review+
Attachment #8454915 - Flags: review?(jolin) → review+
(Assignee)

Comment 6

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1783fc9c59a
https://hg.mozilla.org/integration/mozilla-inbound/rev/39f5872c10e7
Target Milestone: --- → mozilla33
(Assignee)

Comment 7

3 years ago
Nominating for 2.0 since we're not getting a fix for bug 970183 for 2.0; this works around (mostly) the removal of dynamic rotation.  I believe lack of dynamic rotation will be considered a noticeable bug/annoyance by users of FxOS devices and partners developing video-call applications.
blocking-b2g: --- → 2.0?
Whiteboard: [ft:webrtc] → [ft:webrtc][webrtc-uplift]
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/73e71a89498b
unused var bustage fix

Updated

3 years ago
blocking-b2g: 2.0? → 2.0+
https://hg.mozilla.org/mozilla-central/rev/e1783fc9c59a
https://hg.mozilla.org/mozilla-central/rev/39f5872c10e7
https://hg.mozilla.org/mozilla-central/rev/73e71a89498b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/0e4e020dbe8a
https://hg.mozilla.org/releases/mozilla-aurora/rev/53075e07875f
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
status-firefox31: --- → wontfix
status-firefox32: --- → fixed
status-firefox33: --- → fixed
(Assignee)

Updated

3 years ago
Whiteboard: [ft:webrtc][webrtc-uplift] → [ft:webrtc]
You need to log in before you can comment on or make changes to this bug.