Video in chat has wrong aspect ratio and styling

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.74-4
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dubielzyk-expensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1715819494848969

Action Performed:

1. Open any DM or chat
2. Upload any video in portrait orientation

Expected Result:

The uploaded portrait video should have a thumbnail with a portrait aspect ratio and border-radius: 8px corners

Actual Result:

The video thumbnail is landscape and has no border radius

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01235ebb839b18cd37
• Upwork Job ID: 1791469639935733760
• Last Price Increase: 2024-05-27

Issue Owner

Current Issue Owner: @marcochavezf

[melvin-bot]

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01235ebb839b18cd37

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws (External)

[dominictb]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The video thumbnail is landscape and has no border radius

What is the root cause of that problem?

1. The video thumbnail has no border radius
2. The video thumbnail is landscape

What changes do you think we should make in order to solve the problem?

1. Simply update this to

style={[styles.flex1, {borderRadius: variables.componentBorderRadiusNormal}]}

The ImageRenderer also uses the same style variables.componentBorderRadiusNormal for the border radius here

2. This is a back-end issue, when I upload a portrait video recorded on an iPhone, the video is portrait with dimension 2160 x 3840 (width is 2160, height is 3840), but in the back-end response, it returns data-expensify-width=3840 data-expensify-height=2160 which is the other way video.

Because of this, the video thumbnail shows landscape because we're using those 2 attributes to show the thumbnail.

We need to fix the back-end to make sure we return the correct data-expensify-width and data-expensify-height for MOV videos recorded on an iPhone. The front-end has no say here since those attributes are generated in back-end and just returned in response.

More specifically, such MOV video will have video metadata of image_width: 3840, image_height: 2160, rotation: 90 (you can check by uploading such video to https://www.metadata2go.com/). I guess the back-end is simply returning the image_width, image_height without checking the rotation. In this case if the rotation is 90 or similar (270, -90, ...), we need to switch the width and height since the image is rotated already.

What alternative solutions did you explore? (Optional)

For 1. Add borderRadius: variables.componentBorderRadiusNormal and styles.overflowHidden to the outer Views.

For 2. We can make the back-end return the rotation in data-expensify-rotation, and switch width, height in front-end code based on such rotation

[dominictb]

Updated proposal to add an alternative solution.

[aksh1t]

Proposal

Please re-state the problem that we are trying to solve in this issue.

There are two problems that we are trying to solve:

1. The thumbnail image of a video does not have correct corner radius.
2. Portrait videos have their thumbnail in landscape.

What is the root cause of that problem?

The root cause of both problems are different.

1. The Image component in VideoPlayerThumbnail is not styled correctly.
2. The portrait / landscape bug is due to an open issue in react-native-image-picker.

What changes do you think we should make in order to solve the problem?

1. We need to style Image component correctly to apply the corner radius and overflowHidden.
2. To fix the second issue, we need to figure out the EXIF orientation of the image / video before we are uploading it and swap width / height correctly.

What alternative solutions did you explore? (Optional)

Reading the actual size of the thumbnail once it is loaded and updating the size. But this causes a flicker because we wait for the image / video to be laoded first.

[melvin-bot]

Upwork job price has been updated to $125

[joekaufmanexpensify]

This is very minor design issue, so lowering budget for this one.

[melvin-bot]

@Ollyws, @joekaufmanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[Ollyws]

Will get to this one tomorrow.

[Ollyws]

I can't reproduce the portrait issue, works fine for me with a portrait video:

[dominictb]

@Ollyws Yes, but the border issue is there. You can see in the screenshot that the preview doesn't have borders, which is one of the requirement. We should fix it here.

[dubielzyk-expensify]

Have you tested this on all platforms? We had three people reproduce it so that's a bit odd 🤔

Either way like @dominictb said, we'll want the border width, color, and radius to be the same as what we use for photos in a thread

[dominictb]

Have you tested this on all platforms? We had three people reproduce it so that's a bit odd 🤔

@dubielzyk-expensify Yes, could you upload and share the specific video that you used? It might be a video-specific problem

[dubielzyk-expensify]

As @shawnborton could also replicate this I believe it's not a video-specific thing, but it could be a format thing maybe?

You can see a repro here:

recording.mp4

The video I took was portrait and it has landscape thumbnail without rounded corners.

[aksh1t]

Same, I could reproduce the problem with a video taken on a (physical) iOS device. I took portrait videos on my iPhone, but the thumbnails showed up in landscape. It's possible that iPhone's video format has a different way to specify video dimensions. I'll dig into this.

[aksh1t]

Updated

Proposal

[aksh1t]

The portrait / landscape bug is happening due to an open issue in react-native-image-picker library that we are using. The library does not seem to read EXIF orientation correctly and thus give us flipped width and height for portrait videos.

[sonialiap]

Payment summary

• @Ollyws reviewer $125 - please request in ND - please complete checklist
• @dominictb contributor $125 - offer sent

upwork job

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @Ollyws owed $250 via NewDot
• ROLE: @dominictb paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@sonialiap)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1791469639935733760/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[sonialiap]

#42298 (comment)
@Ollyws please request in ND and complete the checklist
@dominictb please accept upwork offer

[joekaufmanexpensify]

I'm back from OOO, so can grab this one back. TY for handling @sonialiap !

[Ollyws]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR:

#42570

• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

https://github.com/Expensify/App/pull/42570/files#r1652682185

• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

N/A

• Determine if we should create a regression test for this bug.

Yes

• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Open any DM or chat
2. Upload any video in portrait orientation
3. The uploaded portrait video should have a thumbnail with a portrait aspect ratio and border-radius: 8px corners

Do we agree 👍 or 👎

[Ollyws]

Requested payment in ND.

[JmillsExpensify]

$125 approved for @Ollyws based on this summary.

[dominictb]

@dominictb please accept upwork offer

@sonialiap Thanks, I accepted

[joekaufmanexpensify]

Checklist is done. This is a pretty niche bug, so IMO we don't need a regression test for this.

[joekaufmanexpensify]

@dominictb $125 sent and contract ended!

[joekaufmanexpensify]

Upwork job closed.

[joekaufmanexpensify]

External portion of this bug is done. However, backend PRs are still in progress. @marcochavezf is there an ETA for getting your draft PRs to review?

[marcochavezf]

I estimate the PRs will be ready for review tomorrow

[dubielzyk-expensify]

Awesome. Let me know when it's ready for testing?

[marcochavezf]

Sure, PR is in review now

[joekaufmanexpensify]

Sounds good. TY! Do we need the web PR and web-PDF PRs too?

[joekaufmanexpensify]

@marcochavezf mind confirming if the web PR and web-PDF draft PRs are needed too? I see the salt PR is merged, so trying to determine if this one is done now.

[marcochavezf]

In the end, the salt PR was the only one needed. Tested real quick and the aspect ratio is now correct for videos in portrait orientation:

I think we can now close this one

[joekaufmanexpensify]

Sweet. Closing. Thanks everyone!