Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
457733002:
Support for H264 HW offload for webRTC. (Closed)
|
Created:
6 years, 4 months ago by hshi1 Modified:
6 years, 4 months ago CC:
chromium-reviews, posciak+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, wjia+watch_chromium.org, xhwang Base URL:
svn://svn.chromium.org/chrome/trunk/src Project:
chromium Visibility:
Public. |
DescriptionSupport for H264 HW offload for webRTC.
RTC video encoder and decoder now recognize H264 as a supported webrtc codec type.
For H264 encoder we are required to parse NALU information to correctly populate the RTP fragmentation header.
BUG=360262
TEST=trybot, manual testing see bug report
R=dalecurtis@chromium.org, posciak@chromium.org, wuchengli@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291466
Patch Set 1 #Patch Set 2 : Replace quick-n-dirty parser with existing H264 parser. #
Total comments: 16
Patch Set 3 : address some comments by posciak. #
Total comments: 2
Patch Set 4 : More nits from posciak. Updated unittest. #
Total comments: 4
Patch Set 5 : Use DLOG. #
Total comments: 7
Patch Set 6 : Comments from wuchengli. #
Total comments: 13
Patch Set 7 : Comments by DaleCurtis #Patch Set 8 : Rebased at 291440. #
Messages
Total messages: 22 (0 generated)
Note that this CL has a dependency on kcwu's CL at https://codereview.chromium.org/426873004/ posciak and kcwu: PTAL scherkus: OWNER
Please also update rtc_video_decoder_unittest.cc. Please describe testing done in the TEST= line, it's not clear from the bug. Also has this been tested with both H264 and VP8 to ensure nothing regressed, also with apprtc? Thanks. https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_decoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_decoder.cc:148: DCHECK(codecSettings->codecType == webrtc::kVideoCodecVP8 || We should verify against the type passed to Create(), since that's what VDA has been Initialize()d with. https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:33: void GetRTPFragmentationHeaderH264( Please document. https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:43: if (result == media::H264Parser::kOk) { Nit: switch? https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:49: break; I think we should handle errors? What happens if there is an error in the stream? https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:689: case webrtc::kVideoCodecGeneric: Generic wasn't here before. And given that it's treated the same as H264 in the other file, why is it together with VP8 here? Also, do we really need both? Could we fix webrtc to not use both instead?
https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:43: if (result == media::H264Parser::kOk) { On 2014/08/10 00:30:30, Pawel Osciak wrote: > Nit: switch? I thought about that but then a "break" in the switch block would just break out of the switch, not the "while" loop. I'd need a goto or a local bool to be able to break out. https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:689: case webrtc::kVideoCodecGeneric: On 2014/08/10 00:30:30, Pawel Osciak wrote: > Generic wasn't here before. And given that it's treated the same as H264 in the > other file, why is it together with VP8 here? > Also, do we really need both? Could we fix webrtc to not use both instead? We're keeping Generic (and the payload type "CAST1") for v1 casting until we're ready to deprecate it. Currently v1 casting is still being used on Stable.
https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:689: case webrtc::kVideoCodecGeneric: On 2014/08/10 00:36:10, hshi1 wrote: > On 2014/08/10 00:30:30, Pawel Osciak wrote: > > Generic wasn't here before. And given that it's treated the same as H264 in > the > > other file, why is it together with VP8 here? > > Also, do we really need both? Could we fix webrtc to not use both instead? > > We're keeping Generic (and the payload type "CAST1") for v1 casting until we're > ready to deprecate it. Currently v1 casting is still being used on Stable. Generic is treated as H264 in the format conversion change, but not as H264 here...
https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:689: case webrtc::kVideoCodecGeneric: On 2014/08/10 01:14:34, Pawel Osciak wrote: > On 2014/08/10 00:36:10, hshi1 wrote: > > On 2014/08/10 00:30:30, Pawel Osciak wrote: > > > Generic wasn't here before. And given that it's treated the same as H264 in > > the > > > other file, why is it together with VP8 here? > > > Also, do we really need both? Could we fix webrtc to not use both instead? > > > > We're keeping Generic (and the payload type "CAST1") for v1 casting until > we're > > ready to deprecate it. Currently v1 casting is still being used on Stable. > > Generic is treated as H264 in the format conversion change, but not as H264 > here... You're correct that Generic is treated differently from H264. In webrtc point of view, "Generic" means opaque payloads not subject to the H.264 packetization logic, they are always passed as a whole chunk with no fragmentation info available. This is what we use for castV1. We only get RTP fragmentation headers for the "real" H264 codec type.
https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_decoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_decoder.cc:148: DCHECK(codecSettings->codecType == webrtc::kVideoCodecVP8 || On 2014/08/10 00:30:29, Pawel Osciak wrote: > We should verify against the type passed to Create(), since that's what VDA has > been Initialize()d with. Done (requires passing |type| to ctor of RTCVD). https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:33: void GetRTPFragmentationHeaderH264( On 2014/08/10 00:30:30, Pawel Osciak wrote: > Please document. Done. https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:49: break; On 2014/08/10 00:30:29, Pawel Osciak wrote: > I think we should handle errors? What happens if there is an error in the > stream? Should errors be expected? The stream is returned by the GpuVEA and not by user input. I think a DLOG here should suffice?
Ok, I see no better way to handle the generic case, agreed. Just as a reminder of my previous comment: please also update rtc_video_decoder_unittest.cc, please describe testing done in the TEST= line, it's not exactly clear from the bug. Did you have a chance to test this with both H264 and VP8 and with apprtc? Thanks. https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:49: break; On 2014/08/12 01:06:55, hshi1 wrote: > On 2014/08/10 00:30:29, Pawel Osciak wrote: > > I think we should handle errors? What happens if there is an error in the > > stream? > > Should errors be expected? The stream is returned by the GpuVEA and not by user > input. I think a DLOG here should suffice? Well, we check for this anyway, and I'd expect things to not work too well if we just pass this along, especially with incorrect fragmentation header. And if we assume GpuVEA is always right, should have even less issue with having a NotifyError() here, or at least dropping it... What do you think? https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:689: case webrtc::kVideoCodecGeneric: On 2014/08/11 17:43:00, hshi1 wrote: > On 2014/08/10 01:14:34, Pawel Osciak wrote: > > On 2014/08/10 00:36:10, hshi1 wrote: > > > On 2014/08/10 00:30:30, Pawel Osciak wrote: > > > > Generic wasn't here before. And given that it's treated the same as H264 > in > > > the > > > > other file, why is it together with VP8 here? > > > > Also, do we really need both? Could we fix webrtc to not use both instead? > > > > > > We're keeping Generic (and the payload type "CAST1") for v1 casting until > > we're > > > ready to deprecate it. Currently v1 casting is still being used on Stable. > > > > Generic is treated as H264 in the format conversion change, but not as H264 > > here... > > You're correct that Generic is treated differently from H264. In webrtc point of > view, "Generic" means opaque payloads not subject to the H.264 packetization > logic, they are always passed as a whole chunk with no fragmentation info > available. This is what we use for castV1. > > We only get RTP fragmentation headers for the "real" H264 codec type. Ok, I'm fine with this, but please add a comment about this. https://codereview.chromium.org/457733002/diff/60001/content/renderer/media/r... File content/renderer/media/rtc_video_decoder.h (right): https://codereview.chromium.org/457733002/diff/60001/content/renderer/media/r... content/renderer/media/rtc_video_decoder.h:198: // The video codec type, as reported to WebRTC. Nit s/to/by/ ?
https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:49: break; On 2014/08/12 11:06:38, Pawel Osciak wrote: > On 2014/08/12 01:06:55, hshi1 wrote: > > On 2014/08/10 00:30:29, Pawel Osciak wrote: > > > I think we should handle errors? What happens if there is an error in the > > > stream? > > > > Should errors be expected? The stream is returned by the GpuVEA and not by > user > > input. I think a DLOG here should suffice? > > Well, we check for this anyway, and I'd expect things to not work too well if we > just pass this along, especially with incorrect fragmentation header. And if we > assume GpuVEA is always right, should have even less issue with having a > NotifyError() here, or at least dropping it... > What do you think? Sounds reasonable. Note that this function is in the anonymous namespace. I'm making it return a bool to indicate error. https://codereview.chromium.org/457733002/diff/40001/content/renderer/media/r... content/renderer/media/rtc_video_encoder.cc:689: case webrtc::kVideoCodecGeneric: On 2014/08/12 11:06:38, Pawel Osciak wrote: > On 2014/08/11 17:43:00, hshi1 wrote: > > On 2014/08/10 01:14:34, Pawel Osciak wrote: > > > On 2014/08/10 00:36:10, hshi1 wrote: > > > > On 2014/08/10 00:30:30, Pawel Osciak wrote: > > > > > Generic wasn't here before. And given that it's treated the same as H264 > > in > > > > the > > > > > other file, why is it together with VP8 here? > > > > > Also, do we really need both? Could we fix webrtc to not use both > instead? > > > > > > > > We're keeping Generic (and the payload type "CAST1") for v1 casting until > > > we're > > > > ready to deprecate it. Currently v1 casting is still being used on Stable. > > > > > > Generic is treated as H264 in the format conversion change, but not as H264 > > > here... > > > > You're correct that Generic is treated differently from H264. In webrtc point > of > > view, "Generic" means opaque payloads not subject to the H.264 packetization > > logic, they are always passed as a whole chunk with no fragmentation info > > available. This is what we use for castV1. > > > > We only get RTP fragmentation headers for the "real" H264 codec type. > > Ok, I'm fine with this, but please add a comment about this. Done. https://codereview.chromium.org/457733002/diff/60001/content/renderer/media/r... File content/renderer/media/rtc_video_decoder.h (right): https://codereview.chromium.org/457733002/diff/60001/content/renderer/media/r... content/renderer/media/rtc_video_decoder.h:198: // The video codec type, as reported to WebRTC. On 2014/08/12 11:06:38, Pawel Osciak wrote: > Nit s/to/by/ ? Done.
lgtm % nits Wu-Cheng: could you please also verify as well? https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:705: LOG(ERROR) << "Failed to get RTP fragmentation header for H264"; s/LOG/DLOG/ https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:706: NotifyError(WEBRTC_VIDEO_CODEC_ERROR); Please use NOTIFY_ERROR()
On 2014/08/14 10:51:41, Pawel Osciak wrote: > lgtm % nits > > Wu-Cheng: could you please also verify as well? You mean verify this doesn't break VP8? > > https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... > File content/renderer/media/rtc_video_encoder.cc (right): > > https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... > content/renderer/media/rtc_video_encoder.cc:705: LOG(ERROR) << "Failed to get > RTP fragmentation header for H264"; > s/LOG/DLOG/ > > https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... > content/renderer/media/rtc_video_encoder.cc:706: > NotifyError(WEBRTC_VIDEO_CODEC_ERROR); > Please use NOTIFY_ERROR()
https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:705: LOG(ERROR) << "Failed to get RTP fragmentation header for H264"; On 2014/08/14 10:51:40, Pawel Osciak wrote: > s/LOG/DLOG/ Done. https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:706: NotifyError(WEBRTC_VIDEO_CODEC_ERROR); On 2014/08/14 10:51:40, Pawel Osciak wrote: > Please use NOTIFY_ERROR() Actually I can't use the macro NOTIFY_ERROR() because it is only defined for the Impl class (it is undef'ed at line 540).
+dalecurtis and +xhwang (OWNER of content/renderer/media) - PTAL? It seems scherkus is OOO this week. Thanks!
On 2014/08/14 13:56:35, wuchengli wrote: > On 2014/08/14 10:51:41, Pawel Osciak wrote: > > lgtm % nits > > > > Wu-Cheng: could you please also verify as well? > You mean verify this doesn't break VP8? Ah sorry, I meant could you just please take a look if this change looks good to you as well. Thanks! > > > > > https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... > > File content/renderer/media/rtc_video_encoder.cc (right): > > > > > https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... > > content/renderer/media/rtc_video_encoder.cc:705: LOG(ERROR) << "Failed to get > > RTP fragmentation header for H264"; > > s/LOG/DLOG/ > > > > > https://codereview.chromium.org/457733002/diff/100001/content/renderer/media/... > > content/renderer/media/rtc_video_encoder.cc:706: > > NotifyError(WEBRTC_VIDEO_CODEC_ERROR); > > Please use NOTIFY_ERROR()
LGTM with some nits https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... File content/renderer/media/rtc_video_decoder.cc (right): https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... content/renderer/media/rtc_video_decoder.cc:138: // vda can be NULL if VP8 is not supported. s/VP8/the codec/ https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... File content/renderer/media/rtc_video_decoder_unittest.cc (right): https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... content/renderer/media/rtc_video_decoder_unittest.cc:111: CreateDecoder(webrtc::kVideoCodecVP8); This can be removed. This test only wants to check RTCVideoDecoder::Create will return null if the codec is unsupported. https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... content/renderer/media/rtc_video_decoder_unittest.cc:124: codec_.codecType = webrtc::kVideoCodecVP8; remove this line
https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... File content/renderer/media/rtc_video_decoder.cc (right): https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... content/renderer/media/rtc_video_decoder.cc:138: // vda can be NULL if VP8 is not supported. On 2014/08/15 03:20:20, wuchengli wrote: > s/VP8/the codec/ Done. https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... File content/renderer/media/rtc_video_decoder_unittest.cc (right): https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... content/renderer/media/rtc_video_decoder_unittest.cc:111: CreateDecoder(webrtc::kVideoCodecVP8); On 2014/08/15 03:20:20, wuchengli wrote: > This can be removed. This test only wants to check RTCVideoDecoder::Create will > return null if the codec is unsupported. I thought so too, but it turns out that, for some reason I don't fully understand, the test fails if I remove this line. Note that before my change, we always create the decoder in SetUp(), so I think there's some assumption made about this during TearDown() that would be broken if we don't create the decoder. https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... content/renderer/media/rtc_video_decoder_unittest.cc:124: codec_.codecType = webrtc::kVideoCodecVP8; On 2014/08/15 03:20:20, wuchengli wrote: > remove this line Done.
https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... File content/renderer/media/rtc_video_decoder_unittest.cc (right): https://codereview.chromium.org/457733002/diff/120001/content/renderer/media/... content/renderer/media/rtc_video_decoder_unittest.cc:111: CreateDecoder(webrtc::kVideoCodecVP8); On 2014/08/15 03:49:04, hshi1 wrote: > On 2014/08/15 03:20:20, wuchengli wrote: > > This can be removed. This test only wants to check RTCVideoDecoder::Create > will > > return null if the codec is unsupported. > > I thought so too, but it turns out that, for some reason I don't fully > understand, the test fails if I remove this line. > > Note that before my change, we always create the decoder in SetUp(), so I think > there's some assumption made about this during TearDown() that would be broken > if we don't create the decoder. I see. Maybe DeleteSoon cannot be called with NULL.
+acolwell to verify the H264 parser usage. I'm not familiar with it. xhwang -> cc since he's OOO. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... File content/renderer/media/rtc_video_decoder.cc (right): https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_decoder.cc:150: DCHECK(video_codec_type_ == codecSettings->codecType); DCHECK_EQ https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:37: webrtc::RTPFragmentationHeader& header, uint8_t* data, uint32_t length) { non-const& not allowed. make it a pointer. Data can be const? https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:44: media::H264Parser::Result result; Combine line with result = below. const result if you like. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:689: webrtc::RTPFragmentationHeader header; Instead of using memset just add a = {0} here. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:716: memset(&info, 0, sizeof(info)); Ditto. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... File content/renderer/media/rtc_video_encoder_factory.cc (right): https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder_factory.cc:18: std::vector<cricket::WebRtcVideoEncoderFactory::VideoCodec> VEAToWebRTCCodecs( Instead of returning a vector by copy, just take a pointer to one and add elements to it. Then you don't need the extra insert below either.
https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... File content/renderer/media/rtc_video_decoder.cc (right): https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_decoder.cc:150: DCHECK(video_codec_type_ == codecSettings->codecType); On 2014/08/15 16:48:49, DaleCurtis wrote: > DCHECK_EQ Done. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:37: webrtc::RTPFragmentationHeader& header, uint8_t* data, uint32_t length) { On 2014/08/15 16:48:50, DaleCurtis wrote: > non-const& not allowed. make it a pointer. Data can be const? Done. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:44: media::H264Parser::Result result; On 2014/08/15 16:48:49, DaleCurtis wrote: > Combine line with result = below. const result if you like. Done. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:689: webrtc::RTPFragmentationHeader header; On 2014/08/15 16:48:50, DaleCurtis wrote: > Instead of using memset just add a = {0} here. Sorry I'm afraid I can't do that. RTPFragmentationHeader is a class, not a struct. "= {0}" is not allowed. If I attempt that I get compiler error "error: could not convert '{0}' from '<brace-enclosed initializer list>' to 'webrtc::RTPFragmentationHeader'". https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:716: memset(&info, 0, sizeof(info)); On 2014/08/15 16:48:49, DaleCurtis wrote: > Ditto. Ditto, not allowed. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... File content/renderer/media/rtc_video_encoder_factory.cc (right): https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder_factory.cc:18: std::vector<cricket::WebRtcVideoEncoderFactory::VideoCodec> VEAToWebRTCCodecs( On 2014/08/15 16:48:50, DaleCurtis wrote: > Instead of returning a vector by copy, just take a pointer to one and add > elements to it. Then you don't need the extra insert below either. Done.
lgtm % verification of h264 parser usage. https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... File content/renderer/media/rtc_video_encoder.cc (right): https://codereview.chromium.org/457733002/diff/140001/content/renderer/media/... content/renderer/media/rtc_video_encoder.cc:689: webrtc::RTPFragmentationHeader header; On 2014/08/15 20:14:56, hshi1 wrote: > On 2014/08/15 16:48:50, DaleCurtis wrote: > > Instead of using memset just add a = {0} here. > > Sorry I'm afraid I can't do that. RTPFragmentationHeader is a class, not a > struct. "= {0}" is not allowed. > > If I attempt that I get compiler error "error: could not convert '{0}' from > '<brace-enclosed initializer list>' to 'webrtc::RTPFragmentationHeader'". Hmm, I don't think this is the right way to zero out header if it's a class then. It should self initialize its member variables or be a struct. Since this is already done in this file, it's okay to fix in a followup.
The CQ bit was checked by hshi@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hshi@chromium.org/457733002/200001
Message was sent while issue was closed.
Committed patchset #8 manually as 291466 (presubmit successful). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
