-
Notifications
You must be signed in to change notification settings - Fork 395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow image scaling on the mjepg stream #138
Allow image scaling on the mjepg stream #138
Conversation
It would be nice to have an integration test for the image scaler class |
@@ -0,0 +1,80 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
It looks like CI is not very happy |
It's a bit happier now. I used Xcode 10 and it was fine with it, but Xcode 9 was complaining... |
} | ||
NSData *scaled = [self scaledImageWithImage:next | ||
scalingFactor:scalingFactor | ||
compressionQuality:compressionQuality]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to recompress the original image after it has been already compressed by XCTest? Wouldn't this be a waste of resources?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, otherwise the size keeps roughly the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the additional setting for compression quality and now we use the same value for screenshot taking and when we create the jpeg after scaling
Everything looks fine to me. I only have one question: does it make sense to apply the compression twice: once while taking the screenshot and the second time while performing scaling? I would assume we could simply set the compression factor to 100% for the first pass and the requested quality to the second one or vice versa |
Setting the |
@@ -23,6 +23,8 @@ | |||
static NSString* const ELEMENT_RESPONSE_ATTRIBUTES = @"elementResponseAttributes"; | |||
static NSString* const MJPEG_SERVER_SCREENSHOT_QUALITY = @"mjpegServerScreenshotQuality"; | |||
static NSString* const MJPEG_SERVER_FRAMERATE = @"mjpegServerFramerate"; | |||
static NSString* const MJPEG_SCALING_FACTOR = @"mjpegScalingFactor"; | |||
static NSString* const MJPEG_COMPRESSION_FACTOR = @"mjpegCompressionFactor"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this setting value in https://github.com/appium/appium/blob/f60b12cae069de3a306b26da622cee0dbbee7d7e/docs/en/advanced-concepts/settings.md#xcuitest after merging this?
Currently we get full scale screenshots on the mjpeg stream. To reduce bandwidth or to make processing the screenshots on the host more efficient we could scale down images already on the device.