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

Side by Side Diff: media/formats/webm/webm_cluster_parser.cc

Issue 883403002: Parsing of encoded duration for unencrypted opus streams. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "media/formats/webm/webm_cluster_parser.h" 5 #include "media/formats/webm/webm_cluster_parser.h"
6 6
7 #include <vector> 7 #include <vector>
8 8
9 #include "base/logging.h" 9 #include "base/logging.h"
10 #include "base/sys_byteorder.h" 10 #include "base/sys_byteorder.h"
11 #include "media/base/buffers.h" 11 #include "media/base/buffers.h"
12 #include "media/base/decrypt_config.h" 12 #include "media/base/decrypt_config.h"
13 #include "media/filters/webvtt_util.h" 13 #include "media/filters/webvtt_util.h"
14 #include "media/formats/webm/webm_constants.h" 14 #include "media/formats/webm/webm_constants.h"
15 #include "media/formats/webm/webm_crypto_helpers.h" 15 #include "media/formats/webm/webm_crypto_helpers.h"
16 #include "media/formats/webm/webm_webvtt_parser.h" 16 #include "media/formats/webm/webm_webvtt_parser.h"
17 17
18 namespace media { 18 namespace media {
19 19
20 WebMClusterParser::WebMClusterParser( 20 WebMClusterParser::WebMClusterParser(
21 int64 timecode_scale, 21 int64 timecode_scale,
22 int audio_track_num, 22 int audio_track_num,
23 base::TimeDelta audio_default_duration, 23 base::TimeDelta audio_default_duration,
24 int video_track_num, 24 int video_track_num,
25 base::TimeDelta video_default_duration, 25 base::TimeDelta video_default_duration,
26 const WebMTracksParser::TextTracks& text_tracks, 26 const WebMTracksParser::TextTracks& text_tracks,
27 const std::set<int64>& ignored_tracks, 27 const std::set<int64>& ignored_tracks,
28 const std::string& audio_encryption_key_id, 28 const std::string& audio_encryption_key_id,
29 const std::string& video_encryption_key_id, 29 const std::string& video_encryption_key_id,
30 const AudioDecoderConfig& audio_config,
30 const LogCB& log_cb) 31 const LogCB& log_cb)
31 : timecode_multiplier_(timecode_scale / 1000.0), 32 : timecode_multiplier_(timecode_scale / 1000.0),
32 ignored_tracks_(ignored_tracks), 33 ignored_tracks_(ignored_tracks),
33 audio_encryption_key_id_(audio_encryption_key_id), 34 audio_encryption_key_id_(audio_encryption_key_id),
34 video_encryption_key_id_(video_encryption_key_id), 35 video_encryption_key_id_(video_encryption_key_id),
36 audio_config_(audio_config),
chcunningham 2015/01/29 22:14:20 I'm not doing anything with this in ::Reset. Shoul
wolenetz 2015/01/30 01:48:08 ::Reset() is only for "let's parse another cluster
chcunningham 2015/02/03 20:34:43 Acknowledged.
35 parser_(kWebMIdCluster, this), 37 parser_(kWebMIdCluster, this),
36 last_block_timecode_(-1), 38 last_block_timecode_(-1),
37 block_data_size_(-1), 39 block_data_size_(-1),
38 block_duration_(-1), 40 block_duration_(-1),
39 block_add_id_(-1), 41 block_add_id_(-1),
40 block_additional_data_size_(0), 42 block_additional_data_size_(0),
41 discard_padding_(-1), 43 discard_padding_(-1),
42 cluster_timecode_(-1), 44 cluster_timecode_(-1),
43 cluster_start_time_(kNoTimestamp()), 45 cluster_start_time_(kNoTimestamp()),
44 cluster_ended_(false), 46 cluster_ended_(false),
(...skipping 88 matching lines...) Expand 10 before | Expand all | Expand 10 after
133 itr != text_track_map_.end(); 135 itr != text_track_map_.end();
134 ++itr) { 136 ++itr) {
135 const BufferQueue& text_buffers = itr->second.ready_buffers(); 137 const BufferQueue& text_buffers = itr->second.ready_buffers();
136 if (!text_buffers.empty()) 138 if (!text_buffers.empty())
137 text_buffers_map_.insert(std::make_pair(itr->first, text_buffers)); 139 text_buffers_map_.insert(std::make_pair(itr->first, text_buffers));
138 } 140 }
139 141
140 return text_buffers_map_; 142 return text_buffers_map_;
141 } 143 }
142 144
145 bool WebMClusterParser::TryGetEncodedAudioDuration(const uint8* data,
wolenetz 2015/01/30 01:48:07 I suggest using a base::TimeDelta duration as retv
chcunningham 2015/02/03 20:34:42 Done, but a question about asserts. I haven't yet
wolenetz 2015/02/03 22:47:00 For now, yeah, let frame processor/etc deal with 0
146 int size,
147 int64* duration) {
148 if (audio_config_.codec() == kCodecOpus) {
149 return ReadOpusDuration(data, size, duration);
150 }
151
152 return false;
153 }
154
155 bool WebMClusterParser::ReadOpusDuration(const uint8* data,
wolenetz 2015/01/30 01:48:08 ditto on retval comment.
chcunningham 2015/02/03 20:34:42 Done.
156 int size,
157 int64* duration) {
158 if (size < 1) {
159 MEDIA_LOG(log_cb_) << "Invalid zero-byte Opus packet.";
wolenetz 2015/01/30 01:48:07 Could it be possible that an append one-byte-at-a-
wolenetz 2015/01/30 20:46:12 nm. OnBlock is done on complete block, not partial
chcunningham 2015/02/03 20:34:42 Acknowledged.
160 return false;
161 }
162
163 // Frame count type described by last 2 bits of Opus TOC byte.
164 int frame_count_type = data[0] & 0x03;
165
166 int frame_count = 0;
167 switch (frame_count_type) {
168 case 0:
169 frame_count = 1;
170 break;
171 case 1:
172 case 2:
173 frame_count = 2;
174 break;
175 case 3:
176 // Type 3 indicates an arbitrary frame count described in the next byte.
177 if (size < 2) {
178 MEDIA_LOG(log_cb_) << "Second byte missing from 'Code 3' Opus packet.";
wolenetz 2015/01/30 01:48:08 ditto: protect against spurious log?
wolenetz 2015/01/30 20:46:12 ditto nm
chcunningham 2015/02/03 20:34:42 Acknowledged.
179 return false;
180 }
181 frame_count = data[1] & 0x3f;
182 DCHECK_GE(frame_count, 1);
183 break;
184 default:
185 MEDIA_LOG(log_cb_) << "Unexpected Opus frame count type: "
186 << frame_count_type;
187 return false;
188 }
189
190 int opusConfig = (data[0] & 0xf8) >> 3;
wolenetz 2015/01/30 20:46:12 nit: put "magic" 0xf8 and others like that into an
chcunningham 2015/02/03 20:34:43 Done, sort of. Did it right here in the .cc - head
wolenetz 2015/02/03 22:47:00 I'm not sure how an enum consumes more mem that a
191 *duration = kOpusFrameDurationsMu[opusConfig] * frame_count;
wolenetz 2015/01/30 01:48:08 frame_count could be zero in a "code 3 packet". Gu
wolenetz 2015/01/30 20:46:12 The strange wording is clarified elsewhere in that
chcunningham 2015/02/03 20:34:42 Agree this is hard to read. I think I have an inte
wolenetz 2015/02/03 22:47:00 I agree. See my comment on current PS.
192
193 return true;
194 }
195
143 WebMParserClient* WebMClusterParser::OnListStart(int id) { 196 WebMParserClient* WebMClusterParser::OnListStart(int id) {
144 if (id == kWebMIdCluster) { 197 if (id == kWebMIdCluster) {
145 cluster_timecode_ = -1; 198 cluster_timecode_ = -1;
146 cluster_start_time_ = kNoTimestamp(); 199 cluster_start_time_ = kNoTimestamp();
147 } else if (id == kWebMIdBlockGroup) { 200 } else if (id == kWebMIdBlockGroup) {
148 block_data_.reset(); 201 block_data_.reset();
149 block_data_size_ = -1; 202 block_data_size_ = -1;
150 block_duration_ = -1; 203 block_duration_ = -1;
151 discard_padding_ = -1; 204 discard_padding_ = -1;
152 discard_padding_set_ = false; 205 discard_padding_set_ = false;
(...skipping 164 matching lines...) Expand 10 before | Expand all | Expand 10 after
317 370
318 if (last_block_timecode_ != -1 && timecode < last_block_timecode_) { 371 if (last_block_timecode_ != -1 && timecode < last_block_timecode_) {
319 MEDIA_LOG(log_cb_) 372 MEDIA_LOG(log_cb_)
320 << "Got a block with a timecode before the previous block."; 373 << "Got a block with a timecode before the previous block.";
321 return false; 374 return false;
322 } 375 }
323 376
324 Track* track = NULL; 377 Track* track = NULL;
325 StreamParserBuffer::Type buffer_type = DemuxerStream::AUDIO; 378 StreamParserBuffer::Type buffer_type = DemuxerStream::AUDIO;
326 std::string encryption_key_id; 379 std::string encryption_key_id;
380 int64 encoded_duration = -1;
wolenetz 2015/01/30 01:48:08 ditto: base::TimeDelta please
chcunningham 2015/02/03 20:34:42 Done.
327 if (track_num == audio_.track_num()) { 381 if (track_num == audio_.track_num()) {
328 track = &audio_; 382 track = &audio_;
329 encryption_key_id = audio_encryption_key_id_; 383 encryption_key_id = audio_encryption_key_id_;
384 if (encryption_key_id.empty()) {
385 TryGetEncodedAudioDuration(data, size, &encoded_duration);
wolenetz 2015/01/30 01:48:08 note the retval was ignored previously... not sure
chcunningham 2015/02/03 20:34:42 Done.
386 }
wolenetz 2015/01/30 01:48:08 nit: no {} necessary for one-line block here. I'm
wolenetz 2015/01/30 01:48:08 I'd like to see at least DVLOG for audio block if
chcunningham 2015/02/03 20:34:42 It became a mutli-line block after adding the DVLO
chcunningham 2015/02/03 20:34:42 Done. How does 10ms for a threshold sound?
wolenetz 2015/02/03 22:47:00 See my comment on current PS.
330 } else if (track_num == video_.track_num()) { 387 } else if (track_num == video_.track_num()) {
331 track = &video_; 388 track = &video_;
332 encryption_key_id = video_encryption_key_id_; 389 encryption_key_id = video_encryption_key_id_;
333 buffer_type = DemuxerStream::VIDEO; 390 buffer_type = DemuxerStream::VIDEO;
334 } else if (ignored_tracks_.find(track_num) != ignored_tracks_.end()) { 391 } else if (ignored_tracks_.find(track_num) != ignored_tracks_.end()) {
335 return true; 392 return true;
336 } else if (Track* const text_track = FindTextTrack(track_num)) { 393 } else if (Track* const text_track = FindTextTrack(track_num)) {
337 if (is_simple_block) // BlockGroup is required for WebVTT cues 394 if (is_simple_block) // BlockGroup is required for WebVTT cues
338 return false; 395 return false;
339 if (block_duration < 0) // not specified 396 if (block_duration < 0) // not specified
(...skipping 60 matching lines...) Expand 10 before | Expand all | Expand 10 after
400 content.length(), 457 content.length(),
401 &side_data[0], 458 &side_data[0],
402 side_data.size(), 459 side_data.size(),
403 true, buffer_type, track_num); 460 true, buffer_type, track_num);
404 } 461 }
405 462
406 buffer->set_timestamp(timestamp); 463 buffer->set_timestamp(timestamp);
407 if (cluster_start_time_ == kNoTimestamp()) 464 if (cluster_start_time_ == kNoTimestamp())
408 cluster_start_time_ = timestamp; 465 cluster_start_time_ = timestamp;
409 466
410 if (block_duration >= 0) { 467 if (encoded_duration >= 0) {
468 buffer->set_duration(base::TimeDelta::FromMicroseconds(encoded_duration));
469 } else if (block_duration >= 0) {
411 buffer->set_duration(base::TimeDelta::FromMicroseconds( 470 buffer->set_duration(base::TimeDelta::FromMicroseconds(
412 block_duration * timecode_multiplier_)); 471 block_duration * timecode_multiplier_));
413 } else { 472 } else {
414 DCHECK_NE(buffer_type, DemuxerStream::TEXT); 473 DCHECK_NE(buffer_type, DemuxerStream::TEXT);
415 buffer->set_duration(track->default_duration()); 474 buffer->set_duration(track->default_duration());
416 } 475 }
417 476
418 if (discard_padding != 0) { 477 if (discard_padding != 0) {
419 buffer->set_discard_padding(std::make_pair( 478 buffer->set_discard_padding(std::make_pair(
420 base::TimeDelta(), 479 base::TimeDelta(),
(...skipping 257 matching lines...) Expand 10 before | Expand all | Expand 10 after
678 WebMClusterParser::FindTextTrack(int track_num) { 737 WebMClusterParser::FindTextTrack(int track_num) {
679 const TextTrackMap::iterator it = text_track_map_.find(track_num); 738 const TextTrackMap::iterator it = text_track_map_.find(track_num);
680 739
681 if (it == text_track_map_.end()) 740 if (it == text_track_map_.end())
682 return NULL; 741 return NULL;
683 742
684 return &it->second; 743 return &it->second;
685 } 744 }
686 745
687 } // namespace media 746 } // namespace media
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698