Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
246853005:
Fix SPS/PPS insertion logic in MP4StreamParser. (Closed)
|
Created:
6 years, 8 months ago by acolwell GONE FROM CHROMIUM Modified:
6 years, 8 months ago CC:
chromium-reviews, feature-media-reviews_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionFix SPS/PPS insertion logic in MP4StreamParser.
The code that inserts SPS & PPS NAL units into keyframes wasn't properly
handling frames that start with AUD NAL units. This patch adds code to
make sure the configuration data is inserted after the AUD NAL units as
the H.264 spec mandates.
I've also added AVC::IsValidAnnexB() DCHECKS to verify that our
code generates proper Annex B data that conforms to the NAL unit
ordering rules outlined in the spec. These DCHECKS caught the old bad
behavior and should help prevent future regressions.
BUG=364925
TEST=AVCConversionTest.NALUSizeTooLarge, AVCConversionTest.NALUSizeIsZero, AVCConversionTest.ValidAnnexBConstructs, AVCConversionTest.InvalidAnnexBConstructs, AVCConversionTest.InsertParamSetsAnnexB
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266316
Patch Set 1 #Patch Set 2 : Add a bunch of tests #
Total comments: 1
Patch Set 3 : Make IsValidAnnexB() more permissive #Patch Set 4 : Move typedef in test to try to make the Android bot happy. #
Total comments: 18
Patch Set 5 : Rebase #Patch Set 6 : Address CR comments #
Total comments: 29
Patch Set 7 : Address CR comments. #
Total comments: 4
Patch Set 8 : Address CR comments #Patch Set 9 : Fix Android compiler error. #Patch Set 10 : Rebase #
Messages
Total messages: 28 (0 generated)
https://codereview.chromium.org/246853005/diff/20001/media/formats/mp4/mp4_st... File media/formats/mp4/mp4_stream_parser.cc (right): https://codereview.chromium.org/246853005/diff/20001/media/formats/mp4/mp4_st... media/formats/mp4/mp4_stream_parser.cc:370: if (!subsamples->empty()) Would be nice if InsertParamSetsAnnexB returns the insert position so that some sanity check can be done for encrypted content: if (!subsamples->empty() && frame_buf->size() > old_size) { DCHECK_LE(insert_position, (*subsamples)[0].clear_bytes); ... }
After chatting w/ Damien and rereading the relevant parts of the spec, I believe the AVC::IsValidAnnexB() is too strict and prevents valid non-VCL NAL unit orderings. I'm going to simplify the logic so it detects the situation that was causing the bug. We can always backfill with further checks as the need arises.
PTAL. The verification state machine is much simpler now and only enforces the constraints explicitly stated in the bullets in Section 7.4.1.2.3 after the sentence "The following constraints shall be obeyed by the order of the coded pictures and non-VCL NAL units within an access unit." I also changed the code to always insert the parameter sets. My understanding is that if there are already parameter sets in the access unit, they will simply override the ones inserted.
https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:31: DVLOG(1) << __FUNCTION__ << " nal_size is 0"; nit: Chome LOG already includes file and line number. Do we really need __FUNCTION__ ? https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:60: DVLOG(1) << __FUNCTION__ << " nal_size is 0"; Ditto. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:149: kExpectAUD, Naming is not really inline with the state machine: the state is not really "expect" but rather kAUDAllowed. Same for the other states. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:186: if (order_state < kExpectFirstVCL) The condition is not needed. At this point, the next state is always kExpectFirstVCL. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:192: if (order_state > kExpectVCL) { I don't think this condition is correct. It should be kExpectFirstVCL (and in this case, we could merge it with the previous cases (which includes kSEIMessage). Having an SPS/PPS after the first VCL NAL would mean the start of a new access unit. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:219: if (order_state < kExpectVCL) Condition could be removed. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:250: if (order_state != kExpectVCL) { From the spec, seems like we could have a Filler NALU after an end of sequence. The condition should be: if (!(order_state >= kExpectVCL && order_state < kEOSStream)) return false; https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:271: case H264Parser::kUnsupportedStream: Just for info: 1) Unsupported streams by the media::H264Parser does not mean it cannot be supported by FFMpegVideoDecoder for example. So you might artificially add some restrictions to the set of streams that can be supported by Chrome. 2) We have the same issue with the Mpeg2 TS parser with some of the streams: for example, gaps_in_frame_num_value_allowed_flag returns kUnsupportedStream. But this flag should not have any impact on the parsing. In this specific case, I need to check whether it's returning kUnsupportedStream because the underlying GPU video decoder does not support such stream (which is different from the parser cannot parse such streams). Summary: the intent of kUnsupportedStream in H264Parser is not really well defined. To me, it should not be related to the capabilities of the GPU video decoder. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/mp4_st... File media/formats/mp4/mp4_stream_parser.cc (left): https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/mp4_st... media/formats/mp4/mp4_stream_parser.cc:372: if (!subsamples->empty()) Would be nice if InsertParamSetsAnnexB returns the insert position so that some sanity check can be done for encrypted content: if (!subsamples->empty() && frame_buf->size() > old_size) { DCHECK_LE(insert_position, (*subsamples)[0].clear_bytes); ... }
https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:31: DVLOG(1) << __FUNCTION__ << " nal_size is 0"; nit: Chome LOG already includes file and line number. Do we really need __FUNCTION__ ? https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:60: DVLOG(1) << __FUNCTION__ << " nal_size is 0"; Ditto. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:149: kExpectAUD, Naming is not really inline with the state machine: the state is not really "expect" but rather kAUDAllowed. Same for the other states. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:186: if (order_state < kExpectFirstVCL) The condition is not needed. At this point, the next state is always kExpectFirstVCL. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:192: if (order_state > kExpectVCL) { I don't think this condition is correct. It should be kExpectFirstVCL (and in this case, we could merge it with the previous cases (which includes kSEIMessage). Having an SPS/PPS after the first VCL NAL would mean the start of a new access unit. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:219: if (order_state < kExpectVCL) Condition could be removed. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:250: if (order_state != kExpectVCL) { From the spec, seems like we could have a Filler NALU after an end of sequence. The condition should be: if (!(order_state >= kExpectVCL && order_state < kEOSStream)) return false; https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:271: case H264Parser::kUnsupportedStream: Just for info: 1) Unsupported streams by the media::H264Parser does not mean it cannot be supported by FFMpegVideoDecoder for example. So you might artificially add some restrictions to the set of streams that can be supported by Chrome. 2) We have the same issue with the Mpeg2 TS parser with some of the streams: for example, gaps_in_frame_num_value_allowed_flag returns kUnsupportedStream. But this flag should not have any impact on the parsing. In this specific case, I need to check whether it's returning kUnsupportedStream because the underlying GPU video decoder does not support such stream (which is different from the parser cannot parse such streams). Summary: the intent of kUnsupportedStream in H264Parser is not really well defined. To me, it should not be related to the capabilities of the GPU video decoder. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/mp4_st... File media/formats/mp4/mp4_stream_parser.cc (left): https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/mp4_st... media/formats/mp4/mp4_stream_parser.cc:372: if (!subsamples->empty()) Would be nice if InsertParamSetsAnnexB returns the insert position so that some sanity check can be done for encrypted content: if (!subsamples->empty() && frame_buf->size() > old_size) { DCHECK_LE(insert_position, (*subsamples)[0].clear_bytes); ... }
https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:31: DVLOG(1) << __FUNCTION__ << " nal_size is 0"; On 2014/04/23 19:07:28, damienv1 wrote: > nit: Chome LOG already includes file and line number. Do we really need > __FUNCTION__ ? Done. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:60: DVLOG(1) << __FUNCTION__ << " nal_size is 0"; On 2014/04/23 19:07:28, damienv1 wrote: > Ditto. Done. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:149: kExpectAUD, On 2014/04/23 19:07:28, damienv1 wrote: > Naming is not really inline with the state machine: the state is not really > "expect" but rather kAUDAllowed. > > Same for the other states. Done. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:186: if (order_state < kExpectFirstVCL) On 2014/04/23 19:07:28, damienv1 wrote: > The condition is not needed. > At this point, the next state is always kExpectFirstVCL. Done. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:192: if (order_state > kExpectVCL) { On 2014/04/23 19:07:28, damienv1 wrote: > I don't think this condition is correct. > It should be kExpectFirstVCL (and in this case, we could merge it with the > previous cases (which includes kSEIMessage). Done. > > Having an SPS/PPS after the first VCL NAL would mean the start of a new access > unit. Yep. Forgot that. :) https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:219: if (order_state < kExpectVCL) On 2014/04/23 19:07:28, damienv1 wrote: > Condition could be removed. Done. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:250: if (order_state != kExpectVCL) { On 2014/04/23 19:07:28, damienv1 wrote: > From the spec, seems like we could have a Filler NALU after an end of sequence. > The condition should be: > if (!(order_state >= kExpectVCL && order_state < kEOSStream)) > return false; Done. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/avc.cc... media/formats/mp4/avc.cc:271: case H264Parser::kUnsupportedStream: On 2014/04/23 19:07:28, damienv1 wrote: > Just for info: > 1) Unsupported streams by the media::H264Parser does not mean it cannot be > supported by FFMpegVideoDecoder for example. So you might artificially add some > restrictions to the set of streams that can be supported by Chrome. > 2) We have the same issue with the Mpeg2 TS parser with some of the streams: for > example, gaps_in_frame_num_value_allowed_flag returns kUnsupportedStream. But > this flag should not have any impact on the parsing. In this specific case, I > need to check whether it's returning kUnsupportedStream because the underlying > GPU video decoder does not support such stream (which is different from the > parser cannot parse such streams). Summary: the intent of kUnsupportedStream in > H264Parser is not really well defined. To me, it should not be related to the > capabilities of the GPU video decoder. ok. Thanks for the info. It doesn't look like AdvanceToNextNALU() should ever return kUnsupportedStream. I've separated these 2 cases and put a NOTREACHED() for the kUnsupportedStream case so that we get notified if that method changes its behavior. https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/mp4_st... File media/formats/mp4/mp4_stream_parser.cc (left): https://codereview.chromium.org/246853005/diff/60001/media/formats/mp4/mp4_st... media/formats/mp4/mp4_stream_parser.cc:372: if (!subsamples->empty()) On 2014/04/23 19:07:28, damienv1 wrote: > Would be nice if InsertParamSetsAnnexB returns the insert position so that some > sanity check can be done for encrypted content: > > if (!subsamples->empty() && frame_buf->size() > old_size) { > DCHECK_LE(insert_position, (*subsamples)[0].clear_bytes); > ... > } This code was totally broken for the AUD case. Updated the code to what I believe the correct fix is. Waiting on receiving an encrypted file from a teammate to verify the changes.
https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:97: subsamples_insert_point++; I don't think the logic is correct here and will not be in the general case. Let's say you have an AUD (from the CENC spec, it will not be encrypted), then let's say you have a slice NALU, the NALU header is not ecnrypted but the content could be encrypted. In this case, you are adding one subsample entry at the wrong location. I think the previous code was correct: only the first subsample should be modified (no need for an extra entry) since an AUD should not be encrypted according to the CENC spec (we could return false if the insert position is strictly greater than subsamples[0].clear_bytes. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:185: for (bool done = false; !done;) { nit: would personally write a while loop here, but up to you. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.h File media/formats/mp4/avc.h (right): https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.h... media/formats/mp4/avc.h:31: std::vector<SubsampleEntry>* subsamples); nit: From http://www.chromium.org/developers/coding-style/cpp-dos-and-donts we could forward declare SubsampleEntry.
lgtm w/ nits I'll leave it up to damien to verify the detailed h264 bitstream stuff https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:31: DVLOG(1) << " nal_size is 0"; nit: remove leading space? https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:60: DVLOG(1) << " nal_size is 0"; ditto https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:176: kBeforeFirstVCL, // VCL == nal_unit_types 1-5 nit: two spaces before // comments https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:185: for (bool done = false; !done;) { On 2014/04/24 00:03:58, damienv1 wrote: > nit: would personally write a while loop here, but up to you. +1 https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:188: DVLOG(1) << " nal_unit_type " << nalu.nal_unit_type; ditto w/ removing leading space? https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... File media/formats/mp4/avc_unittest.cc (right): https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:29: remove extra blank line https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:64: NOTREACHED() << "Unexpected name: " << name; consider the only clients are this unit test, maybe a CHECK()? https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:98: default: leave out default in favour of being explicit w/ all the enums if we change the enum the compiler will catch this https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:104: return "UnsupportedType"; ditto -- but your call https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:285: EXPECT_TRUE(AVC::IsValidAnnexB(buf)) << "'" <<test_cases[i] << "' failed"; add space past << https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:307: EXPECT_FALSE(AVC::IsValidAnnexB(buf)) << "'" <<test_cases[i] << "' failed"; ditto https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:352: remove extra blank line
https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:31: DVLOG(1) << " nal_size is 0"; On 2014/04/24 17:16:35, scherkus wrote: > nit: remove leading space? Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:60: DVLOG(1) << " nal_size is 0"; On 2014/04/24 17:16:35, scherkus wrote: > ditto Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:97: subsamples_insert_point++; Left code as is based on our offline discussion. I did add the DCHECK you suggested and test code to verify the subsamples code path. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:176: kBeforeFirstVCL, // VCL == nal_unit_types 1-5 On 2014/04/24 17:16:35, scherkus wrote: > nit: two spaces before // comments Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:185: for (bool done = false; !done;) { On 2014/04/24 17:16:35, scherkus wrote: > On 2014/04/24 00:03:58, damienv1 wrote: > > nit: would personally write a while loop here, but up to you. > > +1 Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:188: DVLOG(1) << " nal_unit_type " << nalu.nal_unit_type; On 2014/04/24 17:16:35, scherkus wrote: > ditto w/ removing leading space? Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.h File media/formats/mp4/avc.h (right): https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc.h... media/formats/mp4/avc.h:31: std::vector<SubsampleEntry>* subsamples); On 2014/04/24 00:03:58, damienv1 wrote: > nit: From http://www.chromium.org/developers/coding-style/cpp-dos-and-donts > we could forward declare SubsampleEntry. Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... File media/formats/mp4/avc_unittest.cc (right): https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:29: On 2014/04/24 17:16:35, scherkus wrote: > remove extra blank line Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:64: NOTREACHED() << "Unexpected name: " << name; On 2014/04/24 17:16:35, scherkus wrote: > consider the only clients are this unit test, maybe a CHECK()? Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:98: default: On 2014/04/24 17:16:35, scherkus wrote: > leave out default in favour of being explicit w/ all the enums > > if we change the enum the compiler will catch this Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:104: return "UnsupportedType"; On 2014/04/24 17:16:35, scherkus wrote: > ditto -- but your call Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:285: EXPECT_TRUE(AVC::IsValidAnnexB(buf)) << "'" <<test_cases[i] << "' failed"; On 2014/04/24 17:16:35, scherkus wrote: > add space past << Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:307: EXPECT_FALSE(AVC::IsValidAnnexB(buf)) << "'" <<test_cases[i] << "' failed"; On 2014/04/24 17:16:35, scherkus wrote: > ditto Done. https://codereview.chromium.org/246853005/diff/120001/media/formats/mp4/avc_u... media/formats/mp4/avc_unittest.cc:352: On 2014/04/24 17:16:35, scherkus wrote: > remove extra blank line Done.
lgtm https://codereview.chromium.org/246853005/diff/140001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/140001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:99: config_insert_point - buffer->begin()); I first suggested a DCHECK. But if the stream is malformed, I would in fact rather "return false" if we hit that case (the stream will be corrupted otherwise and may be for a long time, since SPS/PPS might not be decoded properly). What do you think ? https://codereview.chromium.org/246853005/diff/140001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:265: nit: I would remove the blank line here and in every case (more readable to me as one case = one block of code).
https://codereview.chromium.org/246853005/diff/140001/media/formats/mp4/avc.cc File media/formats/mp4/avc.cc (right): https://codereview.chromium.org/246853005/diff/140001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:99: config_insert_point - buffer->begin()); On 2014/04/25 00:11:39, damienv1 wrote: > I first suggested a DCHECK. > But if the stream is malformed, I would in fact rather "return false" if we hit > that case (the stream will be corrupted otherwise and may be for a long time, > since SPS/PPS might not be decoded properly). > What do you think ? Done. https://codereview.chromium.org/246853005/diff/140001/media/formats/mp4/avc.c... media/formats/mp4/avc.cc:265: On 2014/04/25 00:11:39, damienv1 wrote: > nit: I would remove the blank line here and in every case (more readable to me > as one case = one block of code). Done.
The CQ bit was checked by acolwell@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/acolwell@chromium.org/246853005/160001
The CQ bit was unchecked by acolwell@chromium.org
The CQ bit was checked by acolwell@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/acolwell@chromium.org/246853005/180001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on chromium_presubmit
The CQ bit was checked by acolwell@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/acolwell@chromium.org/246853005/180001
The CQ bit was unchecked by acolwell@chromium.org
The CQ bit was checked by acolwell@chromium.org
The CQ bit was unchecked by acolwell@chromium.org
The CQ bit was checked by acolwell@chromium.org
The CQ bit was checked by acolwell@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/acolwell@chromium.org/246853005/200001
Message was sent while issue was closed.
Change committed as 266316 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
