Skip to content
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

refactor: Optimize screenshots preprocessing #812

Merged
merged 25 commits into from
Nov 16, 2023
Merged

refactor: Optimize screenshots preprocessing #812

merged 25 commits into from
Nov 16, 2023

Conversation

mykola-mokhnach
Copy link

I can observe that videos generated from MJPEG streams supplied by WDA have rotation issues if device orientation is changed dynamically. No video issues are observed if screenshots orientation is adjusted properly according to the current implementation.

@mykola-mokhnach
Copy link
Author

mykola-mokhnach commented Nov 12, 2023

@KazuCocoa Could you please test it with real iPad?

@Dan-Maor
Copy link
Collaborator

Will run a verification on my test devices tomorrow once I get to the office.

@KazuCocoa
Copy link
Member

KazuCocoa commented Nov 12, 2023

Checked glance over a browser to http://<device address>:9100/. Then, the screens showed a grey screen with errors below a couple of times after 10 sec or so. Then, the xcodebuild stopped even when I kept the same orientation at the beginning.

2023-11-12 09:52:20.552489-0800 WebDriverAgentRunner-Runner[24432:3091682] IOSurface creation failed: e00002bd parentID: 00000000 properties: {
    IOSurfaceAllocSize = 12582912;
    IOSurfaceBytesPerElement = 4;
    IOSurfaceBytesPerRow = 6144;
    IOSurfaceCacheMode = 0;
    IOSurfaceHeight = 2048;
    IOSurfaceMapCacheAttribute = 1;
    IOSurfaceName = CMPhoto;
    IOSurfaceOffset = 0;
    IOSurfacePixelFormat = 1111970369;
    IOSurfacePreallocPages = 0;
    IOSurfaceWidth = 1536;
}
2023-11-12 09:52:20.553251-0800 WebDriverAgentRunner-Runner[24432:3091682] IOSurface creation failed: e00002bd parentID: 00000000 properties: {
    IOSurfaceAllocSize = 12582912;
    IOSurfaceBytesPerElement = 4;
    IOSurfaceBytesPerRow = 6144;
    IOSurfaceCacheMode = 0;
    IOSurfaceHeight = 2048;
    IOSurfaceMapCacheAttribute = 1;
    IOSurfaceName = CMPhoto;
    IOSurfaceOffset = 0;
    IOSurfacePixelFormat = 1111970369;
    IOSurfacePreallocPages = 0;
    IOSurfaceWidth = 1536;
}
2023-11-12 09:52:20.553654-0800 WebDriverAgentRunner-Runner[24432:3091682] [WebDriverAgentRunner-Runner] callDecodeImage:2006: *** ERROR: decodeImageImp failed - NULL _blockArray
2023-11-12 09:52:20.553884-0800 WebDriverAgentRunner-Runner[24432:3091682] [WebDriverAgentRunner-Runner] callDecodeImage:2006: *** ERROR: decodeImageImp failed - NULL _blockArray

The current master did not have the error.

Just a note:
Btw, my env worked orientation via the video stream by following the device orientation with current master. I haven't checked the implementation details well (below log is an example of when I changed the device orientation with the current master).

2023-11-12 09:54:14.007873-0800 WebDriverAgentRunner-Runner[24465:3094189] Got screenshots broadcast client connection at 192.168.4.248:55258
2023-11-12 09:54:14.008612-0800 WebDriverAgentRunner-Runner[24465:3094189] Starting screenshots broadcast for the client at 192.168.4.248:55257
2023-11-12 09:54:14.012400-0800 WebDriverAgentRunner-Runner[24465:3094189] Using singleton test manager
2023-11-12 09:54:16.352697-0800 WebDriverAgentRunner-Runner[24465:3094189] Disconnected a client from screenshots broadcast
    t =   220.14s Device orientation changed to Portrait Upside Down
    t =   221.99s Interface orientation changed to Portrait Upside Down
    t =   221.99s Device orientation changed to Landscape Left
    t =   229.26s Interface orientation changed to Landscape Right
    t =   229.26s Device orientation changed to Portrait Upside Down
    t =   231.66s Interface orientation changed to Portrait Upside Down
    t =   231.67s Device orientation changed to Landscape Left

I have a plan today, so my next look will be late

@mykola-mokhnach
Copy link
Author

Checked glance over a browser to http://<device address>:9100/. Then, the screens showed a grey screen with errors below a couple of times after 10 sec or so. Then, the xcodebuild stopped even when I kept the same orientation at the beginning.

2023-11-12 09:52:20.552489-0800 WebDriverAgentRunner-Runner[24432:3091682] IOSurface creation failed: e00002bd parentID: 00000000 properties: {
    IOSurfaceAllocSize = 12582912;
    IOSurfaceBytesPerElement = 4;
    IOSurfaceBytesPerRow = 6144;
    IOSurfaceCacheMode = 0;
    IOSurfaceHeight = 2048;
    IOSurfaceMapCacheAttribute = 1;
    IOSurfaceName = CMPhoto;
    IOSurfaceOffset = 0;
    IOSurfacePixelFormat = 1111970369;
    IOSurfacePreallocPages = 0;
    IOSurfaceWidth = 1536;
}
2023-11-12 09:52:20.553251-0800 WebDriverAgentRunner-Runner[24432:3091682] IOSurface creation failed: e00002bd parentID: 00000000 properties: {
    IOSurfaceAllocSize = 12582912;
    IOSurfaceBytesPerElement = 4;
    IOSurfaceBytesPerRow = 6144;
    IOSurfaceCacheMode = 0;
    IOSurfaceHeight = 2048;
    IOSurfaceMapCacheAttribute = 1;
    IOSurfaceName = CMPhoto;
    IOSurfaceOffset = 0;
    IOSurfacePixelFormat = 1111970369;
    IOSurfacePreallocPages = 0;
    IOSurfaceWidth = 1536;
}
2023-11-12 09:52:20.553654-0800 WebDriverAgentRunner-Runner[24432:3091682] [WebDriverAgentRunner-Runner] callDecodeImage:2006: *** ERROR: decodeImageImp failed - NULL _blockArray
2023-11-12 09:52:20.553884-0800 WebDriverAgentRunner-Runner[24432:3091682] [WebDriverAgentRunner-Runner] callDecodeImage:2006: *** ERROR: decodeImageImp failed - NULL _blockArray

The current master did not have the error.

Just a note: Btw, my env worked orientation via the video stream by following the device orientation with current master. I haven't checked the implementation details well (below log is an example of when I changed the device orientation with the current master).

2023-11-12 09:54:14.007873-0800 WebDriverAgentRunner-Runner[24465:3094189] Got screenshots broadcast client connection at 192.168.4.248:55258
2023-11-12 09:54:14.008612-0800 WebDriverAgentRunner-Runner[24465:3094189] Starting screenshots broadcast for the client at 192.168.4.248:55257
2023-11-12 09:54:14.012400-0800 WebDriverAgentRunner-Runner[24465:3094189] Using singleton test manager
2023-11-12 09:54:16.352697-0800 WebDriverAgentRunner-Runner[24465:3094189] Disconnected a client from screenshots broadcast
    t =   220.14s Device orientation changed to Portrait Upside Down
    t =   221.99s Interface orientation changed to Portrait Upside Down
    t =   221.99s Device orientation changed to Landscape Left
    t =   229.26s Interface orientation changed to Landscape Right
    t =   229.26s Device orientation changed to Portrait Upside Down
    t =   231.66s Interface orientation changed to Portrait Upside Down
    t =   231.67s Device orientation changed to Landscape Left

I have a plan today, so my next look will be late

thanks for checking it @KazuCocoa

I did some optimisations. How do you record your video? I tried GStreamer with videoflip pipeline method set to automatic. It only properly records the video if the device initial orientation is landscape, but fails to properly adjust the picture if it changes during the test

@KazuCocoa
Copy link
Member

KazuCocoa commented Nov 12, 2023

I didn't record the video at that time. I just opened the stream URL on Chrome to go through refactored methods in order to compare before/after as a quick check. The orientation was both portrait and landscape at the beginning and changed the rotation in the middle.
The error itself was one orientation (no change), I haven't checked well for more complex pattern like change the orientation fequently

@mykola-mokhnach
Copy link
Author

I didn't record the video at that time. I just opened the stream URL on Chrome to go through refactored methods in order to compare before/after as a quick check. The orientation was both portrait and landscape at the beginning and changed the rotation in the middle. The error itself was one orientation (no change), I haven't checked well for more complex pattern like change the orientation fequently

Sorry for not being too precise. This issue is only visible if the actual mjpeg stream is recorded to a video, like x264 or AVC one. Browsers properly handle EXIF tags and adjust shown images automatically.

@KazuCocoa
Copy link
Member

Oh, i see

This issue is only visible if the actual mjpeg stream is recorded to a video, like x264 or AVC one. Browsers properly handle EXIF tags and adjust shown images automatically.

@KazuCocoa
Copy link
Member

I'll need to prepare the recording env on my local, so the video recording test will be late.

At this time, I checked my previous comment's error in #812 (comment). I observed the wda process stop after 20 sec or more (at least in 1 min or less) when I opened the http://<device address>:9100/ in Chrome. I noticed that from the animation perspective, such as YouTube content, the current master was smoother than 8aa5c06.

Hm, the error didn't have the stacktrace in the Xcode so my investigation will take more time for the process stop as well

Copy link
Collaborator

@Dan-Maor Dan-Maor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The performance seems to be much better, the WDA process CPU threshold on my 13 Pro Max doesn't pass the 60% mark in the debugger and memory usage appears to be stable.

However, when I tested with ffmpeg it doesn't look like it solved the original problem (i.e. starting with portrait, going to landscape and then to portrait again - the orientation is incorrect).

For reference, the command I used is ffmpeg -f mjpeg -c:v mjpeg -i http://x.x.x.x:n test.mp4

WebDriverAgentLib/Utilities/FBImageProcessor.m Outdated Show resolved Hide resolved
WebDriverAgentLib/Utilities/FBImageProcessor.m Outdated Show resolved Hide resolved
@mykola-mokhnach
Copy link
Author

mykola-mokhnach commented Nov 14, 2023

The performance seems to be much better, the WDA process CPU threshold on my 13 Pro Max doesn't pass the 60% mark in the debugger and memory usage appears to be stable.

However, when I tested with ffmpeg it doesn't look like it solved the original problem (i.e. starting with portrait, going to landscape and then to portrait again - the orientation is incorrect).

For reference, the command I used is ffmpeg -f mjpeg -c:v mjpeg -i http://x.x.x.x:n test.mp4

Ok, I've finally made it work with the UIGraphicsImageRenderer
the thumbnail creator is nice, but looks like it only works for scaling and does not care about rotation fixing :(

@Dan-Maor
Copy link
Collaborator

Ran a check, unfortunately this approach looks to be too heavy resource-wise as well. When the device is in landscape it doesn't take long until it is stopped due to high CPU and memory utilization.

Adding an autoreleasepool to fixedImageDataWithImageData as @KazuCocoa suggested helps with the memory utilization, however the CPU usage is still extremely high.

@mykola-mokhnach mykola-mokhnach changed the title refactor: Fix screenshots orientation for the MJPEG stream refactor: Optimize screenshots preprocessing Nov 15, 2023
@mykola-mokhnach
Copy link
Author

mykola-mokhnach commented Nov 15, 2023

Ran a check, unfortunately this approach looks to be too heavy resource-wise as well. When the device is in landscape it doesn't take long until it is stopped due to high CPU and memory utilization.

Adding an autoreleasepool to fixedImageDataWithImageData as @KazuCocoa suggested helps with the memory utilization, however the CPU usage is still extremely high.

thanks for confirming it @Dan-Maor
So, let then skip rotation fixes for MJPEG images as it creates too much of the CPU load and only keep resize op which is done via prepareThumbnailOfSize API (which has been confirmed as good enough from the performance perspective)

I will only preserve the UIGraphicsImageRenderer-based transformation for regular PNG screenshots as these are not supposed to be taken so frequently

Copy link
Collaborator

@Dan-Maor Dan-Maor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
I've verified that the original recording behavior is preserved with the setting turned off and that the orientation fix is working with the setting turned on.

uti:UTTypeJPEG
compressionQuality:recompressionQuality
// iOS always returns screnshots in portrait orientation, but puts the real value into the metadata
// Use it with care. See https://github.com/appium/WebDriverAgent/pull/812
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mykola-mokhnach mykola-mokhnach merged commit 0b41757 into master Nov 16, 2023
43 of 46 checks passed
@mykola-mokhnach mykola-mokhnach deleted the orient branch November 16, 2023 14:59
github-actions bot pushed a commit that referenced this pull request Nov 16, 2023
## [5.15.1](v5.15.0...v5.15.1) (2023-11-16)

### Bug Fixes

* Content-Type of the MJPEG server ([b4704da](b4704da))

### Code Refactoring

* Optimize screenshots preprocessing ([#812](#812)) ([0b41757](0b41757))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants