New issue
Advanced search Search tips

Issue 809980 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

MediaRecorder.isTypeSupported lies in Chrome/Android for video/webm; codecs=h264

Reported by soe...@zfaas.com, Feb 7 2018

Issue description

Steps to reproduce the problem:
This problem occurs in Chrome on Android, using a Nexus 5 phone running Android version 6. I am not sure if all devices running Android are affected. 

1. Check that MediaRecorder.isTypeSupported('video/webm; codecs=h264') returns true
2. Instantiate a MediaRecorder instance, setting to "options" parameter to {mimeType: 'video/webm; codecs=h264'}
3. Fetch a MediaStream (eg., from getUserMedia) and use the MediaRecorder instance to produce a recording 
4. Save the resulting Blob object and use a suitable tool (eg., ffprobe) to analyze the file. It *should* contain a H.264 (AVC) video stream when it contains a VP8 video stream instead.

This is obviously wrong (in light of "isTypeSupported" returning true) and behaves correctly in desktop versions of Chrome (checked Linux, macOS, and Windows).

What is the expected behavior?
Same behavior as on desktop computers: MediaRecorder should produce a file that uses the WebM container format, containing an H.264 stream and an audio stream (no expectation re audio codec in this case). At the very least, MediaRecorder.isTypeSupported(...) should return false when the mimeType implies that an H.264 video stream is requested.

What went wrong?
The video codec directive was ignored and a VP8 video stream was produced. Worse, MediaRecorder.isTypeSupported(...) is lying (see above).

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 64  Channel: stable
OS Version: 6
Flash Version: 

This works in the desktop version of Chrome.
 
Labels: Needs-triage-Mobile
Cc: pnangunoori@chromium.org
Labels: Triaged-Mobile Needs-Feedback
soeren@ -- Thanks for reporting this issue. Could you please provide the sample URL or HTML file where the issue can be reproduced from TE's end along with the screencast for better understanding.

This would help us in reproducing and triaging the issue further.

Thanks in advance!
Components: -Blink>MediaStream Blink>MediaRecording
Sounds (to the uneducated me) like it ignores it in the Android code path, rather than lies. ;)
Cc: emir...@chromium.org
Status: Available (was: Unconfirmed)
soeren@ it's true, isTypeSupported lies in this case, since it doesn't
support AVC1 on Android [1] (because there's no Sw fallback). Someone
forgot to tell MediaRecorderHandler, hence the mistake/lie [2]. If you
want to upload a CL I can review it.


[1] https://github.com/yellowdoge/mediacapture-record-implementation-status/blob/master/chromium.md
[2] https://cs.chromium.org/chromium/src/content/renderer/media_recorder/media_recorder_handler.cc?q=mediarecorder+cc&sq=package:chromium&dr=CSs&l=163
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/919d2dee2bb2c4e6f9cbffdf94265378c312ffb0

commit 919d2dee2bb2c4e6f9cbffdf94265378c312ffb0
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Jan 04 22:05:12 2019

MediaRecorder: only announce h264/avc1 support if RTC_USE_H264

Support for AVC1 encoding (a.k.a. H264) is dependent on RTC_USE_H264 [1],
but due to an omission, this was not tested when enumerating the video
codecs. This CL fixes that.

[1] https://cs.chromium.org/chromium/src/third_party/webrtc/webrtc.gni?type=cs&q=rtc_use_h264&sq=package:chromium&g=0&l=156

TBR=emircan@chromium.org

Bug:  809980 , 719023
Change-Id: I03a375b269b042cb8cf64680153b878f60ea723f
Reviewed-on: https://chromium-review.googlesource.com/c/1395887
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620078}
[modify] https://crrev.com/919d2dee2bb2c4e6f9cbffdf94265378c312ffb0/content/renderer/media_recorder/media_recorder_handler.cc
[modify] https://crrev.com/919d2dee2bb2c4e6f9cbffdf94265378c312ffb0/content/renderer/media_recorder/media_recorder_handler_unittest.cc
[modify] https://crrev.com/919d2dee2bb2c4e6f9cbffdf94265378c312ffb0/third_party/blink/web_tests/TestExpectations

Owner: mcasas@chromium.org
Status: Fixed (was: Available)
soeren@zfaas.com if you are still there, could you please verify
once #5 makes it to Canary?  This link should tell us when it lands:
https://storage.googleapis.com/chromium-find-releases-static/919.html#919d2dee2bb2c4e6f9cbffdf94265378c312ffb0
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/612831555b5130eccfa8c6e783a4744c2362dec2

commit 612831555b5130eccfa8c6e783a4744c2362dec2
Author: Matthew Wolenetz <wolenetz@chromium.org>
Date: Fri Jan 04 23:22:49 2019

Revert "MediaRecorder: only announce h264/avc1 support if RTC_USE_H264"

This reverts commit 919d2dee2bb2c4e6f9cbffdf94265378c312ffb0.

Reason for revert: Strongly suspected as culprit in regressed blink web_tests:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/WebKit%20Linux%20Trusty%20ASAN/19960:
media_capabilities/encodingInfo.html

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/WebKit%20Linux%20Trusty%20Leak/28348:
media_capabilities/encodingInfo.html
fast/mediarecorder/MediaRecorder-isTypeSupported.html

Original change's description:
> MediaRecorder: only announce h264/avc1 support if RTC_USE_H264
> 
> Support for AVC1 encoding (a.k.a. H264) is dependent on RTC_USE_H264 [1],
> but due to an omission, this was not tested when enumerating the video
> codecs. This CL fixes that.
> 
> [1] https://cs.chromium.org/chromium/src/third_party/webrtc/webrtc.gni?type=cs&q=rtc_use_h264&sq=package:chromium&g=0&l=156
> 
> TBR=emircan@chromium.org
> 
> Bug:  809980 , 719023
> Change-Id: I03a375b269b042cb8cf64680153b878f60ea723f
> Reviewed-on: https://chromium-review.googlesource.com/c/1395887
> Commit-Queue: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620078}

TBR=mcasas@chromium.org,mbarowsky@chromium.org

Change-Id: I59b3a89c5a2e6c41c589617a3ab4d3f66b53dfa2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  809980 , 719023
Reviewed-on: https://chromium-review.googlesource.com/c/1396716
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620100}
[modify] https://crrev.com/612831555b5130eccfa8c6e783a4744c2362dec2/content/renderer/media_recorder/media_recorder_handler.cc
[modify] https://crrev.com/612831555b5130eccfa8c6e783a4744c2362dec2/content/renderer/media_recorder/media_recorder_handler_unittest.cc
[modify] https://crrev.com/612831555b5130eccfa8c6e783a4744c2362dec2/third_party/blink/web_tests/TestExpectations

Status: Assigned (was: Fixed)
Reactivating per #7
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e79591b31f7374edbdd23c4b3c0b27301ba58475

commit e79591b31f7374edbdd23c4b3c0b27301ba58475
Author: Miguel Casas <mcasas@chromium.org>
Date: Mon Jan 07 20:33:38 2019

RELAND: MediaRecorder: only announce h264/avc1 support if RTC_USE_H264

The original CL got reverted because:
- It broke media_capabilities/encodingInfo.html
- It broke the MediaRecorder LayoutTests on non-Android bots
when RTC_USE_H264 is not defined (which is pretty pervasive, it turns
out). This CL then adds the necessary TestExceptions.


Original CL description ------------------------------------------------
Support for AVC1 encoding (a.k.a. H264) is dependent on RTC_USE_H264 [1],
but due to an omission, this was not tested when enumerating the video
codecs. This CL fixes that.

[1] https://cs.chromium.org/chromium/src/third_party/webrtc/webrtc.gni?type=cs&q=rtc_use_h264&sq=package:chromium&g=0&l=156

TBR=emircan@chromium.org

Bug:  809980 , 719023
Change-Id: I0a1778adb0f5657ca1f06054d62a1a64b9d40ad6
Reviewed-on: https://chromium-review.googlesource.com/c/1395887
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620078}
Reviewed-on: https://chromium-review.googlesource.com/c/1396452
Cr-Commit-Position: refs/heads/master@{#620453}
[modify] https://crrev.com/e79591b31f7374edbdd23c4b3c0b27301ba58475/content/renderer/media_recorder/media_recorder_handler.cc
[modify] https://crrev.com/e79591b31f7374edbdd23c4b3c0b27301ba58475/content/renderer/media_recorder/media_recorder_handler_unittest.cc
[modify] https://crrev.com/e79591b31f7374edbdd23c4b3c0b27301ba58475/third_party/blink/web_tests/TestExpectations
[add] https://crrev.com/e79591b31f7374edbdd23c4b3c0b27301ba58475/third_party/blink/web_tests/fast/mediarecorder/MediaRecorder-isTypeSupported-avc1.html
[modify] https://crrev.com/e79591b31f7374edbdd23c4b3c0b27301ba58475/third_party/blink/web_tests/fast/mediarecorder/MediaRecorder-isTypeSupported.html

Cc: mbarow...@chromium.org
Labels: M-73
Hopefully now #9 sticks :-P
Status: Fixed (was: Assigned)
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6cac8286d5c5b95d187e83f1aa74d2490e1f7ae2

commit 6cac8286d5c5b95d187e83f1aa74d2490e1f7ae2
Author: Matthew Wolenetz <wolenetz@chromium.org>
Date: Mon Jan 07 21:26:52 2019

Revert "RELAND: MediaRecorder: only announce h264/avc1 support if RTC_USE_H264"

This reverts commit e79591b31f7374edbdd23c4b3c0b27301ba58475.

Reason for revert: Unfortunately, the RELAND still regressed
mediacapabilities/encodingInfo.html on
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/WebKit%20Linux%20Trusty%20ASAN/20082

Original change's description:
> RELAND: MediaRecorder: only announce h264/avc1 support if RTC_USE_H264
> 
> The original CL got reverted because:
> - It broke media_capabilities/encodingInfo.html
> - It broke the MediaRecorder LayoutTests on non-Android bots
> when RTC_USE_H264 is not defined (which is pretty pervasive, it turns
> out). This CL then adds the necessary TestExceptions.
> 
> 
> Original CL description ------------------------------------------------
> Support for AVC1 encoding (a.k.a. H264) is dependent on RTC_USE_H264 [1],
> but due to an omission, this was not tested when enumerating the video
> codecs. This CL fixes that.
> 
> [1] https://cs.chromium.org/chromium/src/third_party/webrtc/webrtc.gni?type=cs&q=rtc_use_h264&sq=package:chromium&g=0&l=156
> 
> TBR=emircan@chromium.org
> 
> Bug:  809980 , 719023
> Change-Id: I0a1778adb0f5657ca1f06054d62a1a64b9d40ad6
> Reviewed-on: https://chromium-review.googlesource.com/c/1395887
> Commit-Queue: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#620078}
> Reviewed-on: https://chromium-review.googlesource.com/c/1396452
> Cr-Commit-Position: refs/heads/master@{#620453}

TBR=mcasas@chromium.org,emircan@chromium.org,mbarowsky@chromium.org

Change-Id: Ibbe8fe769d0e2da553fe0d68ffa9d3a0b766d8e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  809980 , 719023
Reviewed-on: https://chromium-review.googlesource.com/c/1399292
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620477}
[modify] https://crrev.com/6cac8286d5c5b95d187e83f1aa74d2490e1f7ae2/content/renderer/media_recorder/media_recorder_handler.cc
[modify] https://crrev.com/6cac8286d5c5b95d187e83f1aa74d2490e1f7ae2/content/renderer/media_recorder/media_recorder_handler_unittest.cc
[modify] https://crrev.com/6cac8286d5c5b95d187e83f1aa74d2490e1f7ae2/third_party/blink/web_tests/TestExpectations
[delete] https://crrev.com/50ac726b54030cf0ea8058ba0e9c4d98ea2af4a5/third_party/blink/web_tests/fast/mediarecorder/MediaRecorder-isTypeSupported-avc1.html
[modify] https://crrev.com/6cac8286d5c5b95d187e83f1aa74d2490e1f7ae2/third_party/blink/web_tests/fast/mediarecorder/MediaRecorder-isTypeSupported.html

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90022e36fddcafdb31ac5bd090215bbea22472c2

commit 90022e36fddcafdb31ac5bd090215bbea22472c2
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Jan 09 15:06:51 2019

RELAND2: MediaRecorder: only announce h264/avc1 support if RTC_USE_H264

The relanded CL got reverted as well (!) because it still broke
media_capabilities/encodingInfo.html; this CL splits the avc1/h264
parts in a file on its own, and rewrites all of those tests to not
use generate_tests (see comment in crrev.com/c/1393538). Only the
-avc1.html tests are marked as [Pass Failure] in TestExpectations.

Original 2nd CL description --------------------------------------------

The original CL got reverted because:
- It broke media_capabilities/encodingInfo.html
- It broke the MediaRecorder LayoutTests on non-Android bots
when RTC_USE_H264 is not defined (which is pretty pervasive, it turns
out). This CL then adds the necessary TestExceptions.

Original CL description ------------------------------------------------
Support for AVC1 encoding (a.k.a. H264) is dependent on RTC_USE_H264 [1],
but due to an omission, this was not tested when enumerating the video
codecs. This CL fixes that.

[1] https://cs.chromium.org/chromium/src/third_party/webrtc/webrtc.gni?type=cs&q=rtc_use_h264&sq=package:chromium&g=0&l=156

Bug:  809980 , 719023
Change-Id: I865e9007ffce810998f62d1e187bf4ecf499badd
Reviewed-on: https://chromium-review.googlesource.com/c/1395887
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#620078}
Reviewed-on: https://chromium-review.googlesource.com/c/1396452
Cr-Original-Commit-Position: refs/heads/master@{#620453}
Reviewed-on: https://chromium-review.googlesource.com/c/1399823
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621142}
[modify] https://crrev.com/90022e36fddcafdb31ac5bd090215bbea22472c2/content/renderer/media_recorder/media_recorder_handler.cc
[modify] https://crrev.com/90022e36fddcafdb31ac5bd090215bbea22472c2/content/renderer/media_recorder/media_recorder_handler_unittest.cc
[modify] https://crrev.com/90022e36fddcafdb31ac5bd090215bbea22472c2/third_party/blink/web_tests/TestExpectations
[add] https://crrev.com/90022e36fddcafdb31ac5bd090215bbea22472c2/third_party/blink/web_tests/fast/mediarecorder/MediaRecorder-isTypeSupported-avc1.html
[modify] https://crrev.com/90022e36fddcafdb31ac5bd090215bbea22472c2/third_party/blink/web_tests/fast/mediarecorder/MediaRecorder-isTypeSupported.html
[add] https://crrev.com/90022e36fddcafdb31ac5bd090215bbea22472c2/third_party/blink/web_tests/media_capabilities/encodingInfo-avc1.html
[modify] https://crrev.com/90022e36fddcafdb31ac5bd090215bbea22472c2/third_party/blink/web_tests/media_capabilities/encodingInfo.html

Sign in to add a comment