Keep all codec parameters in VideoReceiveStream::Decoder

Keep all codec parameters in VideoReceiveStream::Decoder

It will be necessary to keep the H264 profile information in
VideoReceiveStream::Decoder. I think it will be easier now and for the
future to just store all of the codec parameters unmodified in
VideoReceiveStream::Decoder instead of extracting a subset of them to an
ad hoc class.

BUG=webrtc:6743, webrtc:5948
Committed: https://crrev.com/5dfac5681351242808798e7c74a5c52189fae151
Cr-Commit-Position: refs/heads/master@{#15239}

[magjed_webrtc, 2016-11-22 20:50:31]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 20:50:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-22 20:54:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 20:54:25]

Dry run: Try jobs failed on following builders:
  ios32_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/12723)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/20309)

[magjed_webrtc, 2016-11-22 20:57:16]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 20:57:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[magjed_webrtc, 2016-11-22 20:58:57]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-11-22 21:01:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 21:01:35]

Dry run: Try jobs failed on following builders:
  android_more_configs on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_more_configs/bu...)

[magjed_webrtc, 2016-11-23 13:39:51]

Description was changed from

==========
Keep all codec parameters in VideoReceiveStream::Decoder

BUG=webrtc:6743
==========

to

==========
Keep all codec parameters in VideoReceiveStream::Decoder

BUG=webrtc:6743,webrtc:5948
==========

[magjed_webrtc, 2016-11-23 13:45:21]

Description was changed from

==========
Keep all codec parameters in VideoReceiveStream::Decoder

BUG=webrtc:6743,webrtc:5948
==========

to

==========
Keep all codec parameters in VideoReceiveStream::Decoder

It will be necessary to keep the H264 profile information in
VideoReceiveStream::Decoder. I think it will be easier now and for the
future to just store all of the codec parameters unmodified in 
VideoReceiveStream::Decoder instead of extracting a subset of them to an
ad hoc class.

BUG=webrtc:6743,webrtc:5948
==========

[magjed_webrtc, 2016-11-23 13:46:58]

magjed@webrtc.org changed reviewers:
+ johan@webrtc.org, mflodman@webrtc.org

[magjed_webrtc, 2016-11-23 13:47:59]

Please take a look.
mflodman: Owner.
johan: You added the original logic in https://codereview.webrtc.org/2185953002.

[mflodman, 2016-11-23 13:56:16]

mflodman@webrtc.org changed reviewers:
+ sprang@webrtc.org
- johan@webrtc.org

[mflodman, 2016-11-23 13:56:17]

One comment, then LGTM.

Erik,
Can you take a look since Johan is on vacation and it would be good to get this
in pretty soon.

Johan,
Feel free to come with after the fact comments when your back, I moved you from
R to CC.

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/video_receive_stre...
File webrtc/video_receive_stream.h (right):

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/video_receive_stre...
webrtc/video_receive_stream.h:48: std::map<std::string, std::string>
codec_params;
Can you add a comment describing the map.

[sprang_webrtc, 2016-11-24 14:59:46]

I think this is fine, as long as we can access the parameters in
VideoSendStream.

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right):

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:3762: EXPECT_TRUE(it0 !=
cfg.decoders[0].codec_params.end());
maybe even ASSERT_TRUE, since you're dereferencing the iterator on the next
line.

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/video_receive_stre...
File webrtc/video_receive_stream.h (right):

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/video_receive_stre...
webrtc/video_receive_stream.h:48: std::map<std::string, std::string>
codec_params;
On 2016/11/23 13:56:17, mflodman wrote:
> Can you add a comment describing the map.

And preferably add some common keys as constants somewhere, such as the
sprop-parameters-set

[magjed_webrtc, 2016-11-24 16:53:15]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-24 16:53:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[magjed_webrtc, 2016-11-24 17:08:07]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-24 17:08:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-24 17:11:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-24 17:11:02]

Dry run: Try jobs failed on following builders:
  ios32_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/12778)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/19069)

[magjed_webrtc, 2016-11-24 17:11:37]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-24 17:11:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[magjed_webrtc, 2016-11-24 17:11:46]

Patchset #3 (id:60001) has been deleted

[magjed_webrtc, 2016-11-24 17:11:56]

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right):

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:3762: EXPECT_TRUE(it0 !=
cfg.decoders[0].codec_params.end());
On 2016/11/24 14:59:46, språng wrote:
> maybe even ASSERT_TRUE, since you're dereferencing the iterator on the next
> line.

Done.

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/video_receive_stre...
File webrtc/video_receive_stream.h (right):

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/video_receive_stre...
webrtc/video_receive_stream.h:48: std::map<std::string, std::string>
codec_params;
On 2016/11/24 14:59:46, språng wrote:
> On 2016/11/23 13:56:17, mflodman wrote:
> > Can you add a comment describing the map.
> 
> And preferably add some common keys as constants somewhere, such as the
> sprop-parameters-set

Done.

https://codereview.webrtc.org/2523773003/diff/20001/webrtc/video_receive_stre...
webrtc/video_receive_stream.h:48: std::map<std::string, std::string>
codec_params;
On 2016/11/23 13:56:17, mflodman wrote:
> Can you add a comment describing the map.

Done.

[commit-bot: I haz the power, 2016-11-24 17:31:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-24 17:31:53]

Dry run: This issue passed the CQ dry run.

[sprang_webrtc, 2016-11-25 10:51:52]

lgtm

[magjed_webrtc, 2016-11-25 11:54:54]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2016-11-25 11:54:54]

The patchset sent to the CQ was uploaded after l-g-t-m from mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2523773003/#ps80001 (title:
"Add common key for sprop-parameter-sets")

[commit-bot: I haz the power, 2016-11-25 11:55:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-25 11:56:38]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1480074894307680,
"parent_rev": "b5c4946c368eb8b6cf2f7204e4dd4e9c64633b69", "commit_rev":
"e518878c767e4310d06f4e7d341e28cf77bc6afc"}

[commit-bot: I haz the power, 2016-11-25 11:56:40]

Description was changed from

==========
Keep all codec parameters in VideoReceiveStream::Decoder

It will be necessary to keep the H264 profile information in
VideoReceiveStream::Decoder. I think it will be easier now and for the
future to just store all of the codec parameters unmodified in 
VideoReceiveStream::Decoder instead of extracting a subset of them to an
ad hoc class.

BUG=webrtc:6743,webrtc:5948
==========

to

==========
Keep all codec parameters in VideoReceiveStream::Decoder

It will be necessary to keep the H264 profile information in
VideoReceiveStream::Decoder. I think it will be easier now and for the
future to just store all of the codec parameters unmodified in 
VideoReceiveStream::Decoder instead of extracting a subset of them to an
ad hoc class.

BUG=webrtc:6743,webrtc:5948
==========

[commit-bot: I haz the power, 2016-11-25 11:56:42]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-11-25 11:56:44]

Description was changed from

==========
Keep all codec parameters in VideoReceiveStream::Decoder

It will be necessary to keep the H264 profile information in
VideoReceiveStream::Decoder. I think it will be easier now and for the
future to just store all of the codec parameters unmodified in 
VideoReceiveStream::Decoder instead of extracting a subset of them to an
ad hoc class.

BUG=webrtc:6743,webrtc:5948
==========

to

==========
Keep all codec parameters in VideoReceiveStream::Decoder

It will be necessary to keep the H264 profile information in
VideoReceiveStream::Decoder. I think it will be easier now and for the
future to just store all of the codec parameters unmodified in
VideoReceiveStream::Decoder instead of extracting a subset of them to an
ad hoc class.

BUG=webrtc:6743,webrtc:5948

Committed: https://crrev.com/5dfac5681351242808798e7c74a5c52189fae151
Cr-Commit-Position: refs/heads/master@{#15239}
==========

[commit-bot: I haz the power, 2016-11-25 11:56:45]

Patchset 3 (id:??) landed as
https://crrev.com/5dfac5681351242808798e7c74a5c52189fae151
Cr-Commit-Position: refs/heads/master@{#15239}