If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Volume option is displayed as disabled although is enabled

VERIFIED FIXED in Firefox 40

Status

()

Core
Audio/Video
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: adalucinet, Assigned: jya)

Tracking

({regression})

Trunk
mozilla42
regression
Points:
---

Firefox Tracking Flags

(firefox39 unaffected, firefox40+ verified, firefox41 verified, firefox42 verified, firefox-esr38 unaffected)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Reproducible on latest 40.0b3 (Build ID: 20150709163524), latest 41.0a2 and 42.0a1 (both from 2015-07-09)
Affected platforms: Ubuntu 14.04 32-bit, Windows 10 32-bit, Mac OS X 10.8.5

Prerequisites: Camera and microphone available.

Steps to reproduce:
1. Launch Firefox and navigate to http://mozilla.github.io/webrtc-landing/gum_test.html
2. Click on Audio&Video button and share devices when prompted.
3. Right click on video area and select Show controls.

Expected results: All the available options from the container's controls bar are properly displayed.
Actual results: Volume is displayed as disabled.

Additional notes:
1. Screenshot showing this issue: https://goo.gl/pnhiUx
2. Not reproducible with 39.0 RC build 6 (Build ID: 20150630154324): https://goo.gl/hyJ9Ig
(Reporter)

Comment 1

2 years ago
Also reproducible with 40.0b1 (Build ID: 20150702173756); will investigate further for the regression range.

Comment 2

2 years ago
[Tracking Requested - why for this release]:
Sounds like a bug we should not ship to release.
status-firefox40: --- → affected
status-firefox41: --- → affected
tracking-firefox40: --- → ?
This is not a bug because the video container seen on this page is simply displaying the video frames that are coming in. The video does not have audio being played back, which is why the controls are showing that audio is not available (crossed out/disabled).

The final output of the WebRTC page will have the streams merged, but the "preview" doesn't merge them.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INVALID
status-firefox40: affected → ---
status-firefox41: affected → ---
status-firefox42: affected → ---
tracking-firefox40: ? → ---
Hmm, well...

As comment 0 notes, this works in Firefox 39. When I mute with the video element's controls, the audio mutes. So on 39, audio output can definitely going to the video element. 

[I'm a little confused, looking at the source, because it seems sometimes a separate audio element is being created... But I don't get one in the page, looks like that's maybe only intended to be used when ONLY audio is being used?]

The test page is broken for me in my running 7/7 Nightly (nothing seen/heard on the page, no errors in console), but works in a new profile. The controls have the mute button disabled, but the content menu has Mute command enabled and it successfully mutes the video. So audio is being streamed through the <video> element and can be muted, but we're not showing the UI.

Something in WebRTC / DOM media is incorrectly setting the mozHasAudio property on the video element now -- it's false, when I clearly hear audio. So that's why the controls show as disabled.

[Also, we should probably be checking .mozHasAudio in the context menu, and disabling Mute when it's not available.]
Status: RESOLVED → REOPENED
Component: Video/Audio Controls → WebRTC: Audio/Video
Flags: needinfo?(rjesup)
Product: Toolkit → Core
Resolution: INVALID → ---
Filed bug 1182606 for the context menu.
(In reply to Justin Dolske [:Dolske] from comment #4)
> Hmm, well...
> 
> As comment 0 notes, this works in Firefox 39. When I mute with the video
> element's controls, the audio mutes. So on 39, audio output can definitely
> going to the video element. 
> 
> [I'm a little confused, looking at the source, because it seems sometimes a
> separate audio element is being created... But I don't get one in the page,
> looks like that's maybe only intended to be used when ONLY audio is being
> used?]

Correct.  And when you select audio+video, the <video> element should be getting both from gUM.

And in Nightly (7/7, 7/8) windows, in gum_test.html, if I right-click and Mute, it mutes, and UnMute, it unmutes (and I can hear audio, at least in a headset).  Note that audio will sound funny in some cases since the AEC will be fighting you....

However, I agree that Show Controls is showing audio as disabled, and it doesn't mute/unmute if you click it (though the right-button context menu does).

> The test page is broken for me in my running 7/7 Nightly (nothing seen/heard
> on the page, no errors in console), but works in a new profile. The controls
> have the mute button disabled, but the content menu has Mute command enabled
> and it successfully mutes the video. So audio is being streamed through the
> <video> element and can be muted, but we're not showing the UI.

Right.  (Though I'm interested how your profile broke it... perhaps you set a disable on gum in about:config?  (navigator.something pref)

> Something in WebRTC / DOM media is incorrectly setting the mozHasAudio
> property on the video element now -- it's false, when I clearly hear audio.
> So that's why the controls show as disabled.

Perhaps it has to do with how this is set with a mozSrcObject stream?  This appears to come from MediaInfo::HasAudio() which is AudioInfo::IsValid(), which is mChannels > 0 && mRate > 0.  But I haven't gdb'd it to be sure.

> [Also, we should probably be checking .mozHasAudio in the context menu, and
> disabling Mute when it's not available.]

yes.
Component: WebRTC: Audio/Video → Video/Audio
Flags: needinfo?(rjesup)
Flags: needinfo?(jyavenard)
Flags: needinfo?(cpearce)
This is a regression right? A range would be helpful to pinpoint what changed here.
Flags: needinfo?(cpearce)
Keywords: regressionwindow-wanted
(Assignee)

Comment 8

2 years ago
This is likely a result of bug 1153049; when I removed what I thought were redundant member (and for normal media playback they certainly were)
Flags: needinfo?(jyavenard)
[Tracking Requested - why for this release]:
Important usability regression

Set flags based on presumption that bug 1153049 is the cause and is on 40 and later.

jya - can you take this?  We'll need uplift to beta
Blocks: 1153049
status-firefox39: --- → unaffected
status-firefox40: --- → affected
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox-esr38: --- → unaffected
tracking-firefox40: --- → ?
Flags: needinfo?(jyavenard)
Keywords: regression
(Reporter)

Comment 10

2 years ago
Regression range:

(m-c): 
Last good revision: 2c9708e6b54d
First bad revision: 2ee2da378d12
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2c9708e6b54d&tochange=2ee2da378d12

(m-i): 
Last good revision: e17f5910da75
First bad revision: 65492fe83362
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e17f5910da75&tochange=65492fe83362
Keywords: regressionwindow-wanted
(Assignee)

Updated

2 years ago
Assignee: nobody → jyavenard
Flags: needinfo?(jyavenard)
Regression so let's track it.
tracking-firefox40: ? → +
(Assignee)

Comment 12

2 years ago
Created attachment 8634625 [details] [diff] [review]
Show audio as enabled when an audio track is present in stream.

The HTML MediaElement's audio info configuration isn't set via a call to MetadataLoaded as it normally would with standard media playback when streams are in use.
Ideally, we would want to set the MediaInfo object like we do for video with a call to HTMLMediaElement::UpdateMediaSize.
However, this would be a much greater change and the aim of this one is to make it as simple and quick as possible so it can be uplifted to beta.
Attachment #8634625 - Flags: review?(rjesup)

Updated

2 years ago
Attachment #8634625 - Flags: review?(rjesup) → review+

Comment 13

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7e2eb0b375c
https://hg.mozilla.org/mozilla-central/rev/c7e2eb0b375c
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Looks like a straightforward change. This is a regression in 40 and 40 and 41 are marked as affected. What do you think about uplifting to Beta and Aurora?
Flags: needinfo?(jyavenard)
(Assignee)

Comment 16

2 years ago
Comment on attachment 8634625 [details] [diff] [review]
Show audio as enabled when an audio track is present in stream.

Approval Request Comment
[Feature/regressing bug #]: bug 1153049
[User impact if declined]: Audio will appear as muted but is active. Audio can't be muted
[Describe test coverage new/current, TreeHerder]: In m-c, local tests
[Risks and why]: Low. Though not what I would call an ideal fix, it was designed to have the least impact and lowest chance of regression.
[String/UUID change made/needed]: None
Flags: needinfo?(jyavenard)
Attachment #8634625 - Flags: approval-mozilla-beta?
Attachment #8634625 - Flags: approval-mozilla-aurora?
(In reply to Jean-Yves Avenard [:jya] from comment #16)
> [Describe test coverage new/current, TreeHerder]: In m-c, local tests

Given that the patch involves setting dummy values for for a function to return true, I'd prefer to see this fix verified before uplifting.

Florin - Does you team have time to verify this fix today?
Flags: needinfo?(florin.mezei)
Verified fixed on Windows 7 64bit, Mac OSX 10.9.5 and Ubuntu 14.04 32bit using latest Nightly 42.0a1 (buildID: 20150720030213).
Status: RESOLVED → VERIFIED
status-firefox42: fixed → verified
Flags: needinfo?(florin.mezei)
Comment on attachment 8634625 [details] [diff] [review]
Show audio as enabled when an audio track is present in stream.

The fix has been verified. Thanks to QE for the quick turnaround. Let's get this into beta6. Beta+ Aurora+
Attachment #8634625 - Flags: approval-mozilla-beta?
Attachment #8634625 - Flags: approval-mozilla-beta+
Attachment #8634625 - Flags: approval-mozilla-aurora?
Attachment #8634625 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/20553f5788f3
status-firefox41: affected → fixed
https://hg.mozilla.org/releases/mozilla-beta/rev/e28e902ee47b
status-firefox40: affected → fixed
Setting this for quick verification in Firefox 40 Beta.
Flags: qe-verify+
(Assignee)

Comment 23

2 years ago
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #17)
> Given that the patch involves setting dummy values for for a function to
> return true, I'd prefer to see this fix verified before uplifting.

This is pretty much what the original code was doing, except that it stored a boolean to say "active"

But yeah, it's pretty ugly regardless.
(Reporter)

Comment 24

2 years ago
Verified fixed with 40.0b6 (Build ID: 20150720220238) and latest 41.0a2 (from 2015-07-21), across platforms [1].

[1] Ubuntu 14.04 32-bit, Mac OSX 10.10.4, Windows 7 64-bit and Windows 10 32-bit
status-firefox40: fixed → verified
status-firefox41: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.