Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1932093002:
Reland: Use actual audio channel count in Spitzer audio decoder (Closed)
|
Created:
4 years, 7 months ago by Tima Vaisburd Modified:
4 years, 7 months ago CC:
chromium-reviews, feature-media-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@bug607024 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReland: Use actual audio channel count in Spitzer audio decoder
The actual channel count used by MediaCodec for audio may be
less than requested in case of multichannel AAC.
MediaCodecAudioDecoder obtains the actual channel count
in response to OUTPUT_FORMAT_CHANGED and uses it for the
calculation of the sample length.
In this CL we keep creating output buffers according to the
requested configuration. If the actual number of channels is
less than the requsted this CL fills the missing channels
with zero data.
The reland contains the fix for improper use of the |size| and
|offset| obtained from DequeueOutputBuffer. The size of the
useful decoded data chunk is |size| and does not depend on |offset|.
BUG=607024
> Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc
> Cr-Commit-Position: refs/heads/master@{#391416}
Committed: https://crrev.com/bbcbcba694946898e54f0c8709f4226477e487e0
Cr-Commit-Position: refs/heads/master@{#391618}
Patch Set 1 #
Total comments: 12
Patch Set 2 : Avoided second copy by exposing MediaCodecBridge::GetOutputBufferAddress() #
Total comments: 4
Patch Set 3 : Rebased, addressed comments #Patch Set 4 : Removed stale method in NdkMediaCodecBridge #Patch Set 5 : Rebase only #Patch Set 6 : Fix the use of out.offset: decoded data size does not depend on it. #
Messages
Total messages: 53 (22 generated)
timav@chromium.org changed reviewers: + dalecurtis@chromium.org, qinmin@chromium.org
Dale, in this CL I tried to accommodate the unexpected number of channels within decoder, i.e. without changing the pipeline. Please advice. The 5.1 file plays, but the sound became depleted without low frequencies. An extra copy could be avoided and the code could be cleaned up a little. qinmin@: fyi
https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:612: sample_format, config_.channel_layout(), config_channel_count, It's okay to specify the real channel count here. AudioRendererImpl will handle resampling and remixing if possible and throw a decode error otherwise. You don't need to worry about that here.
https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:612: sample_format, config_.channel_layout(), config_channel_count, On 2016/04/29 18:00:05, DaleCurtis wrote: > It's okay to specify the real channel count here. AudioRendererImpl will handle > resampling and remixing if possible and throw a decode error otherwise. You > don't need to worry about that here. If I do this I get "PIPELINE_ERROR pipeline: decode error" from https://code.google.com/p/chromium/codesearch#chromium/src/media/renderers/au... exactly like you described in the bug.
dalecurtis@chromium.org changed reviewers: + chcunningham@chromium.org
https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:612: sample_format, config_.channel_layout(), config_channel_count, On 2016/04/29 at 18:52:25, Tima Vaisburd wrote: > On 2016/04/29 18:00:05, DaleCurtis wrote: > > It's okay to specify the real channel count here. AudioRendererImpl will handle > > resampling and remixing if possible and throw a decode error otherwise. You > > don't need to worry about that here. > > If I do this I get "PIPELINE_ERROR pipeline: decode error" from > https://code.google.com/p/chromium/codesearch#chromium/src/media/renderers/au... > exactly like you described in the bug. Ah, you're saying this behavior happens under normal circumstances and we don't want a decode error in this case. That's a little trickier. Probably the most efficient thing to do is always enable "expecting config changes in AudioRendererImpl" then. If you just use a ChannelMixer here we'll still have created the audio output device at 5.1 which leads to some overhead before we eventually would downmix to stereo (if the OS does not support 5.1 output, which at least <=K does not). +chcunningham for thoughts.
https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:612: sample_format, config_.channel_layout(), config_channel_count, On 2016/04/29 18:58:55, DaleCurtis wrote: > On 2016/04/29 at 18:52:25, Tima Vaisburd wrote: > > On 2016/04/29 18:00:05, DaleCurtis wrote: > > > It's okay to specify the real channel count here. AudioRendererImpl will > handle > > > resampling and remixing if possible and throw a decode error otherwise. You > > > don't need to worry about that here. > > > > If I do this I get "PIPELINE_ERROR pipeline: decode error" from > > > https://code.google.com/p/chromium/codesearch#chromium/src/media/renderers/au... > > exactly like you described in the bug. > > Ah, you're saying this behavior happens under normal circumstances and we don't > want a decode error in this case. That's a little trickier. Probably the most > efficient thing to do is always enable "expecting config changes in > AudioRendererImpl" then. If you just use a ChannelMixer here we'll still have > created the audio output device at 5.1 which leads to some overhead before we > eventually would downmix to stereo (if the OS does not support 5.1 output, which > at least <=K does not). +chcunningham for thoughts. Please notice that this MediaCodec AAC behavior (i.e. downmixing in MediaCodec) depends on the system property "media.aac_51_output_enabled" being false or non-existent and thus not directly related to the abilities of the output device. For instance, on Nexus 5 + Android K the MediaCodec normally creates 6 output channels from aac 5.1, whereas the output device with 6 channel cannot be created.
https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:612: sample_format, config_.channel_layout(), config_channel_count, On 2016/04/29 at 19:19:05, Tima Vaisburd wrote: > On 2016/04/29 18:58:55, DaleCurtis wrote: > > On 2016/04/29 at 18:52:25, Tima Vaisburd wrote: > > > On 2016/04/29 18:00:05, DaleCurtis wrote: > > > > It's okay to specify the real channel count here. AudioRendererImpl will > > handle > > > > resampling and remixing if possible and throw a decode error otherwise. You > > > > don't need to worry about that here. > > > > > > If I do this I get "PIPELINE_ERROR pipeline: decode error" from > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/media/renderers/au... > > > exactly like you described in the bug. > > > > Ah, you're saying this behavior happens under normal circumstances and we don't > > want a decode error in this case. That's a little trickier. Probably the most > > efficient thing to do is always enable "expecting config changes in > > AudioRendererImpl" then. If you just use a ChannelMixer here we'll still have > > created the audio output device at 5.1 which leads to some overhead before we > > eventually would downmix to stereo (if the OS does not support 5.1 output, which > > at least <=K does not). +chcunningham for thoughts. > > Please notice that this MediaCodec AAC behavior (i.e. downmixing in MediaCodec) depends on the system property "media.aac_51_output_enabled" being false or non-existent and thus not directly related to the abilities of the output device. > > For instance, on Nexus 5 + Android K the MediaCodec normally creates 6 output channels from aac 5.1, whereas the output device with 6 channel cannot be created. Are we able to check that property and clamp the initialized configuration value instead?
On 2016/04/29 19:22:28, DaleCurtis wrote: > > Please notice that this MediaCodec AAC behavior (i.e. downmixing in > MediaCodec) depends on the system property "media.aac_51_output_enabled" being > false or non-existent and thus not directly related to the abilities of the > output device. > > Are we able to check that property and clamp the initialized configuration value > instead? There is public API System.getProperty() so I think technically we can do that. This is how a particular version of SoftAAC behaves though, that behavior is not guaranteed on all phones and codec implementations. They also say they normally downmix 7.1 -> 5.1. If we can reconfigure the pipeline based on actual count, would it be preferable?
On 2016/04/29 at 19:38:52, timav wrote: > On 2016/04/29 19:22:28, DaleCurtis wrote: > > > Please notice that this MediaCodec AAC behavior (i.e. downmixing in > > MediaCodec) depends on the system property "media.aac_51_output_enabled" being > > false or non-existent and thus not directly related to the abilities of the > > output device. > > > > Are we able to check that property and clamp the initialized configuration value > > instead? > > There is public API System.getProperty() so I think technically we can do that. > This is how a particular version of SoftAAC behaves though, that behavior is not > guaranteed on all phones and codec implementations. > > They also say they normally downmix 7.1 -> 5.1. > > If we can reconfigure the pipeline based on actual count, would it be preferable? Allowing pipeline reconfiguration is a bit inefficient since we lose some optimizations for multi-stream resampling. Instead we must remix and mux every single stream to the hardware parameters. We allow it for MSE because we have to, but haven't enabled it for src= in the past.
After discussion this seems like a better solution. https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:634: MediaCodecStatus status = media_codec_->CopyFromOutputBuffer( Why not expose GetOutputBufferAddress() directly and avoid the extra copy? You can then just copy directly into the deinterleaved buffer? https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:644: for (int ch = 0; ch < channel_count_; ++ch) { Do the tiny loop on the outside and the large loop on the inside for better efficiency. https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:693: DVLOG(0) << "GetOutputChannelCount failed."; I see you use this pattern elsewhere, but a better one is DLOG(ERROR)
Chatted with Dale - good with the plan for do it this way (for sake of simplicity).
qinmin@: Min, please review as well since I modified MediaCodecBridge and made GetOutputBufferAddress public. https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:634: MediaCodecStatus status = media_codec_->CopyFromOutputBuffer( On 2016/05/02 21:01:01, DaleCurtis wrote: > Why not expose GetOutputBufferAddress() directly and avoid the extra copy? You > can then just copy directly into the deinterleaved buffer? Done. https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:644: for (int ch = 0; ch < channel_count_; ++ch) { On 2016/05/02 21:01:01, DaleCurtis wrote: > Do the tiny loop on the outside and the large loop on the inside for better > efficiency. Done. https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:693: DVLOG(0) << "GetOutputChannelCount failed."; On 2016/05/02 21:01:01, DaleCurtis wrote: > I see you use this pattern elsewhere, but a better one is DLOG(ERROR) Done.
lgtm https://codereview.chromium.org/1932093002/diff/20001/media/filters/android/m... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/20001/media/filters/android/m... media/filters/android/media_codec_audio_decoder.cc:68: // The planes are populated in the other of channels in the interleaved frame. Comment needs cleanup, "other of channels" doesn't make sense.
lgtm
lgtm % comment https://codereview.chromium.org/1932093002/diff/20001/media/filters/android/m... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/20001/media/filters/android/m... media/filters/android/media_codec_audio_decoder.cc:82: int16_t* src_sample = (int16_t*)src_frame + ch; static_cast here and below
https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/1/media/filters/android/media... media/filters/android/media_codec_audio_decoder.cc:644: for (int ch = 0; ch < channel_count_; ++ch) { On 2016/05/02 23:55:35, Tima Vaisburd wrote: > On 2016/05/02 21:01:01, DaleCurtis wrote: > > Do the tiny loop on the outside and the large loop on the inside for better > > efficiency. > > Done. My change of the order of loops contained a bug and I reverted to the original code. The proper change should have been this: for (size_t ch = 0; ch < channel_count; ++ch) { const uint8_t* src_begin = interleaved_data + ch * kBytesPerOutputSample; for (size_t i = 0; i < frame_count; ++i, src_begin += bytes_per_frame) { const int16_t* src_sample = reinterpret_cast<const int16_t*>(src_begin); int16_t* dst_sample = reinterpret_cast<int16_t*>(planes[ch]) + i; *dst_sample = *src_sample; } } Here the loop for each channel starts near beginning (+ offset) and goes with |bytes per frame| step till the end. Then |src_begin| comes back to the beginning for the next channel. We can change it in the next CL if you feel strongly. https://codereview.chromium.org/1932093002/diff/20001/media/filters/android/m... File media/filters/android/media_codec_audio_decoder.cc (right): https://codereview.chromium.org/1932093002/diff/20001/media/filters/android/m... media/filters/android/media_codec_audio_decoder.cc:68: // The planes are populated in the other of channels in the interleaved frame. On 2016/05/03 00:11:51, DaleCurtis wrote: > Comment needs cleanup, "other of channels" doesn't make sense. s/other/order/ https://codereview.chromium.org/1932093002/diff/20001/media/filters/android/m... media/filters/android/media_codec_audio_decoder.cc:82: int16_t* src_sample = (int16_t*)src_frame + ch; On 2016/05/03 17:16:12, qinmin wrote: > static_cast here and below I think you meant reinterpret_cast<>, done.
The CQ bit was checked by timav@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chcunningham@chromium.org, qinmin@chromium.org, dalecurtis@chromium.org Link to the patchset: https://codereview.chromium.org/1932093002/#ps40001 (title: "Rebased, addressed comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932093002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932093002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by timav@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chcunningham@chromium.org, qinmin@chromium.org, dalecurtis@chromium.org Link to the patchset: https://codereview.chromium.org/1932093002/#ps60001 (title: "Removed stale method in NdkMediaCodecBridge")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932093002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932093002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
The CQ bit was checked by timav@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932093002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932093002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
The CQ bit was checked by timav@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932093002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932093002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply the patch.
The CQ bit was unchecked by commit-bot@chromium.org
The CQ bit was checked by timav@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chcunningham@chromium.org, qinmin@chromium.org, dalecurtis@chromium.org Link to the patchset: https://codereview.chromium.org/1932093002/#ps80001 (title: "Rebase only")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932093002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932093002/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. BUG=607024 ========== to ========== Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. BUG=607024 Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc Cr-Commit-Position: refs/heads/master@{#391416} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc Cr-Commit-Position: refs/heads/master@{#391416}
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/1945273002/ by mlamouri@chromium.org. The reason for reverting is: This is breaking media_unittests. For some reasons, it seem to only apply on internal bots. See: https://uberchromegw.corp.google.com/i/chromium.android/builders/Lollipop%20P... Log extract: I 46.291s Main FINISHED TRY #3/3 I 46.291s Main 1 failed tests remain. C 46.297s Main ******************************************************************************** C 46.297s Main Detailed Logs C 46.297s Main ******************************************************************************** C 46.299s Main [CRASH] MediaCodecAudioDecoderTest/AudioDecoderTest.ProduceAudioSamples/0: C 46.299s Main [ RUN ] MediaCodecAudioDecoderTest/AudioDecoderTest.ProduceAudioSamples/0 C 46.299s Main [ERROR:aligned_memory.cc(38)] If you crashed here, your aligned allocation is incorrect: size=4294966720, alignment=32 C 46.299s Main [FATAL:aligned_memory.cc(40)] Check failed: false. C 46.299s Main #00 0xa31c9979 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00c17979 C 46.299s Main #01 0xa31ca9e7 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00c189e7 C 46.299s Main #02 0xa2bc4779 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00612779 C 46.299s Main #03 0xa2bc49b5 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x006129b5 C 46.299s Main #04 0xa2830a7d /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x0027ea7d C 46.299s Main #05 0xa2830ee5 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x0027eee5 C 46.299s Main #06 0xa2831031 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x0027f031 C 46.299s Main #07 0xa2831385 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x0027f385 C 46.299s Main #08 0xa279984b /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x001e784b C 46.299s Main #09 0xa279a1b3 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x001e81b3 C 46.299s Main #10 0xa279d063 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x001eb063 C 46.299s Main #11 0xa31aa80f /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00bf880f C 46.299s Main #12 0xa31aa941 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00bf8941 C 46.299s Main #13 0xa31aa9c9 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00bf89c9 C 46.300s Main #14 0xa31ad2d5 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00bfb2d5 C 46.300s Main #15 0xa31a6e17 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00bf4e17 C 46.300s Main #16 0xa28533b9 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x002a13b9 C 46.300s Main #17 0xa285e43b /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x002ac43b C 46.300s Main #18 0xa26bc287 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x0010a287 C 46.300s Main #19 0xa319a8d5 /data/app/org.chromium.native_test-1/lib/arm/lib_media_unittests__library.so+0x00be88d5 C 46.300s Main #20 0xaf35ea9f /data/dalvik-cache/arm/data@app@org.chromium.native_test-1@base.apk@classes.d... C 46.300s Main C 46.300s Main [ERROR:test_suite.cc(254)] Currently running: MediaCodecAudioDecoderTest/AudioDecoderTest.ProduceAudioSamples/0 C 46.300s Main ******************************************************************************** C 46.300s Main Summary C 46.300s Main ******************************************************************************** C 46.304s Main [==========] 2837 tests ran. C 46.304s Main [ PASSED ] 2836 tests. C 46.304s Main [ FAILED ] 1 test, listed below: C 46.304s Main [ FAILED ] MediaCodecAudioDecoderTest/AudioDecoderTest.ProduceAudioSamples/0 (CRASHED) .
Message was sent while issue was closed.
mlamouri@chromium.org changed reviewers: + mlamouri@chromium.org
Message was sent while issue was closed.
My bad, the bots are not internal: https://build.chromium.org/p/chromium.android/builders/Lollipop%20Phone%20Tester https://build.chromium.org/p/chromium.android/builders/Lollipop%20Tablet%20Te...
Message was sent while issue was closed.
Description was changed from ========== Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. BUG=607024 Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc Cr-Commit-Position: refs/heads/master@{#391416} ========== to ========== Reland: Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. The reland contains the fix for improper use of the |size| and |offset| obtained from DequeueOutputBuffer. The size of the useful decoded data chunk is |size| and does not depend on |offset|. BUG=607024 > Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc > Cr-Commit-Position: refs/heads/master@{#391416} ==========
The CQ bit was checked by timav@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chcunningham@chromium.org, qinmin@chromium.org, dalecurtis@chromium.org Link to the patchset: https://codereview.chromium.org/1932093002/#ps100001 (title: "Fix the use of out.offset: decoded data size does not depend on it.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932093002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932093002/100001
Message was sent while issue was closed.
Description was changed from ========== Reland: Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. The reland contains the fix for improper use of the |size| and |offset| obtained from DequeueOutputBuffer. The size of the useful decoded data chunk is |size| and does not depend on |offset|. BUG=607024 > Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc > Cr-Commit-Position: refs/heads/master@{#391416} ========== to ========== Reland: Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. The reland contains the fix for improper use of the |size| and |offset| obtained from DequeueOutputBuffer. The size of the useful decoded data chunk is |size| and does not depend on |offset|. BUG=607024 > Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc > Cr-Commit-Position: refs/heads/master@{#391416} ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Reland: Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. The reland contains the fix for improper use of the |size| and |offset| obtained from DequeueOutputBuffer. The size of the useful decoded data chunk is |size| and does not depend on |offset|. BUG=607024 > Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc > Cr-Commit-Position: refs/heads/master@{#391416} ========== to ========== Reland: Use actual audio channel count in Spitzer audio decoder The actual channel count used by MediaCodec for audio may be less than requested in case of multichannel AAC. MediaCodecAudioDecoder obtains the actual channel count in response to OUTPUT_FORMAT_CHANGED and uses it for the calculation of the sample length. In this CL we keep creating output buffers according to the requested configuration. If the actual number of channels is less than the requsted this CL fills the missing channels with zero data. The reland contains the fix for improper use of the |size| and |offset| obtained from DequeueOutputBuffer. The size of the useful decoded data chunk is |size| and does not depend on |offset|. BUG=607024 > Committed: https://crrev.com/f6d2c9a3aa0a72ff766a132ef7f245ea26f66dfc > Cr-Commit-Position: refs/heads/master@{#391416} Committed: https://crrev.com/bbcbcba694946898e54f0c8709f4226477e487e0 Cr-Commit-Position: refs/heads/master@{#391618} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/bbcbcba694946898e54f0c8709f4226477e487e0 Cr-Commit-Position: refs/heads/master@{#391618} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
