New issue
Advanced search Search tips

Issue 650735 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Implicit audio configuration changes don't work in MediaCodecAudioDecoder.

Project Member Reported by dalecur...@chromium.org, Sep 27 2016

Issue description

MCAD explicitly rejects these even though we now support such behavior in AudioRendererImpl for MSE based playbacks. Don't do this, instead set the correct parameters on the AudioBuffer and send to renderer.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 28 2016

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

commit 5769876ade1eadde031ff44094a30f31196e7cf1
Author: dalecurtis <dalecurtis@chromium.org>
Date: Tue Sep 27 20:22:39 2016

Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG= 650735 
TEST=new unittest

Review-Url: https://codereview.chromium.org/2376513002
Cr-Commit-Position: refs/heads/master@{#421318}

[modify] https://crrev.com/5769876ade1eadde031ff44094a30f31196e7cf1/media/filters/android/media_codec_audio_decoder.cc
[modify] https://crrev.com/5769876ade1eadde031ff44094a30f31196e7cf1/media/filters/android/media_codec_audio_decoder.h
[modify] https://crrev.com/5769876ade1eadde031ff44094a30f31196e7cf1/media/filters/audio_decoder_unittest.cc

Labels: Merge-Request-54

Comment 3 by dimu@chromium.org, Sep 28 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 28 2016

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

commit ae5d3869802f69c43a1c03790c9292453ea5e9fd
Author: timav <timav@chromium.org>
Date: Wed Sep 28 20:03:33 2016

Fix initialization in MediaCodecAudioDecoder

Move the call to ResetTimestampState() that sets several
init parameters before we return from Initialize waiting
for the media crypto.

Should be part of https://codereview.chromium.org/2376513002/

BUG= 650735 

Review-Url: https://codereview.chromium.org/2378643003
Cr-Commit-Position: refs/heads/master@{#421619}

[modify] https://crrev.com/ae5d3869802f69c43a1c03790c9292453ea5e9fd/media/filters/android/media_codec_audio_decoder.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 28 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94f6690a12fd8398e621140a9c259dc67561b1f9

commit 94f6690a12fd8398e621140a9c259dc67561b1f9
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:35:52 2016

Merge M54: "Fix implicit configuration changes in MediaCodecAudioDecoder."

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG= 650735 
TEST=new unittest

Review-Url: https://codereview.chromium.org/2376513002
Cr-Commit-Position: refs/heads/master@{#421318}
(cherry picked from commit 5769876ade1eadde031ff44094a30f31196e7cf1)

Review URL: https://codereview.chromium.org/2381703002 .

Cr-Commit-Position: refs/branch-heads/2840@{#569}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/94f6690a12fd8398e621140a9c259dc67561b1f9/media/filters/android/media_codec_audio_decoder.cc
[modify] https://crrev.com/94f6690a12fd8398e621140a9c259dc67561b1f9/media/filters/android/media_codec_audio_decoder.h
[modify] https://crrev.com/94f6690a12fd8398e621140a9c259dc67561b1f9/media/filters/audio_decoder_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 28 2016

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

commit 7296c30e2911ac765de4c92830d8236052a4805f
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:40:41 2016

Merge M54: "Fix initialization in MediaCodecAudioDecoder"

Move the call to ResetTimestampState() that sets several
init parameters before we return from Initialize waiting
for the media crypto.

Should be part of https://codereview.chromium.org/2376513002/

BUG= 650735 

Review-Url: https://codereview.chromium.org/2378643003
Cr-Commit-Position: refs/heads/master@{#421619}
(cherry picked from commit ae5d3869802f69c43a1c03790c9292453ea5e9fd)

Review URL: https://codereview.chromium.org/2374243002 .

Cr-Commit-Position: refs/branch-heads/2840@{#570}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/7296c30e2911ac765de4c92830d8236052a4805f/media/filters/android/media_codec_audio_decoder.cc

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 29 2016

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

commit b7f5facc5a9b7c6d04206a413c88dbafade336c1
Author: dalecurtis <dalecurtis@chromium.org>
Date: Thu Sep 29 20:06:56 2016

Fix MSE ADTS parsing on Android.

Android needs the extra_data fields in order to playback ADTS content,
so fill them in only on Android.

BUG= 610848 ,  650735 
TEST=http://storage.googleapis.com/chcunningham-chrome-shared/534301/aac_test.html

Review-Url: https://codereview.chromium.org/2378443002
Cr-Commit-Position: refs/heads/master@{#421913}

[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/BUILD.gn
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/filters/audio_decoder_unittest.cc
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mp2t/es_parser_adts.cc
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mp2t/es_parser_adts.h
[delete] https://crrev.com/ecc7bb5f24660afd661d27a4430b057b2c453387/media/formats/mpeg/adts_header_parser.cc
[delete] https://crrev.com/ecc7bb5f24660afd661d27a4430b057b2c453387/media/formats/mpeg/adts_header_parser.h
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mpeg/adts_stream_parser.cc
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mpeg/adts_stream_parser.h
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mpeg/mpeg1_audio_stream_parser.cc
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mpeg/mpeg1_audio_stream_parser.h
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mpeg/mpeg_audio_stream_parser_base.cc
[modify] https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1/media/formats/mpeg/mpeg_audio_stream_parser_base.h

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 30 2016

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

commit c27e432072ef22112251bbe874c8a1060a0c7ca5
Author: dalecurtis <dalecurtis@chromium.org>
Date: Fri Sep 30 19:34:39 2016

Fix media_log() crash in fuzzer tests.

When called from this context, no MediaLog exists, so ensure we
don't try to use it.

BUG= 610848 ,  650735 ,  651739 
TEST=libfuzzer

Review-Url: https://codereview.chromium.org/2388443002
Cr-Commit-Position: refs/heads/master@{#422182}

[modify] https://crrev.com/c27e432072ef22112251bbe874c8a1060a0c7ca5/media/formats/mpeg/adts_stream_parser.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 27 2016

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

commit 94f6690a12fd8398e621140a9c259dc67561b1f9
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:35:52 2016

Merge M54: "Fix implicit configuration changes in MediaCodecAudioDecoder."

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG= 650735 
TEST=new unittest

Review-Url: https://codereview.chromium.org/2376513002
Cr-Commit-Position: refs/heads/master@{#421318}
(cherry picked from commit 5769876ade1eadde031ff44094a30f31196e7cf1)

Review URL: https://codereview.chromium.org/2381703002 .

Cr-Commit-Position: refs/branch-heads/2840@{#569}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/94f6690a12fd8398e621140a9c259dc67561b1f9/media/filters/android/media_codec_audio_decoder.cc
[modify] https://crrev.com/94f6690a12fd8398e621140a9c259dc67561b1f9/media/filters/android/media_codec_audio_decoder.h
[modify] https://crrev.com/94f6690a12fd8398e621140a9c259dc67561b1f9/media/filters/audio_decoder_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 27 2016

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

commit 7296c30e2911ac765de4c92830d8236052a4805f
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:40:41 2016

Merge M54: "Fix initialization in MediaCodecAudioDecoder"

Move the call to ResetTimestampState() that sets several
init parameters before we return from Initialize waiting
for the media crypto.

Should be part of https://codereview.chromium.org/2376513002/

BUG= 650735 

Review-Url: https://codereview.chromium.org/2378643003
Cr-Commit-Position: refs/heads/master@{#421619}
(cherry picked from commit ae5d3869802f69c43a1c03790c9292453ea5e9fd)

Review URL: https://codereview.chromium.org/2374243002 .

Cr-Commit-Position: refs/branch-heads/2840@{#570}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/7296c30e2911ac765de4c92830d8236052a4805f/media/filters/android/media_codec_audio_decoder.cc

Sign in to add a comment