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

Update video files to improve external_texture* related cts coverage #3245

Merged
merged 5 commits into from
Jan 12, 2024

Conversation

shaoboyan
Copy link
Contributor

@shaoboyan shaoboyan commented Jan 8, 2024

This PR updated video files by referring webgl related cases here: https://github.com/KhronosGroup/WebGL/blob/main/sdk/tests/conformance/textures/misc/video-rotation.html#L112

The PR changes:

  • README.md with latest cmds to generate test video files.
  • Rotated video content.
  • Update video expected pixels values based on new test video files.
  • Related cts logic changes, including removing magic numbers.
  • New Vp9 mp4 rotated videos

Fixes #3232


Requirements for PR author:

  • All missing test coverage is tracked with "TODO" or .unimplemented().
  • New helpers are /** documented */ and new helper files are found in helper_index.txt.
  • Test behaves as expected in a WebGPU implementation. (If not passing, explain above.)

Requirements for reviewer sign-off:

  • Tests are properly located in the test tree.
  • Test descriptions allow a reader to "read only the test plans and evaluate coverage completeness", and accurately reflect the test code.
  • Tests provide complete coverage (including validation control cases). Missing coverage MUST be covered by TODOs.
  • Helpers and types promote readability and maintainability.

When landing this PR, be sure to make any necessary issue status updates.

This PR updated video files by referring webgl related cases here:
https://github.com/KhronosGroup/WebGL/blob/main/sdk/tests/conformance/textures/misc/video-rotation.html#L112

The PR changes:
- README.md with latest cmds to generate test video files.
- Rotated video content.
- Update video expected pixels values based on new test video files.
- Related cts logic changes, including removing magic numbers.

Fix:gpuweb#3232
@shaoboyan
Copy link
Contributor Author

shaoboyan commented Jan 8, 2024

Note this change fails tests due to implementation bugs.
webgpu:web_platform,external_texture,video:importExternalTexture,sampleWithVideoFrameWithVisibleRectParam:videoName="four-colors-h264-bt601-rotate-90.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,sampleWithVideoFrameWithVisibleRectParam:videoName="four-colors-h264-bt601-rotate-180.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,sampleWithVideoFrameWithVisibleRectParam:videoName="four-colors-h264-bt601-rotate-270.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,sampleWithVideoFrameWithVisibleRectParam:videoName="four-colors-vp9-bt601-rotate-90.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,sampleWithVideoFrameWithVisibleRectParam:videoName="four-colors-vp9-bt601-rotate-180.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,sampleWithVideoFrameWithVisibleRectParam:videoName="four-colors-vp9-bt601-rotate-270.mp4";*

webgpu:web_platform,external_texture,video:importExternalTexture,compute:videoName="four-colors-h264-bt601-rotate-90.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,compute:videoName="four-colors-h264-bt601-rotate-270.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,compute:videoName="four-colors-vp9-bt601-rotate-90.mp4";*
webgpu:web_platform,external_texture,video:importExternalTexture,compute:videoName="four-colors-vp9-bt601-rotate-270.mp4";*

webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-90.mp4";*
webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-90.mp4";sourceType="VideoFrame";srcDoFlipYDuringCopy=true;dstColorSpace="display-p3"
webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-90.mp4";sourceType="VideoFrame";srcDoFlipYDuringCopy=false;dstColorSpace="display-p3"

webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-180.mp4";sourceType="VideoElement";srcDoFlipYDuringCopy=true;dstColorSpace="display-p3"

webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-180.mp4";sourceType="VideoFrame";srcDoFlipYDuringCopy=true;dstColorSpace="display-p3"

webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-180.mp4";sourceType="VideoFrame";srcDoFlipYDuringCopy=false;dstColorSpace="display-p3"

webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-270.mp4";*

webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-270.mp4";sourceType="VideoFrame";srcDoFlipYDuringCopy=true;dstColorSpace="display-p3"

webgpu:web_platform,copyToTexture,video:copy_from_video:videoName="four-colors-vp9-bt601-rotate-270.mp4";sourceType="VideoFrame";srcDoFlipYDuringCopy=false;dstColorSpace="display-p3"

@shaoboyan shaoboyan requested review from kainino0x and Kangz January 8, 2024 08:30
@shaoboyan shaoboyan requested a review from Kangz January 9, 2024 00:42
@kainino0x
Copy link
Collaborator

I don't think I need to review since Corentin has already reviewed. (Skimmed it - looks good overall)

Just for my knowledge - are we making / did we make a change in Chromium that will flip the rotation of rotated videos (for example vertical phone videos)? Technically this is breaking, though I don't think it's likely any application could have depended on this (they would have to know the rotation of the video in order to compensate for it, which isn't trivial afaik). Still we'll need to be cautious of it.

@shaoboyan
Copy link
Contributor Author

Just for my knowledge - are we making / did we make a change in Chromium that will flip the rotation of rotated videos (for example vertical phone videos)? Technically this is breaking, though I don't think it's likely any application could have depended on this (they would have to know the rotation of the video in order to compensate for it, which isn't trivial afaik). Still we'll need to be cautious of it.

I don't think we have any changes in chromium to flip the rotation of rotated videos unless the metadata(the transform matrix) guided us to do this.

For the rotation changes, ffmpeg update the transform matrix in video metadata but uses ccw direction in cmd. But in browser (at least in chromium), it extracts rotation degree (and mirror) from transform matrix in cw direction. So I made some small changes in the cmd to generate our test video files to align with browser 'terms'.

And in chromium, what we want to do is to ensure the GPUExternalTexture samples/load the pixels with on-screen videos. This means browser plays such video source with HTMLVideoElments tag also apply rotations with transform matrix in video file metadata.

Previous test cases are skipped on chromium CI due to h264 support issue. This PR add vp9 codec videos to fail the bots.

@shaoboyan
Copy link
Contributor Author

shaoboyan commented Jan 9, 2024

@Kangz @kainino0x sry but pls don't merge this PR right now. After reading more chromium code, I think I still need to double check the order to apply rotate, visible rect and mirror. I'm seeking help on slack/media channel.

@kainino0x kainino0x marked this pull request as draft January 10, 2024 01:17
@kainino0x
Copy link
Collaborator

Converted to draft to prevent landing.

@shaoboyan
Copy link
Contributor Author

After some more investigation, I think I fully understand the order to apply visible_rect clipping, rotation (and mirror).
The order should be:
Apply visible_rect to raw decoded video frame first and then apply transforms. In transform, there are rotation and mirror (X-flip), which should apply rotation first and then do mirror.

The new video files contains both vertical videos and horizontal videos. Keep them for coverage.

The cts has been updated based on this and update the comments to describe these steps.

@shaoboyan
Copy link
Contributor Author

@kainino0x @Kangz sry but PTAL again.

@kainino0x kainino0x marked this pull request as ready for review January 12, 2024 00:19
@shaoboyan shaoboyan requested a review from kainino0x January 12, 2024 06:36
@kainino0x kainino0x enabled auto-merge (squash) January 12, 2024 20:53
@kainino0x kainino0x merged commit f6d89c7 into gpuweb:main Jan 12, 2024
1 check passed
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.

Rotated videos in webgpu:web_platform,external_texture,video:* is ccw instead of cw
3 participants