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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 306783002: [Cast] Clean-up: Merge AudioReceiverConfig+VideoReceiverConfig-->FrameReceiverConfig. (Closed)

Created:
6 years, 6 months ago by miu
Modified:
6 years, 6 months ago
Reviewers:
Alpha Left Google
CC:
chromium-reviews, hclam+watch_chromium.org, imcheng+watch_chromium.org, hguihot+watch_chromium.org, jasonroberts+watch_google.com, avayvod+watch_chromium.org, pwestin+watch_google.com, feature-media-reviews_chromium.org, miu+watch_chromium.org, hubbe+watch_chromium.org, mikhal+watch_chromium.org
Visibility:
Public.

Description

[Cast] Clean-up: Merge AudioReceiverConfig+VideoReceiverConfig-->FrameReceiverConfig. This is the last prerequisite before merging AudioReceiver with VideoReceiver. As it turns out, certain configuration details that seemed to be audio-specific or video-specific are in-fact applicable to both. Examples: 1) max_frame_rate for audio should be 100 (implying 10ms frames). 2) frequency (i.e., RTP timebase) for video should be 90 kHz, by convention. BUG=378568 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273700

Patch Set 1 #

Patch Set 2 : fix typo #

Total comments: 4

Patch Set 3 : Removed kFakeSoftwareAudio in AudioCodec enum, per hclam@. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+194 lines, -175 lines) Patch
M media/cast/audio_receiver/audio_decoder.h View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/audio_receiver/audio_decoder.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/audio_receiver/audio_decoder_unittest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M media/cast/audio_receiver/audio_receiver.h View 2 chunks +4 lines, -1 line 0 comments Download
M media/cast/audio_receiver/audio_receiver.cc View 3 chunks +10 lines, -8 lines 0 comments Download
M media/cast/audio_receiver/audio_receiver_unittest.cc View 4 chunks +8 lines, -9 lines 0 comments Download
M media/cast/cast_config.h View 1 2 chunks +53 lines, -36 lines 0 comments Download
M media/cast/cast_config.cc View 1 chunk +4 lines, -14 lines 0 comments Download
M media/cast/cast_receiver.h View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/cast_receiver_impl.h View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/cast_receiver_impl.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M media/cast/rtp_receiver/rtp_receiver.h View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/rtp_receiver/rtp_receiver.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/test/end2end_unittest.cc View 19 chunks +23 lines, -26 lines 0 comments Download
M media/cast/test/receiver.cc View 7 chunks +17 lines, -17 lines 0 comments Download
M media/cast/test/utility/default_config.h View 1 chunk +4 lines, -4 lines 0 comments Download
M media/cast/test/utility/default_config.cc View 1 chunk +14 lines, -10 lines 0 comments Download
M media/cast/test/utility/in_process_receiver.h View 2 chunks +6 lines, -6 lines 0 comments Download
M media/cast/test/utility/in_process_receiver.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/transport/cast_transport_config.h View 1 2 1 chunk +8 lines, -2 lines 0 comments Download
M media/cast/video_receiver/video_decoder.h View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/video_receiver/video_decoder.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M media/cast/video_receiver/video_decoder_unittest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M media/cast/video_receiver/video_receiver.h View 2 chunks +2 lines, -2 lines 0 comments Download
M media/cast/video_receiver/video_receiver.cc View 3 chunks +4 lines, -6 lines 0 comments Download
M media/cast/video_receiver/video_receiver_unittest.cc View 4 chunks +6 lines, -6 lines 0 comments Download
M media/cast/video_sender/external_video_encoder.cc View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
miu
hclam: PTAL.
6 years, 6 months ago (2014-05-28 23:56:41 UTC) #1
miu
hclam: BTW--Start with cast_config.h. The rest of the change is around that.
6 years, 6 months ago (2014-05-29 00:00:30 UTC) #2
Alpha Left Google
Looking pretty good. Just have a question and a request. https://codereview.chromium.org/306783002/diff/20001/media/cast/audio_receiver/audio_receiver.h File media/cast/audio_receiver/audio_receiver.h (right): https://codereview.chromium.org/306783002/diff/20001/media/cast/audio_receiver/audio_receiver.h#newcode161 ...
6 years, 6 months ago (2014-05-29 02:35:10 UTC) #3
miu
hclam: Addressed your comments: https://codereview.chromium.org/306783002/diff/20001/media/cast/audio_receiver/audio_receiver.h File media/cast/audio_receiver/audio_receiver.h (right): https://codereview.chromium.org/306783002/diff/20001/media/cast/audio_receiver/audio_receiver.h#newcode161 media/cast/audio_receiver/audio_receiver.h:161: const base::TimeDelta expected_frame_duration_; On 2014/05/29 ...
6 years, 6 months ago (2014-05-29 19:03:09 UTC) #4
Alpha Left Google
Okay. LGTM.
6 years, 6 months ago (2014-05-29 19:03:51 UTC) #5
miu
The CQ bit was checked by miu@chromium.org
6 years, 6 months ago (2014-05-29 20:28:56 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/miu@chromium.org/306783002/40001
6 years, 6 months ago (2014-05-29 20:33:26 UTC) #7
commit-bot: I haz the power
6 years, 6 months ago (2014-05-30 02:16:44 UTC) #8
Message was sent while issue was closed. 
Change committed as 273700

Powered by Google App Engine
This is Rietveld 408576698