Issue metadata
Sign in to add a comment
|
MediaStream media player requires a video frame before audio is started even when the local video track is disabled
Reported by
a...@tokbox.com,
Feb 16 2018
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.51 Safari/537.36 Steps to reproduce the problem: 1. Visit https://output.jsbin.com/hipoker in 2 browser tabs in Chrome 65 2. Allow access to the camera/microphone 3. Wait 15 seconds What is the expected behavior? You can hear your audio being streamed and you don't get any errors What went wrong? You get a timeout error waiting for the loadedmetadata event to come through Did this work before? Yes 64 Does this work in other browsers? Yes Chrome version: 65.0.3325.51 Channel: beta OS Version: OS X 10.12.6 Flash Version: This is a regression in Chrome 65. Prior to Chrome 65 we were able to disable the local video track on the media stream and audio would play. Now this no longer works and we never get any audio playing and the loadedmetadata event never fires. Details of this workaround are from https://bugs.chromium.org/p/chromium/issues/detail?id=403710#c8 In a peer to peer Chrome to Chrome scenario black video frames are sent when the video track is muted on the sender side. But when the peer connection is going through our SFU we do not forward those video frames if the video is disabled. Also, it is possible that there are other WebRTC endpoints that when video is disabled they do not send black video frames. Another workaround was suggested at https://bugs.chromium.org/p/chromium/issues/detail?id=403710#c39 however for us to use this workaround we would need to update all of our customers applications before Chrome 65 is released which is something that is not easy or ideal. Given this is a change of behaviour we would like to see it fixed before 65 is released.
,
Feb 17 2018
,
Feb 17 2018
Re comment #1: That sounds likely. I looked at the CL and it's not clear to me if that change is the right fix for the 2x2 issue. Is there a chance that the 2x2 issue could be fixed differently and we can simply revert that CL?
,
Feb 17 2018
See discussion in https://crbug.com/403710 - not playing audio before you know the dimensions of the video seems to be a requirement of the specs for <video>.
,
Feb 19 2018
See https://bugs.chromium.org/p/chromium/issues/detail?id=403710#c45 for further comment. For clarity, guys, what's the plan to fix the regression ?
,
Feb 19 2018
Let's revert r523152 and merge the revert to M65. Then reland the fix in a way that does not cause a regression.
,
Feb 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1f07e92571d6a2db9ee116c267c2963f7b091ba commit d1f07e92571d6a2db9ee116c267c2963f7b091ba Author: Guido Urdaneta <guidou@chromium.org> Date: Mon Feb 19 18:14:35 2018 Revert "Don't trigger end of track when starting a new disabled track" This reverts commit 447c696aaf19064a79d7bc894ce4a0aa3422c69c. Reason for revert: Introduces a regression. See https://crbug.com/813243 Original change's description: > Don't trigger end of track when starting a new disabled track > > Fix reporting a size of 2x2 on the video element during the loading > phase. > > Bug: 684288 > Change-Id: Id4adceefaf4c3df57a1ba41fd285862d72792b1a > Reviewed-on: https://chromium-review.googlesource.com/806215 > Commit-Queue: Florent Castelli <orphis@chromium.org> > Commit-Queue: Guido Urdaneta <guidou@chromium.org> > Reviewed-by: Guido Urdaneta <guidou@chromium.org> > Cr-Commit-Position: refs/heads/master@{#523152} TBR=guidou@chromium.org,orphis@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 684288, 813243 Change-Id: I60bc092c23d1840a9c7e95b4b1d8d02c52c6fb46 Reviewed-on: https://chromium-review.googlesource.com/924190 Commit-Queue: Guido Urdaneta <guidou@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#537675} [modify] https://crrev.com/d1f07e92571d6a2db9ee116c267c2963f7b091ba/content/browser/webrtc/webrtc_getusermedia_browsertest.cc [modify] https://crrev.com/d1f07e92571d6a2db9ee116c267c2963f7b091ba/content/renderer/media/stream/media_stream_video_renderer_sink.cc [modify] https://crrev.com/d1f07e92571d6a2db9ee116c267c2963f7b091ba/content/test/data/media/getusermedia.html
,
Feb 20 2018
,
Feb 20 2018
This bug requires manual review: We are only 13 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 20 2018
Before we approve merge to M65, could you pls confirm followings? Is the revert listed at #7 well baked/verified in Canary, having enough automation tests coverage and safe to merge?
,
Feb 20 2018
I have verified it locally with ToT, not yet with a Canary. I plan to let it bake in Canary for 24 hours and merge after verifying with it (the current canary does not yet have r537675). There is no specific automatic test coverage for the regression that was introduced with r523152. We plan to add such a regression test when we reland a modified version of it. The revert is safe and easy to merge (just one line of code), but it will require a minor manual intervention because the file was moved from its original directory (content/renderer/media in M65 and content/renderer/media/stream in M66).
,
Feb 20 2018
Just to add to this: The revert adds back a single function call that is already there in 64 and used to be there in 65. Missing this function call is what caused the regression in 65.
,
Feb 20 2018
Thank you guidou@ and tommi@. Approving merge to M65 branch based on comments #11 and #12. Pls merge ASAP so we can pick it up for this week beta release. Also pls verify this bug on next canary too. Thank you.
,
Feb 21 2018
Pls verify the bug on tonight's canary and then merge to M65.
,
Feb 21 2018
The NextAction date has arrived: 2018-02-21
,
Feb 21 2018
Still waiting for a new Canary. The latest is still 66.0.3350.0 from 2018-02-16, which does not contain the fix.
,
Feb 21 2018
Yeah, last night canary also failed. Pls verify with tomorrow's canary.
,
Feb 22 2018
The NextAction date has arrived: 2018-02-22
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e62f75e6cdfbd502181616c82032bdcd7eaeb38e commit e62f75e6cdfbd502181616c82032bdcd7eaeb38e Author: Guido Urdaneta <guidou@chromium.org> Date: Thu Feb 22 13:04:14 2018 Revert "Don't trigger end of track when starting a new disabled track" This reverts commit 447c696aaf19064a79d7bc894ce4a0aa3422c69c. Reason for revert: Introduces a regression. See https://crbug.com/813243 Original change's description: > Don't trigger end of track when starting a new disabled track > > Fix reporting a size of 2x2 on the video element during the loading > phase. > > Bug: 684288 > Change-Id: Id4adceefaf4c3df57a1ba41fd285862d72792b1a > Reviewed-on: https://chromium-review.googlesource.com/806215 > Commit-Queue: Florent Castelli <orphis@chromium.org> > Commit-Queue: Guido Urdaneta <guidou@chromium.org> > Reviewed-by: Guido Urdaneta <guidou@chromium.org> > Cr-Commit-Position: refs/heads/master@{#523152} TBR=guidou@chromium.org, orphis@chromium.org (cherry picked from commit d1f07e92571d6a2db9ee116c267c2963f7b091ba) Bug: 684288, 813243 Change-Id: I60bc092c23d1840a9c7e95b4b1d8d02c52c6fb46 Reviewed-on: https://chromium-review.googlesource.com/924190 Commit-Queue: Guido Urdaneta <guidou@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#537675} Reviewed-on: https://chromium-review.googlesource.com/928768 Cr-Commit-Position: refs/branch-heads/3325@{#547} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/e62f75e6cdfbd502181616c82032bdcd7eaeb38e/content/browser/webrtc/webrtc_getusermedia_browsertest.cc [modify] https://crrev.com/e62f75e6cdfbd502181616c82032bdcd7eaeb38e/content/renderer/media/media_stream_video_renderer_sink.cc [modify] https://crrev.com/e62f75e6cdfbd502181616c82032bdcd7eaeb38e/content/test/data/media/getusermedia.html
,
Feb 22 2018
Merged after verifying on Canary 66.0.3352.0. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by guidou@chromium.org
, Feb 17 2018Labels: -Pri-2 Pri-1
Owner: guidou@chromium.org
Status: Assigned (was: Unconfirmed)