Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(291)

Issue 2378443002: Fix MSE ADTS parsing on Android. (Closed)

Created:
4 years, 2 months ago by DaleCurtis
Modified:
4 years, 2 months ago
Reviewers:
chcunningham
CC:
chromium-reviews, feature-media-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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 Committed: https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1 Cr-Commit-Position: refs/heads/master@{#421913}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Address comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -169 lines) Patch
M media/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M media/filters/audio_decoder_unittest.cc View 1 2 chunks +11 lines, -4 lines 0 comments Download
M media/formats/mp2t/es_parser_adts.h View 1 3 chunks +4 lines, -1 line 0 comments Download
M media/formats/mp2t/es_parser_adts.cc View 1 4 chunks +25 lines, -10 lines 0 comments Download
D media/formats/mpeg/adts_header_parser.h View 1 1 chunk +0 lines, -29 lines 0 comments Download
D media/formats/mpeg/adts_header_parser.cc View 1 1 chunk +0 lines, -88 lines 0 comments Download
M media/formats/mpeg/adts_stream_parser.h View 1 2 chunks +3 lines, -2 lines 0 comments Download
M media/formats/mpeg/adts_stream_parser.cc View 1 3 chunks +22 lines, -3 lines 0 comments Download
M media/formats/mpeg/mpeg1_audio_stream_parser.h View 1 chunk +2 lines, -1 line 0 comments Download
M media/formats/mpeg/mpeg1_audio_stream_parser.cc View 1 chunk +9 lines, -7 lines 0 comments Download
M media/formats/mpeg/mpeg_audio_stream_parser_base.h View 1 chunk +2 lines, -1 line 0 comments Download
M media/formats/mpeg/mpeg_audio_stream_parser_base.cc View 5 chunks +14 lines, -21 lines 0 comments Download

Messages

Total messages: 16 (9 generated)
DaleCurtis
4 years, 2 months ago (2016-09-27 19:38:22 UTC) #2
chcunningham
Can you also remove the parser call in this test: https://cs.chromium.org/chromium/src/media/filters/audio_decoder_unittest.cc?rcl=0&l=208 and instead make it ...
4 years, 2 months ago (2016-09-28 18:50:00 UTC) #4
DaleCurtis
PTAL. https://codereview.chromium.org/2378443002/diff/1/media/formats/mpeg/adts_stream_parser.cc File media/formats/mpeg/adts_stream_parser.cc (right): https://codereview.chromium.org/2378443002/diff/1/media/formats/mpeg/adts_stream_parser.cc#newcode23 media/formats/mpeg/adts_stream_parser.cc:23: int ADTSStreamParser::ParseFrameHeader(const uint8_t* data, On 2016/09/28 at 18:50:00, ...
4 years, 2 months ago (2016-09-29 01:23:53 UTC) #6
chcunningham
lgtm
4 years, 2 months ago (2016-09-29 20:01:07 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2378443002/40001
4 years, 2 months ago (2016-09-29 20:02:17 UTC) #13
commit-bot: I haz the power
Committed patchset #2 (id:40001)
4 years, 2 months ago (2016-09-29 20:07:17 UTC) #14
commit-bot: I haz the power
4 years, 2 months ago (2016-09-29 20:10:44 UTC) #16
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/b7f5facc5a9b7c6d04206a413c88dbafade336c1
Cr-Commit-Position: refs/heads/master@{#421913}

Powered by Google App Engine
This is Rietveld 408576698