Issue metadata
Sign in to add a comment
|
Opus Phasing Feature Still Enabled When Downmixing To Mono
Reported by
tom.jenk...@soundcloud.com,
Jan 26 2018
|
||||||||||||||||||||||
Issue descriptionChrome Version : 64.0.3282.119 (Official Build) (64-bit) OS version : Mac OS High Sierra Audio/Video format : OggOpus Behavior in Safari: Safari can't play OggOpus Video issue, Audio issue, both, neither? Anything with OggOpus audio stream. Including WebM Flash or HTML5? HTML5 What steps will reproduce the problem? For the following please play the "Opus stereo music samples, various bitrates" example on http://opus-codec.org/examples/ (1) Make sure your output is set to mono. On mac this can be done from the accessibility settings page (2) Play from 15 seconds (uncompressed) (2) Play from 15 seconds at 64 kb/s What is the expected result? The uncompressed and opus version sound almost identical. What is the actual result? With the opus version there is flanging in the high frequencies Why does this happen? For some frequencies at various points in a stream the opus encoder switches to a form of intensity stereo encoding, where it stores one channel, and then either makes the other channel 180 degree out of phase or identical. When playing back a track in stereo, this works great, and makes the encoding sound closer to the original. However, if this stereo output is later downmixed to mono, the parts that have been encoded with intensity stereo, and inverted, get cancelled out of the output. Possible solutions 1) The opus decoder is informed how many channels to decode to. If this is set to 1, it outputs mono AND disables the phase inversion (since v1.3-beta), meaning the output won't have this problem. This should happen if the browser is able to detect that the final output is mono (which might not be feasible). 2) The opus decoder now has an official option to disable phase inversion (since v1.3-beta). If phase inversion is disabled this way, and the stereo output is later downmixed to mono, there is no problem. The downside is that if the user actually listens to the output in stereo, it would sound slightly worse than if the phase inversion had been enabled. 3) It is also possible to disable the phase inversion feature at encode time now too. If the decoder gets a file that has been encoded with inversion disabled, the output of the decoder would be identical if phase inversion is enabled there or not. I wouldn't really call this a fix though, because it is removing information from the encode, which would require a new encode to bring back later. Also someone might want to use an encode with phase inversion enabled on a device that they have complete control over, and know that users will be listening to in stereo. They might also want to play the same encode in a browser. It's not ideal for them to have to store 2 encodes of everything, one with the phase inversion disabled at encode time. I will get in touch with the opus devs and send them a link to this ticket. They will probably be able to provide more detail/context than I have. cc chcunningham@chromium.org cc dalecurtis@chromium.org cc jmvalin@jmvalin.ca cc media-streaming@soundcloud.com
,
Jan 26 2018
> (there's no device change signal to renderers) Was that signal in the roadmap (something Stockholm folks were working on) ? Re option 2) I'd like to hear how badly stereo is degraded with and without phase inversion. Do you have some content that highlights the difference?
,
Jan 26 2018
No, I worked on it long ago, but we abandoned the idea since it made more sense to let the browser side handle it seamlessly instead of going through a long notification chain with shmem teardown and ping-ponging browser-renderer IPC (close, create, play, etc) calls.
,
Jan 26 2018
For 2) you'll probably need to flip the flag in our opus decoder and take a listen.
,
Apr 22 2018
Thanks for providing the metrics it's great to have a sense of how many people this could be effecting. Any update on this?
,
Apr 24 2018
Sorry, I haven't had a chance yet. I'll take a look on Wednesday.
,
Apr 26 2018
Wrote some plumbing code to get the hw channel count to the decoder... but out of time today. CL out tomorrow.
,
Apr 26 2018
,
Apr 26 2018
Code out for review https://chromium-review.googlesource.com/c/chromium/src/+/1030907
,
Apr 29 2018
The NextAction date has arrived: 2018-04-29
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/90210b5e10d3917567a3025e4853704bfefd8384 commit 90210b5e10d3917567a3025e4853704bfefd8384 Author: chcunningham <chcunningham@chromium.org> Date: Tue May 01 18:08:21 2018 Add av_dict_count to ffmpeg.sigs Needed by opus phase inversion CL (ffmpeg_audio_decoder.cc) https://chromium-review.googlesource.com/c/chromium/src/+/1030907 Bug: 806219 Change-Id: I744fb3ef5189a267f0be144399cb4acad74ca7e5 TBR: dalecurtis@chromium.org Change-Id: I744fb3ef5189a267f0be144399cb4acad74ca7e5 Reviewed-on: https://chromium-review.googlesource.com/1036735 Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> [modify] https://crrev.com/90210b5e10d3917567a3025e4853704bfefd8384/chromium/ffmpeg.sigs
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5e1c7c43ae6f4e925d131944f9191d964661e098 commit 5e1c7c43ae6f4e925d131944f9191d964661e098 Author: chcunningham <chcunningham@chromium.org> Date: Tue May 01 19:57:10 2018 media: Disable opus decode phase inversion for mono output. Encoders may use "phase inversion" to improve stereo compression. But this creates artifacts if the decoded stereo is later down-mixed to mono. This CL sets a flag on the decoder to disable the phase inversion decode step whenever the HW is detected as mono. Bug: 806219 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: Ie5f911910dbcc61e48ea58da51c48c0c2c2ebdc3 Reviewed-on: https://chromium-review.googlesource.com/1030907 Commit-Queue: Chrome Cunningham <chcunningham@chromium.org> Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#555156} [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/DEPS [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/base/audio_decoder_config.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/base/audio_decoder_config.h [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/audio_decoder_selector_unittest.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/decoder_selector.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/decoder_stream.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/decoder_stream.h [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/decoder_stream_traits.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/decoder_stream_traits.h [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/ffmpeg_audio_decoder.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/filters/video_frame_stream_unittest.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/renderers/audio_renderer_impl.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/renderers/video_renderer_impl.cc [add] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/test/data/bunny-opus-intensity-stereo.webm [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/test/pipeline_integration_test.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/test/pipeline_integration_test_base.cc [modify] https://crrev.com/5e1c7c43ae6f4e925d131944f9191d964661e098/media/test/pipeline_integration_test_base.h
,
May 1 2018
Will ship in 68.
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b4d9b945ac4e215b45df6c82e85d0586578ceae commit 5b4d9b945ac4e215b45df6c82e85d0586578ceae Author: Chrome Cunningham <chcunningham@chromium.org> Date: Tue May 01 21:10:21 2018 Revert "media: Disable opus decode phase inversion for mono output." This reverts commit 5e1c7c43ae6f4e925d131944f9191d964661e098. Reason for revert: Broke cast-linux and linux-chromeos https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Cast%20Linux/51797 https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-rel/7449 Original change's description: > media: Disable opus decode phase inversion for mono output. > > Encoders may use "phase inversion" to improve stereo compression. But > this creates artifacts if the decoded stereo is later down-mixed to > mono. This CL sets a flag on the decoder to disable the phase inversion > decode step whenever the HW is detected as mono. > > Bug: 806219 > > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel > Change-Id: Ie5f911910dbcc61e48ea58da51c48c0c2c2ebdc3 > Reviewed-on: https://chromium-review.googlesource.com/1030907 > Commit-Queue: Chrome Cunningham <chcunningham@chromium.org> > Reviewed-by: Dale Curtis <dalecurtis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#555156} TBR=dalecurtis@chromium.org,chcunningham@chromium.org Change-Id: I4cd81fc8ab975b8016a002d2f97b9cdab6b5525a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 806219 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Reviewed-on: https://chromium-review.googlesource.com/1038443 Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Commit-Queue: Chrome Cunningham <chcunningham@chromium.org> Cr-Commit-Position: refs/heads/master@{#555182} [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/DEPS [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/base/audio_decoder_config.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/base/audio_decoder_config.h [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/audio_decoder_selector_unittest.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/decoder_selector.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/decoder_stream.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/decoder_stream.h [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/decoder_stream_traits.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/decoder_stream_traits.h [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/ffmpeg_audio_decoder.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/filters/video_frame_stream_unittest.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/renderers/audio_renderer_impl.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/renderers/video_renderer_impl.cc [delete] https://crrev.com/3a85529b33a9f6f8b920932884503ab60d1c9b49/media/test/data/bunny-opus-intensity-stereo.webm [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/test/pipeline_integration_test.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/test/pipeline_integration_test_base.cc [modify] https://crrev.com/5b4d9b945ac4e215b45df6c82e85d0586578ceae/media/test/pipeline_integration_test_base.h
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/26bcc610d2d00379598554c1128319c05edc6767 commit 26bcc610d2d00379598554c1128319c05edc6767 Author: chcunningham <chcunningham@chromium.org> Date: Wed May 02 00:43:52 2018 (RELAND) media: Disable opus decode phase inversion for mono output. Encoders may use "phase inversion" to improve stereo compression. But this creates artifacts if the decoded stereo is later down-mixed to mono. This CL sets a flag on the decoder to disable the phase inversion decode step whenever the HW is detected as mono. Bug:806219 TBR:dalecurtis@chromium.org Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: Icff0387c7fc0fc0e0e31657176721cee5abf6fd9 Reviewed-on: https://chromium-review.googlesource.com/1038617 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Commit-Queue: Chrome Cunningham <chcunningham@chromium.org> Cr-Commit-Position: refs/heads/master@{#555257} [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/DEPS [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/base/audio_decoder_config.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/base/audio_decoder_config.h [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/audio_decoder_selector_unittest.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/decoder_selector.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/decoder_stream.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/decoder_stream.h [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/decoder_stream_traits.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/decoder_stream_traits.h [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/ffmpeg_audio_decoder.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/filters/video_frame_stream_unittest.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/renderers/audio_renderer_impl.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/renderers/video_renderer_impl.cc [add] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/test/data/bunny-opus-intensity-stereo.webm [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/test/pipeline_integration_test.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/test/pipeline_integration_test_base.cc [modify] https://crrev.com/26bcc610d2d00379598554c1128319c05edc6767/media/test/pipeline_integration_test_base.h
,
May 2 2018
Forgot to mention: the repro from the initial report will still sound bad if you simply use mac's accessibility settings to force mono. This is because Chrome still see's the output sink as stereo in spite of the accessibility setting (downmix happens downstream of chrome). > Per metrics, ~8.5% of daily clients are using mono output. ^^^ These clients will get the fix! In this case, the sink reports as mono. Not sure how create that circumstance locally. For testing purposes I just hard coded the sink to be mono.
,
May 8 2018
Awesome! Thanks for fixing this. It's a shame it doesn't also apply when the user sets the output to mono in accessibility settings (which seems to be the only way to get mono), but I understand that there's nothing chrome can do about this. I wonder if it would also be possible to add a flag to the media element to override this behaviour? e.g <audio disablePhaseInversion src="something.opus" /> This way if someone wants the phase inversion disabled all the time no matter on the output channel count, they have the option. (Happy to create a separate ticket for this)
,
May 8 2018
> I wonder if it would also be possible to add a flag to the media element to override this behaviour? Its technically possible, but standardization would be very challenging. We don't have any precedent for codec specific attributes. Having said that, if you as the app developer know when you'd like to disable phase inversion, I wonder if you might instead remedy the situation using WebAudio? I know *very little* about that API, but its got a lot of filters - maybe one that could be useful (customized?) for this use case?
,
May 29 2018
I don't think this is possible to fix with web audio as the information needed to know when phase inversion is happening is lost after the encode. I.e is the current sample L+R or A+Inverted A |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dalecur...@chromium.org
, Jan 26 2018Status: Available (was: Unconfirmed)