Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1297653002:
Tool to convert RtcEventLog files to RtpDump format. (Closed)
|
Created:
5 years, 4 months ago by ivoc Modified:
5 years, 3 months ago CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), Andrew MacDonald, stefan-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, andresp, perkj_webrtc, mflodman Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionTool to convert RtcEventLog files to RtpDump format.
This is a small utility that reads RtcEventLog files, and converts the RTP headers within it to RtpDump format. All other types of events are ignored. Three command-line flags are supported, --audio-only, --video-only and --data-only. When one of these flags is supplied, only RTP packets that match the requested type are converted.
BUG=webrtc:4741
R=henrik.lundin@webrtc.org, kjellander@webrtc.org, stefan@webrtc.org, terelius@webrtc.org
Committed: https://crrev.com/35624c2c3686a2ad40daffe073aa78507b0ef88e
Cr-Commit-Position: refs/heads/master@{#9980}
Patch Set 1 : Initial version #
Total comments: 13
Patch Set 2 : Added flag for SSRC-based filtering. #
Total comments: 4
Patch Set 3 : Addressed review comments from hlundin. #Patch Set 4 : Added missing include. #
Total comments: 5
Patch Set 5 : Changed the meaning of the command-line flags, based on Stefan's feedback. #Patch Set 6 : Added support for logging RTCP packets. #
Total comments: 5
Patch Set 7 : Inverted the command line flags and fixed bug when converting RTCP packets. #
Total comments: 2
Patch Set 8 : Rewrote SSRC parsing function to use stringstream instead of strtoul #Patch Set 9 : Fixed order of includes. #Patch Set 10 : Rebase. #Patch Set 11 : Changed CHECK to RTC_CHECK. #
Total comments: 3
Messages
Total messages: 41 (9 generated)
ivoc@webrtc.org changed reviewers: + henrik.lundin@webrtc.org, stefan@webrtc.org, terelius@webrtc.org
Please have a look at this conversion tool. Once terelius@' CL for parsing convenience functions lands, that functionality can be used in this tool, but that should only be a relatively small change to the code.
lgtm Maybe we should consider making a separate folder for the event log? I think we have 7 or 8 files by now, that are related to RtcEventLog. https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp File webrtc/webrtc.gyp (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp#newcode39 webrtc/webrtc.gyp:39: 'rtc_event_log_proto', Do we need to explicitly depend on both rtc_event_log and rtc_event_log_proto? rtc_event_log already depends on rtc_event_log_proto (at least assuming that protobuf is enabled).
https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:26: DEFINE_bool(audio_only, Feature request: Can you add a flag to store only packets with a specific SSRC? There is a similar feature in NetEqRtpPlay. https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:81: const webrtc::rtclog::Event& event = event_stream.stream(i); const auto& ? Or is the compiler unable to deduce the type in this case? https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:104: packet.original_length = event.rtp_packet().packet_length(); I think it would be handy to know if the output is a header-only RTP dump or if the packets are complete with payloads. Suggestion: declare a bool header_only before the for loop and initialize to false. If any packet has an original_length > length, then set it to true. Print out to screen after loop. https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp File webrtc/webrtc.gyp (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp#newcode33 webrtc/webrtc.gyp:33: 'target_name': 'rtc_event_log2rtp_dump', I'm not sure that webrtc.gyp is the right place for this target. Since the source file is in webrtc/video, is there any gyp(i) file closer to the source?
https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:81: const webrtc::rtclog::Event& event = event_stream.stream(i); On 2015/08/17 11:14:34, hlundin-webrtc wrote: > const auto& ? Or is the compiler unable to deduce the type in this case? Personally, I like being explicit with the types. I guess it is a matter of project conventions though... https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp File webrtc/webrtc.gyp (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp#newcode33 webrtc/webrtc.gyp:33: 'target_name': 'rtc_event_log2rtp_dump', On 2015/08/17 11:14:34, hlundin-webrtc wrote: > I'm not sure that webrtc.gyp is the right place for this target. Since the > source file is in webrtc/video, is there any gyp(i) file closer to the source? Yes, there is a webrtc_video.gypi, but it just contains one big list of source files. It does not contain any targets or build rules, which is why we placed the rtc_event_log target in webrtc.gyp. I agree that creating a large number of (not really video-related) files in webrtc/video and corresponding targets in webrtc.gyp is bad. That's why I suggested moving the rtc_event_log stuff to it's own folder, but I think that might be better to do in a separate CL. Do you agree?
https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:26: DEFINE_bool(audio_only, On 2015/08/17 11:14:34, hlundin-webrtc wrote: > Feature request: Can you add a flag to store only packets with a specific SSRC? > There is a similar feature in NetEqRtpPlay. Done. https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:81: const webrtc::rtclog::Event& event = event_stream.stream(i); On 2015/08/17 11:39:06, terelius wrote: > On 2015/08/17 11:14:34, hlundin-webrtc wrote: > > const auto& ? Or is the compiler unable to deduce the type in this case? > > Personally, I like being explicit with the types. I guess it is a matter of > project conventions though... I tend to agree with terelius@ on this one, let me know if you think it should change to auto. https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:104: packet.original_length = event.rtp_packet().packet_length(); On 2015/08/17 11:14:33, hlundin-webrtc wrote: > I think it would be handy to know if the output is a header-only RTP dump or if > the packets are complete with payloads. Suggestion: declare a bool header_only > before the for loop and initialize to false. If any packet has an > original_length > length, then set it to true. Print out to screen after loop. Done. https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp File webrtc/webrtc.gyp (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp#newcode33 webrtc/webrtc.gyp:33: 'target_name': 'rtc_event_log2rtp_dump', On 2015/08/17 11:39:06, terelius wrote: > On 2015/08/17 11:14:34, hlundin-webrtc wrote: > > I'm not sure that webrtc.gyp is the right place for this target. Since the > > source file is in webrtc/video, is there any gyp(i) file closer to the source? > > Yes, there is a webrtc_video.gypi, but it just contains one big list of source > files. It does not contain any targets or build rules, which is why we placed > the rtc_event_log target in webrtc.gyp. > > I agree that creating a large number of (not really video-related) files in > webrtc/video and corresponding targets in webrtc.gyp is bad. That's why I > suggested moving the rtc_event_log stuff to it's own folder, but I think that > might be better to do in a separate CL. Do you agree? For now I think webrtc.gyp is the right place, since that is also where the rtc_event_log and rtc_event_log_proto targets are defined. I agree that moving the event log files to a separate folder (and separate gyp file) makes sense, but it should indeed be in a seperate CL. https://codereview.webrtc.org/1297653002/diff/1/webrtc/webrtc.gyp#newcode39 webrtc/webrtc.gyp:39: 'rtc_event_log_proto', On 2015/08/17 08:19:48, terelius wrote: > Do we need to explicitly depend on both rtc_event_log and rtc_event_log_proto? > rtc_event_log already depends on rtc_event_log_proto (at least assuming that > protobuf is enabled). I'm afraid it is needed here, because the log-parsing functionality is used. The path to the automatically generated protobuf headers is set in rtc_event_log_proto.
LGTM with two nits. https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/1/webrtc/video/rtc_event_log2rt... webrtc/video/rtc_event_log2rtp_dump.cc:81: const webrtc::rtclog::Event& event = event_stream.stream(i); On 2015/08/21 14:30:57, ivoc wrote: > On 2015/08/17 11:39:06, terelius wrote: > > On 2015/08/17 11:14:34, hlundin-webrtc wrote: > > > const auto& ? Or is the compiler unable to deduce the type in this case? > > > > Personally, I like being explicit with the types. I guess it is a matter of > > project conventions though... > > I tend to agree with terelius@ on this one, let me know if you think it should > change to auto. Acknowledged. https://codereview.webrtc.org/1297653002/diff/20001/webrtc/video/rtc_event_lo... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/20001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:21: #include "webrtc/modules/rtp_rtcp/source/byte_io.h" Wrong order. https://codereview.webrtc.org/1297653002/diff/20001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:137: uint32_t packet_ssrc = webrtc::ByteReader<uint32_t>::ReadBigEndian( const uint32_t
The CQ bit was checked by ivoc@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1297653002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1297653002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/501)
The CQ bit was checked by ivoc@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1297653002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1297653002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/506)
https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:44: "RTPdump file."); Maybe these flags should be: --video (includes video) --audio (includes audio) etc... and without any flag all are included?
https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:44: "RTPdump file."); On 2015/09/07 11:12:20, stefan-webrtc (holmer) wrote: > Maybe these flags should be: > --video (includes video) > --audio (includes audio) > etc... > > and without any flag all are included? Maybe you want an option for including rtcp as well?
Thanks for the feedback. See my replies below. https://codereview.webrtc.org/1297653002/diff/20001/webrtc/video/rtc_event_lo... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/20001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:21: #include "webrtc/modules/rtp_rtcp/source/byte_io.h" On 2015/08/26 09:24:37, hlundin-webrtc wrote: > Wrong order. Done. https://codereview.webrtc.org/1297653002/diff/20001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:137: uint32_t packet_ssrc = webrtc::ByteReader<uint32_t>::ReadBigEndian( On 2015/08/26 09:24:37, hlundin-webrtc wrote: > const uint32_t Done. https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:44: "RTPdump file."); On 2015/09/07 11:13:20, stefan-webrtc (holmer) wrote: > On 2015/09/07 11:12:20, stefan-webrtc (holmer) wrote: > > Maybe these flags should be: > > --video (includes video) > > --audio (includes audio) > > etc... > > > > and without any flag all are included? > > Maybe you want an option for including rtcp as well? About the flags: good idea, have changed them. About RTCP packets: This was briefly discussed but we couldn't think of a compelling use case for this, do you think it will be a useful feature to have? Maybe for BWE?
https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:44: "RTPdump file."); On 2015/09/09 09:18:56, ivoc wrote: > On 2015/09/07 11:13:20, stefan-webrtc (holmer) wrote: > > On 2015/09/07 11:12:20, stefan-webrtc (holmer) wrote: > > > Maybe these flags should be: > > > --video (includes video) > > > --audio (includes audio) > > > etc... > > > > > > and without any flag all are included? > > > > Maybe you want an option for including rtcp as well? > About the flags: good idea, have changed them. > > About RTCP packets: This was briefly discussed but we couldn't think of a > compelling use case for this, do you think it will be a useful feature to have? > Maybe for BWE? Yes, I think it could be useful for BWE, as RTCP is what drives BWE on the send-side.
Added support for RTCP packets, and two additional command-line flags. The --rtp flag indicates that RTP packets should be converted and the --rtcp flag indicates that RTCP packets should be converted. If neither is specified, both types of packets are converted (similar to the --audio --video and --data flags).
https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/60001/webrtc/video/rtc_event_lo... webrtc/video/rtc_event_log2rtp_dump.cc:44: "RTPdump file."); On 2015/09/09 09:27:16, stefan-webrtc (holmer) wrote: > On 2015/09/09 09:18:56, ivoc wrote: > > On 2015/09/07 11:13:20, stefan-webrtc (holmer) wrote: > > > On 2015/09/07 11:12:20, stefan-webrtc (holmer) wrote: > > > > Maybe these flags should be: > > > > --video (includes video) > > > > --audio (includes audio) > > > > etc... > > > > > > > > and without any flag all are included? > > > > > > Maybe you want an option for including rtcp as well? > > About the flags: good idea, have changed them. > > > > About RTCP packets: This was briefly discussed but we couldn't think of a > > compelling use case for this, do you think it will be a useful feature to > have? > > Maybe for BWE? > > Yes, I think it could be useful for BWE, as RTCP is what drives BWE on the > send-side. Forgot to add a comment here. I added support for RTCP packets in the latest version. There are now two extra command-line switches: --rtp and --rtcp, and if neither of these is used then both types of packets will be converted, just like how the --audio --video and --data flags work.
https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... webrtc/video/rtc_event_log2rtp_dump.cc:34: false, Shouldn't they all be default true? https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... webrtc/video/rtc_event_log2rtp_dump.cc:188: if (FLAGS_rtcp && event.has_type() && event.type() == event.RTCP_EVENT) { According to http://www.cs.columbia.edu/irt/software/rtptools/#rtpdump, RTCP packets should be written to file with a plen value of 0.
https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... webrtc/video/rtc_event_log2rtp_dump.cc:34: false, On 2015/09/11 10:31:47, hlundin-webrtc wrote: > Shouldn't they all be default true? You have a point that the default values are confusing now (these are visible in the help menu). I inverted all of the flags, so there is now a flag to exclude audio (--noaudio) instead of to include it. The default value makes sense now and it is also more intuitive when not using any flags. https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... webrtc/video/rtc_event_log2rtp_dump.cc:188: if (FLAGS_rtcp && event.has_type() && event.type() == event.RTCP_EVENT) { On 2015/09/11 10:31:47, hlundin-webrtc wrote: > According to http://www.cs.columbia.edu/irt/software/rtptools/#rtpdump, RTCP > packets should be written to file with a plen value of 0. Good catch, thanks! I hade to remove a CHECK in the RtpDumpWriter class to make this work.
lgtm again. https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/100001/webrtc/video/rtc_event_l... webrtc/video/rtc_event_log2rtp_dump.cc:188: if (FLAGS_rtcp && event.has_type() && event.type() == event.RTCP_EVENT) { On 2015/09/11 12:34:40, ivoc wrote: > On 2015/09/11 10:31:47, hlundin-webrtc wrote: > > According to http://www.cs.columbia.edu/irt/software/rtptools/#rtpdump, RTCP > > packets should be written to file with a plen value of 0. > > Good catch, thanks! I hade to remove a CHECK in the RtpDumpWriter class to make > this work. Acknowledged.
LG, but what do you think of my last suggestion? https://codereview.webrtc.org/1297653002/diff/120001/webrtc/video/rtc_event_l... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/120001/webrtc/video/rtc_event_l... webrtc/video/rtc_event_log2rtp_dump.cc:56: bool ParseSsrc(const std::string& str, uint32_t* ssrc) { I wrote some code to do something similar here: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc... I think you can make this a lot more readable with a stringstream object.
https://codereview.webrtc.org/1297653002/diff/120001/webrtc/video/rtc_event_l... File webrtc/video/rtc_event_log2rtp_dump.cc (right): https://codereview.webrtc.org/1297653002/diff/120001/webrtc/video/rtc_event_l... webrtc/video/rtc_event_log2rtp_dump.cc:56: bool ParseSsrc(const std::string& str, uint32_t* ssrc) { On 2015/09/11 14:10:29, stefan-webrtc (holmer) wrote: > I wrote some code to do something similar here: > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc... > > I think you can make this a lot more readable with a stringstream object. Thanks for the suggestion, I rewrote the function using stringstream, and I think the result is shorter and more readable.
lgtm
The CQ bit was checked by ivoc@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1297653002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1297653002/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_drmemory_light on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
ivoc@webrtc.org changed reviewers: + kjellander@webrtc.org
https://codereview.webrtc.org/1297653002/diff/200001/webrtc/test/rtp_file_wri... File webrtc/test/rtp_file_writer.cc (left): https://codereview.webrtc.org/1297653002/diff/200001/webrtc/test/rtp_file_wri... webrtc/test/rtp_file_writer.cc:43: RTC_CHECK_GE(packet->original_length, packet->length); Can you explain why this check needs to go away?
https://codereview.chromium.org/1297653002/diff/200001/webrtc/test/rtp_file_w... File webrtc/test/rtp_file_writer.cc (left): https://codereview.chromium.org/1297653002/diff/200001/webrtc/test/rtp_file_w... webrtc/test/rtp_file_writer.cc:43: RTC_CHECK_GE(packet->original_length, packet->length); On 2015/09/17 14:49:36, kjellander (webrtc) wrote: > Can you explain why this check needs to go away? Of course, I added support for writing RTCP packets in the RTPdump format, and for RTCP packets the original_length should be set to 0 (see for example the plen field: http://www.cs.columbia.edu/irt/software/rtptools/#rtpdump ). So this CHECK is preventing RTCP packets from being written.
lgtm https://codereview.webrtc.org/1297653002/diff/200001/webrtc/test/rtp_file_wri... File webrtc/test/rtp_file_writer.cc (left): https://codereview.webrtc.org/1297653002/diff/200001/webrtc/test/rtp_file_wri... webrtc/test/rtp_file_writer.cc:43: RTC_CHECK_GE(packet->original_length, packet->length); On 2015/09/17 14:53:29, ivoc wrote: > On 2015/09/17 14:49:36, kjellander (webrtc) wrote: > > Can you explain why this check needs to go away? > > Of course, I added support for writing RTCP packets in the RTPdump format, and > for RTCP packets the original_length should be set to 0 (see for example the > plen field: http://www.cs.columbia.edu/irt/software/rtptools/#rtpdump ). So this > CHECK is preventing RTCP packets from being written. Thanks for the explanation!
Message was sent while issue was closed.
Committed patchset #11 (id:200001) manually as 35624c2c3686a2ad40daffe073aa78507b0ef88e (presubmit successful).
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/35624c2c3686a2ad40daffe073aa78507b0ef88e Cr-Commit-Position: refs/heads/master@{#9980}
Message was sent while issue was closed.
kjellander@google.com changed reviewers: + kjellander@google.com
Message was sent while issue was closed.
This breaks the bots in https://build.chromium.org/p/chromium.webrtc.fyi because all our test code should be inside "include_tests==1" conditions in order to not be included in Chromium. Chromium doesn't have gflags.gyp, which is why it breaks. The fix is easy - just move the target inside include_tests==1. Feel free to TBR me as I have to run on an errand now.
Message was sent while issue was closed.
A revert of this CL (patchset #11 id:200001) has been created in https://codereview.webrtc.org/1353123002/ by grunell@google.com. The reason for reverting is: Breaks Chromium WebRTC FYI bots. Updating projects from gyp files... gyp: /b/build/slave/linux/build/src/third_party/gflags/gflags.gyp not found (cwd: /b/build/slave/linux/build) Error: Command '/usr/bin/python src/build/gyp_chromium' returned non-zero exit status 1 in /b/build/slave/linux/build .
Message was sent while issue was closed.
A revert of this CL (patchset #11 id:200001) has been created in https://codereview.webrtc.org/1345983009/ by henrikg@webrtc.org. The reason for reverting is: Breaks Chromium WebRTC FYI bots. Updating projects from gyp files... gyp: /b/build/slave/linux/build/src/third_party/gflags/gflags.gyp not found (cwd: /b/build/slave/linux/build) Error: Command '/usr/bin/python src/build/gyp_chromium' returned non-zero exit status 1 in /b/build/slave/linux/build . |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
