[0296] 'upd video stream cap/r4': refine the PR with the review comments

10 files changed, 73 insertions, 49 deletions

diff --git a/SmartDeviceLink/private/SDLImageResolution + StreamingVideoExtensions.h b/SmartDeviceLink/private/SDLImageResolution + StreamingVideoExtensions.h
index d80991111..ea0936e8a 100644
--- a/SmartDeviceLink/private/SDLImageResolution + StreamingVideoExtensions.h
+++ b/SmartDeviceLink/private/SDLImageResolution + StreamingVideoExtensions.h
@@ -23,6 +23,10 @@ typedef NS_ENUM(NSInteger, SDLImageResolutionKind) {
 */
 @property (nonatomic, readonly) SDLImageResolutionKind kind;
 
+/**
+ It creates and returns a CGSize struct initialized with the resolution width and height
+ @return CGSize struct
+ */
 - (CGSize)makeSize;
 
 // the return value is always equals or above 1.0 if valid, or is 0.0 if size is invalid
diff --git a/SmartDeviceLink/private/SDLStreamingVideoLifecycleManager.m b/SmartDeviceLink/private/SDLStreamingVideoLifecycleManager.m
index 9b6afd8ef..e9e148aca 100644
--- a/SmartDeviceLink/private/SDLStreamingVideoLifecycleManager.m
+++ b/SmartDeviceLink/private/SDLStreamingVideoLifecycleManager.m
@@ -329,17 +329,17 @@ typedef void(^SDLVideoCapabilityResponseHandler)(SDLVideoStreamingCapability *_N
     if (_showVideoBackgroundDisplay) {
         [self sdl_sendBackgroundFrames];
     }
-    [self suspendVideo];
+    [self sdl_suspendVideo];
 }
 
 // Per Apple's guidelines: https://developer.apple.com/library/content/documentation/iPhone/Conceptual/iPhoneOSProgrammingGuide/StrategiesforHandlingAppStateTransitions/StrategiesforHandlingAppStateTransitions.html
 // We should be waiting to start any OpenGL drawing until UIApplicationDidBecomeActive is called.
 - (void)didEnterStateAppActive {
     SDLLogD(@"App became active");
-    [self resumeVideo];
+    [self sdl_resumeVideo];
 }
 
-- (void)resumeVideo {
+- (void)sdl_resumeVideo {
     self.shouldAutoResume = NO;
 
     if (self.protocol == nil) {
@@ -354,7 +354,7 @@ typedef void(^SDLVideoCapabilityResponseHandler)(SDLVideoStreamingCapability *_N
     }
 }
 
-- (void)suspendVideo {
+- (void)sdl_suspendVideo {
     if (!self.protocol) {
         SDLLogV(@"No session established with head unit. Cannot suspend video.");
         return;
@@ -510,7 +510,7 @@ typedef void(^SDLVideoCapabilityResponseHandler)(SDLVideoStreamingCapability *_N
             if (capability) {
                 [self sdl_applyVideoCapability:capability];
             }
-            [self resumeVideo];
+            [self sdl_resumeVideo];
         });
     }
     [[NSNotificationCenter defaultCenter] postNotificationName:SDLVideoStreamSuspendedNotification object:nil];
@@ -720,12 +720,19 @@ typedef void(^SDLVideoCapabilityResponseHandler)(SDLVideoStreamingCapability *_N
     if (videoCapability) {
         [self sdl_useVideoCapability:videoCapability];
     } else {
-        typeof(self) weakSelf = self;
-        [self sdl_requestVideoCapabilities:^(SDLVideoStreamingCapability * _Nullable capability) {
-            SDLLogD(@"Received video capability response");
-            SDLLogV(@"Capability: %@", capability);
-            [weakSelf sdl_useVideoCapability:capability];
-        }];
+        // If no response, assume that the format is H264 RAW and get the screen resolution from the RAI response's display capabilities.
+        SDLVideoStreamingFormat *format = [[SDLVideoStreamingFormat alloc] initWithCodec:SDLVideoStreamingCodecH264 protocol:SDLVideoStreamingProtocolRAW];
+        SDLImageResolution *resolution = [[SDLImageResolution alloc] initWithWidth:(uint16_t)self.videoScaleManager.displayViewportResolution.width height:(uint16_t)self.videoScaleManager.displayViewportResolution.height];
+        self.preferredFormats = @[format];
+        self.preferredResolutions = @[resolution];
+        if (self.focusableItemManager != nil) {
+            self.focusableItemManager.enableHapticDataRequests = NO;
+        }
+        SDLLogD(@"Using generic video capabilites, preferred formats: %@, resolutions: %@, haptics disabled", self.preferredFormats, self.preferredResolutions);
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wnonnull"
+        [self sdl_useVideoCapability:nil];
+#pragma clang diagnostic pop
     }
 }
 
@@ -742,13 +749,13 @@ typedef void(^SDLVideoCapabilityResponseHandler)(SDLVideoStreamingCapability *_N
 }
 
 - (void)sdl_applyVideoCapabilityWhileStarting:(nullable SDLVideoStreamingCapability *)videoCapabilityUpdated {
-    SDLVideoStreamingCapability *videoCapability = (nil == videoCapabilityUpdated) ? [self sdl_defaultVideoCapability] : videoCapabilityUpdated;
+    SDLVideoStreamingCapability *videoCapability = (videoCapabilityUpdated == nil) ? [self sdl_defaultVideoCapability] : videoCapabilityUpdated;
     NSArray<SDLVideoStreamingCapability *> *capabilityMatches = [self matchVideoCapability:videoCapability];
     [self sdl_sendOnAppCapabilityUpdated:capabilityMatches];
 
     if (capabilityMatches.count == 0) {
         // use default video capability if no match found
-        capabilityMatches = @[[self sdl_defaultVideoCapability]];
+        capabilityMatches = (videoCapabilityUpdated == nil) ? @[[self sdl_defaultVideoCapability]] : @[[videoCapabilityUpdated shortCopy]];
     }
 
     SDLVideoStreamingCapability *matchedVideoCapability = capabilityMatches.firstObject;
@@ -845,7 +852,7 @@ typedef void(^SDLVideoCapabilityResponseHandler)(SDLVideoStreamingCapability *_N
         return @[];
     }
 
-    NSArray <SDLVideoStreamingCapability*> *allCapabilities = [videoStreamingCapability allVideoStreamingCapabilitiesPlain];
+    NSArray <SDLVideoStreamingCapability*> *allCapabilities = [videoStreamingCapability allVideoStreamingCapabilities];
     NSMutableArray *matchCapabilities = [NSMutableArray arrayWithCapacity:allCapabilities.count];
     for (SDLVideoStreamingCapability *nextCapability in allCapabilities) {
         SDLImageResolution *imageResolution = [nextCapability makeImageResolution];
@@ -1167,18 +1174,6 @@ typedef void(^SDLVideoCapabilityResponseHandler)(SDLVideoStreamingCapability *_N
     return (self.systemCapabilityManager != nil) ? [self.systemCapabilityManager isCapabilitySupported:SDLSystemCapabilityTypeVideoStreaming] : YES;
 }
 
-- (void)setPreferredResolutions:(NSArray<SDLImageResolution *> *)resolutions {
-    _preferredResolutions = resolutions;
-    _preferredResolutionIndex = 0;
-    [self didChangeValueForKey:@"preferredResolutions"];
-}
-
-- (void)setPreferredFormats:(NSArray<SDLVideoStreamingFormat *> *)formats {
-    _preferredFormats = formats;
-    _preferredFormatIndex = 0;
-    [self didChangeValueForKey:@"preferredFormats"];
-}
-
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.h b/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.h
index 6bce7fb0f..e508554fc 100644
--- a/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.h
+++ b/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.h
@@ -14,16 +14,33 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface SDLVideoStreamingCapability (StreamingVideoExtensions)
 
-// this returns a copy array of all capabilities including itself but with no recursion
-// in the result objects the .additionalVideoStreamingCapabilities will be nil
-- (NSArray <SDLVideoStreamingCapability *> *)allVideoStreamingCapabilitiesPlain;
-
+// this returns a copy array of all capabilities including itself
+// in the result objects the .additionalVideoStreamingCapabilities will be ommited
+- (NSArray <SDLVideoStreamingCapability *> *)allVideoStreamingCapabilities;
+
+/**
+ Create a SDLImageResolution from  preferredResolution respecting the scale factor
+ @return SDLImageResolution - an initialized image resolution struct
+ */
 - (SDLImageResolution *)makeImageResolution;
 
+/**
+ It traverces through all capabilities and collects all image resolutions into an array respecting the scale factor
+ @return [SDLImageResolution] - an array of initialized image resolution structs
+ */
 - (NSArray<SDLImageResolution *> *)allImageResolutionsScaled;
 
+/**
+ It traverces through all capabilities and collects all image resolutions into an array ignoring the scale factor
+ @return [SDLImageResolution] - an array of initialized image resolution structs
+ */
 - (NSArray<SDLImageResolution *> *)allImageResolutions;
 
+/**
+ It produces a shallow copy of itself object, additionalVideoStreamingCapabilities will be nil
+ */
+- (instancetype)shortCopy;
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.m b/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.m
index 65d396aa7..66bae1338 100644
--- a/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.m
+++ b/SmartDeviceLink/private/SDLVideoStreamingCapability + StreamingVideoExtensions.m
@@ -32,10 +32,10 @@ NS_ASSUME_NONNULL_BEGIN
     return aCopy;
 }
 
-- (NSArray <SDLVideoStreamingCapability *> *)allVideoStreamingCapabilitiesPlain {
+- (NSArray <SDLVideoStreamingCapability *> *)allVideoStreamingCapabilities {
     NSMutableArray *capabilitiesArray = [NSMutableArray arrayWithObject:[self shortCopy]];
     for (SDLVideoStreamingCapability *capability in self.additionalVideoStreamingCapabilities) {
-        NSArray *childCapabilities = [capability allVideoStreamingCapabilitiesPlain];
+        NSArray *childCapabilities = [capability allVideoStreamingCapabilities];
         if (childCapabilities.count) {
             [capabilitiesArray addObjectsFromArray:childCapabilities];
         }
diff --git a/SmartDeviceLink/public/SDLImageResolution.h b/SmartDeviceLink/public/SDLImageResolution.h
index 16f30e0ec..85000b3b3 100644
--- a/SmartDeviceLink/public/SDLImageResolution.h
+++ b/SmartDeviceLink/public/SDLImageResolution.h
@@ -1,7 +1,6 @@
 //  SDLImageResolution.h
 //
 
-#import <CoreGraphics/CGGeometry.h>
 #import "SDLRPCMessage.h"
 
 NS_ASSUME_NONNULL_BEGIN
diff --git a/SmartDeviceLink/public/SDLStreamingMediaConfiguration.h b/SmartDeviceLink/public/SDLStreamingMediaConfiguration.h
index 5a4b7a3a2..b964d7894 100644
--- a/SmartDeviceLink/public/SDLStreamingMediaConfiguration.h
+++ b/SmartDeviceLink/public/SDLStreamingMediaConfiguration.h
@@ -83,8 +83,19 @@ typedef NS_ENUM(NSUInteger, SDLCarWindowRenderingType) {
  */
 @property (assign, nonatomic) BOOL allowMultipleViewControllerOrientations;
 
+/**
+ A configuration object of type SDLVideoStreamingRange. If the application supports a specific video parameters in landscape orientation like screen size etc then please provide the parameters in this struct.
+ */
 @property (strong, nonatomic, nullable) SDLVideoStreamingRange *supportedLandscapeStreamingRange;
+
+/**
+ A configuration object of type SDLVideoStreamingRange. If the application supports a specific video parameters in portrait orientation like screen size etc then please provide the parameters in this struct.
+ */
 @property (strong, nonatomic, nullable) SDLVideoStreamingRange *supportedPortraitStreamingRange;
+
+/**
+ The configuration delegate, this is an object conforming to the SDLStreamingVideoDelegate protocol. If video streaming parameters change then this object will be called on.
+ */
 @property (weak, nonatomic, nullable) id<SDLStreamingVideoDelegate> delegate;
 
 /**
diff --git a/SmartDeviceLink/public/SDLSystemCapabilityManager.m b/SmartDeviceLink/public/SDLSystemCapabilityManager.m
index bd61a2e05..ff63f7dde 100644
--- a/SmartDeviceLink/public/SDLSystemCapabilityManager.m
+++ b/SmartDeviceLink/public/SDLSystemCapabilityManager.m
@@ -383,15 +383,13 @@ typedef NSString * SDLServiceID;
     [self.connectionManager sendConnectionRequest:getSystemCapability withResponseHandler:^(__kindof SDLRPCRequest * _Nullable request, __kindof SDLRPCResponse * _Nullable response, NSError * _Nullable error) {
         if (![response isKindOfClass:[SDLGetSystemCapabilityResponse class]]) {
             SDLLogE(@"GetSystemCapability failed, type: %@, did not return a GetSystemCapability response", type);
-            if (handler == nil) { return; }
-            handler(nil, NO, [NSError sdl_systemCapabilityManager_moduleDoesNotSupportSystemCapabilities]);
+            [self sdl_notifyObserversOfCapabilityType:getSystemCapability.systemCapabilityType capability:nil error:[NSError sdl_systemCapabilityManager_moduleDoesNotSupportSystemCapabilities]];
             return;
         }
 
         if (response.success.boolValue == false) {
             SDLLogE(@"GetSystemCapability failed, type: %@, error: %@", type, error);
-            if (handler == nil) { return; }
-            handler(nil, NO, error);
+            [self sdl_notifyObserversOfCapabilityType:getSystemCapability.systemCapabilityType capability:nil error:error];
             return;
         }
 
@@ -408,6 +406,12 @@ typedef NSString * SDLServiceID;
     }];
 }
 
+- (void)sdl_notifyObserversOfCapabilityType:(SDLSystemCapabilityType)type capability:(nullable SDLSystemCapability *)capability error:(nullable NSError *)error {
+    for (SDLSystemCapabilityObserver *observer in self.capabilityObservers[type]) {
+        [self sdl_invokeObserver:observer withCapabilityType:type capability:capability error:error];
+    }
+}
+
 #pragma mark Saving Capability Responses
 
 /**
@@ -662,10 +666,7 @@ typedef NSString * SDLServiceID;
     SDLLogV(@"Calling observers for type: %@ with update: %@", type, capability);
 
     [self sdl_removeNilObserversAndUnsubscribeIfNecessary];
-
-    for (SDLSystemCapabilityObserver *observer in self.capabilityObservers[type]) {
-        [self sdl_invokeObserver:observer withCapabilityType:type capability:capability error:error];
-    }
+    [self sdl_notifyObserversOfCapabilityType:type capability:capability error:error];
 
     if (handler == nil) { return; }
     handler(capability, self.subscriptionStatus[type].boolValue, error);
diff --git a/SmartDeviceLink/public/SmartDeviceLink.h b/SmartDeviceLink/public/SmartDeviceLink.h
index d1a5fcf71..712580dda 100644
--- a/SmartDeviceLink/public/SmartDeviceLink.h
+++ b/SmartDeviceLink/public/SmartDeviceLink.h
@@ -151,7 +151,6 @@ FOUNDATION_EXPORT const unsigned char SmartDeviceLinkVersionString[];
 #import "SDLUnsubscribeWayPointsResponse.h"
 #import "SDLUpdateTurnListResponse.h"
 
-// Notifications
 #pragma mark Notifications
 #import "SDLOnAppCapabilityUpdated.h"
 #import "SDLOnAppInterfaceUnregistered.h"
diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m
index d5492c4c6..3874066b2 100644
--- a/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m
+++ b/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m
@@ -71,7 +71,6 @@
 @end
 
 @implementation SDLLifecycleTestManager
-// override parent method
 - (void)sendConnectionManagerRequest:(__kindof SDLRPCMessage *)request withResponseHandler:(nullable SDLResponseHandler)handler {
     self.testRequest = request;
 }
@@ -102,7 +101,6 @@ QuickConfigurationEnd
 QuickSpecBegin(SDLLifecycleManagerSpec)
 
 describe(@"test lifecycle manager internals", ^{
-    // note: this test was created to satisfy Cocodev
     context(@"init and assign version", ^{
         SDLLifecycleTestManager *manager = [[SDLLifecycleTestManager alloc] init];
         it(@"expect object to be created", ^{
diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLStreamingVideoLifecycleManagerSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLStreamingVideoLifecycleManagerSpec.m
index 62cd27015..ea97a86b2 100644
--- a/SmartDeviceLinkTests/DevAPISpecs/SDLStreamingVideoLifecycleManagerSpec.m
+++ b/SmartDeviceLinkTests/DevAPISpecs/SDLStreamingVideoLifecycleManagerSpec.m
@@ -66,7 +66,7 @@
 
 - (void)shutDown;
 - (NSArray<SDLVideoStreamingCapability *>* __nullable)matchVideoCapability:(SDLVideoStreamingCapability *)videoStreamingCapability;
-- (void)suspendVideo;
+- (void)sdl_suspendVideo;
 - (void)didEnterStateVideoStreamStopped;
 - (void)didEnterStateVideoStreamStarting;
 - (void)didEnterStateVideoStreamReady;
@@ -120,7 +120,7 @@
 
 // expose private methods to the test suite
 @interface SDLVideoStreamingCapability (test)
-- (NSArray <SDLVideoStreamingCapability *> *)allVideoStreamingCapabilitiesPlain;
+- (NSArray <SDLVideoStreamingCapability *> *)allVideoStreamingCapabilities;
 - (instancetype)shortCopy;
 @end
 
@@ -246,10 +246,10 @@ describe(@"test internals", ^{
         SDLProtocol *protocolMock = OCMClassMock([SDLProtocol class]);
         it(@"suspendVideo with and without a protocol", ^{
             expect(streamingLifecycleManager.didStopVideoSession).to(equal(NO));
-            [streamingLifecycleManager suspendVideo];
+            [streamingLifecycleManager sdl_suspendVideo];
             expect(streamingLifecycleManager.didStopVideoSession).to(equal(NO));
             streamingLifecycleManager.protocol = protocolMock;
-            [streamingLifecycleManager suspendVideo];
+            [streamingLifecycleManager sdl_suspendVideo];
             expect(streamingLifecycleManager.didStopVideoSession).to(equal(YES));
         });
     });
@@ -1396,7 +1396,7 @@ describe(@"supported video capabilities and formats", ^{
     SDLStreamingVideoLifecycleManager *streamingLifecycleManager = [[SDLStreamingVideoLifecycleManager alloc] initWithConnectionManager:testConnectionManager configuration:testConfig systemCapabilityManager:nil];
 
     context(@"neither landscape nor portrait constraint set", ^{
-        NSArray <SDLVideoStreamingCapability*>* allCapabilities = [capability0 allVideoStreamingCapabilitiesPlain];
+        NSArray <SDLVideoStreamingCapability*>* allCapabilities = [capability0 allVideoStreamingCapabilities];
 
         it(@"should let all capabilities in (nothing filtered out)", ^{
             streamingLifecycleManager.supportedLandscapeStreamingRange = nil;