Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2526813002:
Add unit tests for avfoundation format mapper functions and fix wrong implementation. (Closed)
|
Created:
4 years ago by daniela-webrtc Modified:
4 years ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, qiang.lu, niklas.enbom, yujie_mao (webrtc), peah-webrtc Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd unit tests for avfoundation format mapper functions.
The CL fixes adds tests that fully test the functions that manipulate the cricket::VideoFormat<->AVCaptureDeviceFormat
relation.
BUG=webrtc:6680
Committed: https://crrev.com/28c2487c8551369f47b89b9b29cc60999498f783
Cr-Commit-Position: refs/heads/master@{#15444}
Patch Set 1 #
Total comments: 11
Patch Set 2 : Address review comments #
Total comments: 18
Patch Set 3 : Remove static methods exposure #
Total comments: 4
Patch Set 4 : Remove test fixture and simplify tests #Patch Set 5 : Replace static method with static var #Patch Set 6 : Rename test #
Total comments: 1
Patch Set 7 : Replace #include <vector> with <set> #
Total comments: 12
Patch Set 8 : Remove explicit ivar declaration #
Messages
Total messages: 24 (8 generated)
denicija@webrtc.org changed reviewers: + kthelgason@webrtc.org, magjed@webrtc.org
Please take a look when you have time
https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Cla... File webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm (right): https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Cla... webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm:25: return range.minFrameRate <= fps && range.maxFrameRate <= fps; Is this really a fix? The previous implementation looks correct if you indeed want to test if fps is within the range. https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Cla... webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm:28: namespace webrtc { Why have you moved the namespace up here? Functions that are not in the header should not be exposed in the compilation unit. Either have implementation helper functions in an anonymous namespace, or mark them as static as it was done previously. https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:20: // Widht and height don't play any role so lets use predefined values troughout nit: Spelling: Width and throughout https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:25: // Hardcoded framrate to be used troughout the tests. nit: spelling: throughout https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:29: NSArray<AVCaptureDeviceFormat*>* GetEligibleDeviceFormats( Are you testing some implementation helper function here? This function is not in the header file. Why not just test the public functions?
https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Cla... File webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm (right): https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Cla... webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm:25: return range.minFrameRate <= fps && range.maxFrameRate <= fps; On 2016/11/24 13:42:59, magjed_webrtc wrote: > Is this really a fix? The previous implementation looks correct if you indeed > want to test if fps is within the range. Ah you are right! I got i mixed up. Basically we want formats that have the specified fps (in the actual code 30) within their range. I got confused and tested on formats that have maxFrameRate lower than 30. Great catch! Fixing it right away https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Cla... webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm:28: namespace webrtc { On 2016/11/24 13:42:59, magjed_webrtc wrote: > Why have you moved the namespace up here? Functions that are not in the header > should not be exposed in the compilation unit. Either have implementation helper > functions in an anonymous namespace, or mark them as static as it was done > previously. Done. https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:20: // Widht and height don't play any role so lets use predefined values troughout On 2016/11/24 13:42:59, magjed_webrtc wrote: > nit: Spelling: Width and throughout Done. https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:25: // Hardcoded framrate to be used troughout the tests. On 2016/11/24 13:42:59, magjed_webrtc wrote: > nit: spelling: throughout Done. https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:29: NSArray<AVCaptureDeviceFormat*>* GetEligibleDeviceFormats( On 2016/11/24 13:42:59, magjed_webrtc wrote: > Are you testing some implementation helper function here? This function is not > in the header file. Why not just test the public functions? Yes it's an implementation helper function. I wanted to test it in isolation as it does most of the heavy lifting when filtering device formats.
https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/1/webrtc/sdk/objc/Framework/Uni... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:29: NSArray<AVCaptureDeviceFormat*>* GetEligibleDeviceFormats( On 2016/11/25 08:58:15, denicija-webrtc wrote: > On 2016/11/24 13:42:59, magjed_webrtc wrote: > > Are you testing some implementation helper function here? This function is not > > in the header file. Why not just test the public functions? > > Yes it's an implementation helper function. I wanted to test it in isolation as > it does most of the heavy lifting when filtering device formats. I see. I'm sorry, but you have to stick to testing the public functions. In some cases, private functions or methods are so huge and complex that they require their own tests, and in those cases you have to make them public or break out the logic to a new class and make it public in order to test it. I don't think that's the case with these functions, and it should be fine to just test GetSupportedVideoFormatsForDevice and SetFormatForCaptureDevice. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm (right): https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.mm:30: NSArray<AVCaptureDeviceFormat*>* GetEligibleDeviceFormats( You are not allowed to expose implementation details from .cc or .mm files. Every exposed function needs to be present in the header file. I understand you want to test these functions directly, but I discourage you from doing that since they are implementation details. Test only the public functions GetSupportedVideoFormatsForDevice and SetFormatForCaptureDevice. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:57: NSNumber *subtype = [mockDataDictionary objectForKey:kMediaSubtypeKey]; What's the benefit of sending these arguments in an NSDictionary compared to using explicit arguments? https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:98: void testEligibleWithValidFormats() { It looks like you can make the unittests more simple if you only add one format in every test. You can have one test with multiple formats to exercise that logic, but the other unittests should be as simple as possible. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:122: EXPECT_TRUE(result.count == 2); EXPECT_EQ(2, result.count); Same for other places. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:208: EXPECT_EQ(result.size(), 1); You need to flip the order of the arguments, i.e. EXPECT_EQ(1, result.size()). https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:212: EXPECT_TRUE(it != result.end()); Do this instead: EXPECT_EQ(1u, result.count(expectedFormat)); https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:246: EXPECT_TRUE([resultFormat isEqual:mockTwo]); So EXPECT_EQ doesn't work here? https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:290: testEligibleWithValidFormats(); Inline these functions instead, unless you have some good ObjC reason to do like this.
https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:57: NSNumber *subtype = [mockDataDictionary objectForKey:kMediaSubtypeKey]; On 2016/11/28 12:21:23, magjed_webrtc wrote: > What's the benefit of sending these arguments in an NSDictionary compared to > using explicit arguments? Done. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:98: void testEligibleWithValidFormats() { On 2016/11/28 12:21:23, magjed_webrtc wrote: > It looks like you can make the unittests more simple if you only add one format > in every test. You can have one test with multiple formats to exercise that > logic, but the other unittests should be as simple as possible. Done. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:122: EXPECT_TRUE(result.count == 2); On 2016/11/28 12:21:23, magjed_webrtc wrote: > EXPECT_EQ(2, result.count); Same for other places. Done. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:208: EXPECT_EQ(result.size(), 1); On 2016/11/28 12:21:23, magjed_webrtc wrote: > You need to flip the order of the arguments, i.e. EXPECT_EQ(1, result.size()). Done. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:212: EXPECT_TRUE(it != result.end()); On 2016/11/28 12:21:23, magjed_webrtc wrote: > Do this instead: EXPECT_EQ(1u, result.count(expectedFormat)); Done. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:246: EXPECT_TRUE([resultFormat isEqual:mockTwo]); On 2016/11/28 12:21:23, magjed_webrtc wrote: > So EXPECT_EQ doesn't work here? Done.
https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:98: void testEligibleWithValidFormats() { On 2016/11/29 12:05:55, denicija-webrtc wrote: > On 2016/11/28 12:21:23, magjed_webrtc wrote: > > It looks like you can make the unittests more simple if you only add one > format > > in every test. You can have one test with multiple formats to exercise that > > logic, but the other unittests should be as simple as possible. > > Done. It's not done. What I mean is that if we want to unittest that invalidFpsFormat is rejected, we should only add that format and not a valid format as well. I.e. I want the test to look like this: AVCaptureDeviceFormatMock* mockFormat = [AVCaptureDeviceFormatMock invalidFpsFormat]; [[[mockDevice stub] andReturn:@[ mockFormat ]] formats]; std::set<cricket::VideoFormat> result = webrtc::GetSupportedVideoFormatsForDevice(mockDevice); EXPECT_TRUE(result.empty()); Same for testSuportedCricketFormatsWithInvalidMediaSubtypeFormats. We can have a separate test with multiple formats, e.g. testMultipleFormats where you add more than one format to the mock device. https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:212: EXPECT_TRUE(it != result.end()); On 2016/11/29 12:05:55, denicija-webrtc wrote: > On 2016/11/28 12:21:23, magjed_webrtc wrote: > > Do this instead: EXPECT_EQ(1u, result.count(expectedFormat)); > > Done. Actually, since you only have one element, it might be better check that the only element is equal to the expected format: EXPECT_EQ(expectedFormat, *result.begin()); https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:290: testEligibleWithValidFormats(); On 2016/11/28 12:21:23, magjed_webrtc wrote: > Inline these functions instead, unless you have some good ObjC reason to do like > this. I still want this done. You don't need the fixture class AVFormatMapperTest at all since your tests are so simple. So remove the class AVFormatMapperTest, rename TEST_F to TEST, and inline the test function. I.e.: TEST(AVFormatMapperTest, EligibleFormatsPerDeviceWithValidFormats) { // given id mockDevice = [OCMockObject mockForClass:[AVCaptureDevice class]]; ... } https://codereview.webrtc.org/2526813002/diff/40001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:25: static cricket::VideoFormat SupportedCricketVideoFormat() { It should be possible to just have a constant for this: const cricket::VideoFormat kSupportedCricketVideoFormat( kFormatWidth, kFormatHeight, cricket::VideoFormat::FpsToInterval(kFramerate), cricket::FOURCC_NV12); https://codereview.webrtc.org/2526813002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:184: void testAVFormatForCricketFormat() { nit: I would like to rename this test to something like testMediaSubtypePreference.
https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:290: testEligibleWithValidFormats(); On 2016/11/29 13:34:01, magjed_webrtc wrote: > On 2016/11/28 12:21:23, magjed_webrtc wrote: > > Inline these functions instead, unless you have some good ObjC reason to do > like > > this. > > I still want this done. You don't need the fixture class AVFormatMapperTest at > all since your tests are so simple. So remove the class AVFormatMapperTest, > rename TEST_F to TEST, and inline the test function. I.e.: > > TEST(AVFormatMapperTest, EligibleFormatsPerDeviceWithValidFormats) { > // given > id mockDevice = [OCMockObject mockForClass:[AVCaptureDevice class]]; > ... > } Sorry, I missed this one :/ It's changed now https://codereview.webrtc.org/2526813002/diff/40001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:25: static cricket::VideoFormat SupportedCricketVideoFormat() { On 2016/11/29 13:34:01, magjed_webrtc wrote: > It should be possible to just have a constant for this: > const cricket::VideoFormat kSupportedCricketVideoFormat( > kFormatWidth, kFormatHeight, > cricket::VideoFormat::FpsToInterval(kFramerate), > cricket::FOURCC_NV12); Done. https://codereview.webrtc.org/2526813002/diff/40001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:184: void testAVFormatForCricketFormat() { On 2016/11/29 13:34:01, magjed_webrtc wrote: > nit: I would like to rename this test to something like > testMediaSubtypePreference. Done.
lgtm % nit https://codereview.webrtc.org/2526813002/diff/100001/webrtc/sdk/objc/Framewor... File webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.h (right): https://codereview.webrtc.org/2526813002/diff/100001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/Classes/avfoundationformatmapper.h:13: #include <vector> nit: You should include set instead. Also, place standard library includes before any other include, like this: #include <set> #import <AVFoundation/AVFoundation.h> ...
lgtm % the include that magjed@ mentioned.
denicija@webrtc.org changed reviewers: + tkchin@webrtc.org
@zeke mind taking a look when you have time?
a few comments, but lgtm https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:13: #include "avfoundationformatmapper.h" nit: include frameworks first, and then only include the source headers https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:33: // CMVideoDescriptionRef mocking. ooc why not? It should just be a pointer type and it should be possible to make it return the format description. But it's probably better the way you have it anyway because we need to release it later. https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:35: CMVideoFormatDescriptionRef _format; nit: declare these in the @implementation, or otherwise expose as properties if you need them https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:58: [[[_rangeMock stub] andReturnValue:@(minFps)] minFrameRate]; This is old OCMock format. Use OCMock3 if possible, it's much easier to read. https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:91: [_rangeMock dealloc]; I don't recall these needing explicit dealloc. They should be ARC-counted as well. Is that not true? https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:112: TEST(AVFormatMapperTest, SuportedCricketFormatsWithInvalidFramerateFormats) { xctest still not possible?
Also, update the CL title and description before landing, i.e. remove the 'fix wrong implementation' parts.
https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... File webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm (right): https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:13: #include "avfoundationformatmapper.h" On 2016/12/02 21:12:57, tkchin_webrtc wrote: > nit: include frameworks first, and then only include the source headers Done. https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:33: // CMVideoDescriptionRef mocking. On 2016/12/02 21:12:57, tkchin_webrtc wrote: > ooc why not? It should just be a pointer type and it should be possible to make > it return the format description. But it's probably better the way you have it > anyway because we need to release it later. Yes, proper release can be done this way and that's one of the reasons. Also, OCMock can only return NSObject classes or predefined set of simple types (ints, floats, bool) when stubbing methods. That's why I couldn't mock the AVCaptureDeviceFormat class and return the format description via stub https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:35: CMVideoFormatDescriptionRef _format; On 2016/12/02 21:12:57, tkchin_webrtc wrote: > nit: declare these in the @implementation, or otherwise expose as properties if > you need them Done. https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:58: [[[_rangeMock stub] andReturnValue:@(minFps)] minFrameRate]; On 2016/12/02 21:12:57, tkchin_webrtc wrote: > This is old OCMock format. Use OCMock3 if possible, it's much easier to read. The OCMock dependency is not updated to OCMock 3 yet :/ https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:91: [_rangeMock dealloc]; On 2016/12/02 21:12:57, tkchin_webrtc wrote: > I don't recall these needing explicit dealloc. They should be ARC-counted as > well. Is that not true? You are right. This is not needed. https://codereview.webrtc.org/2526813002/diff/120001/webrtc/sdk/objc/Framewor... webrtc/sdk/objc/Framework/UnitTests/avformatmappertests.mm:112: TEST(AVFormatMapperTest, SuportedCricketFormatsWithInvalidFramerateFormats) { On 2016/12/02 21:12:57, tkchin_webrtc wrote: > xctest still not possible? Not yet :(
Description was changed from ========== Add unit tests for avfoundation format mapper functions and fix wrong implementation. The tests discovered flaw in the intial implementation. This CL fixes that and adds tests that fully test the functions that manipulate the cricket::VideoFormat<->AVCaptureDeviceFormat relation. BUG=webrtc:6680 ========== to ========== Add unit tests for avfoundation format mapper functions. The CL fixes adds tests that fully test the functions that manipulate the cricket::VideoFormat<->AVCaptureDeviceFormat relation. BUG=webrtc:6680 ==========
The CQ bit was checked by denicija@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org, kthelgason@webrtc.org, tkchin@webrtc.org Link to the patchset: https://codereview.webrtc.org/2526813002/#ps140001 (title: "Remove explicit ivar declaration")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1481029344097090,
"parent_rev": "533e2b2b84a698ad50f85b811ea59df278bea4f1", "commit_rev":
"274880fd0cc92f254c8e4c255afabcc5309568b0"}
Message was sent while issue was closed.
Description was changed from ========== Add unit tests for avfoundation format mapper functions. The CL fixes adds tests that fully test the functions that manipulate the cricket::VideoFormat<->AVCaptureDeviceFormat relation. BUG=webrtc:6680 ========== to ========== Add unit tests for avfoundation format mapper functions. The CL fixes adds tests that fully test the functions that manipulate the cricket::VideoFormat<->AVCaptureDeviceFormat relation. BUG=webrtc:6680 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Add unit tests for avfoundation format mapper functions. The CL fixes adds tests that fully test the functions that manipulate the cricket::VideoFormat<->AVCaptureDeviceFormat relation. BUG=webrtc:6680 ========== to ========== Add unit tests for avfoundation format mapper functions. The CL fixes adds tests that fully test the functions that manipulate the cricket::VideoFormat<->AVCaptureDeviceFormat relation. BUG=webrtc:6680 Committed: https://crrev.com/28c2487c8551369f47b89b9b29cc60999498f783 Cr-Commit-Position: refs/heads/master@{#15444} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/28c2487c8551369f47b89b9b29cc60999498f783 Cr-Commit-Position: refs/heads/master@{#15444} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
