Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix sample video frame rate and reenable ExternalTextureTests #34479

Merged
auto-submit merged 7 commits into from
Jul 18, 2022
Merged

Fix sample video frame rate and reenable ExternalTextureTests #34479

auto-submit merged 7 commits into from
Jul 18, 2022

Conversation

@ds84182
Copy link
Contributor

@ds84182 ds84182 commented Jul 6, 2022

FFMPEG selects a default video frame rate of 25fps when encoding a static image as a video. Since the video is 1 second long, we want a frame rate of 1fps so there is only a single frame encoded for the entire video.

Verified using ffprobe that the file contains a single frame now.

Closes flutter/flutter#106673

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@skia-gold
Copy link

skia-gold commented Jul 6, 2022

Gold has detected about 7 new digest(s) on patchset 2.
View them at https://flutter-engine-gold.skia.org/cl/github/34479

@skia-gold
Copy link

skia-gold commented Jul 7, 2022

Gold has detected about 5 new digest(s) on patchset 3.
View them at https://flutter-engine-gold.skia.org/cl/github/34479

@skia-gold
Copy link

skia-gold commented Jul 7, 2022

Gold has detected about 7 new digest(s) on patchset 4.
View them at https://flutter-engine-gold.skia.org/cl/github/34479

@skia-gold
Copy link

skia-gold commented Jul 7, 2022

Gold has detected about 7 new digest(s) on patchset 5.
View them at https://flutter-engine-gold.skia.org/cl/github/34479

@skia-gold
Copy link

skia-gold commented Jul 7, 2022

Gold has detected about 7 new digest(s) on patchset 6.
View them at https://flutter-engine-gold.skia.org/cl/github/34479

@chinmaygarde
Copy link
Contributor

chinmaygarde commented Jul 11, 2022

cc @dnfield

@dnfield
Copy link
Contributor

dnfield commented Jul 11, 2022

Looks like the test is not completing...

@ds84182
Copy link
Contributor Author

ds84182 commented Jul 11, 2022

There's a race condition where the wrong scenario is displayed sometimes. Haven't dug deeper into it, but animated_color_square is incorrect and it never completes because it's the wrong scenario. https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8809266353870242305/+/u/Android_Unit_Tests/stdout#L21359_0

@ds84182
Copy link
Contributor Author

ds84182 commented Jul 11, 2022

Nevermind about the race condition above, I realized the specific test that's failing isn't the scenario app integration test.

@chinmaygarde chinmaygarde mentioned this pull request Jul 18, 2022
Closed
@ds84182 ds84182 marked this pull request as ready for review July 18, 2022 17:59
@ds84182 ds84182 requested a review from jason-simmons July 18, 2022 18:04
@skia-gold
Copy link

skia-gold commented Jul 18, 2022

Gold has detected about 4 new digest(s) on patchset 7.
View them at https://flutter-engine-gold.skia.org/cl/github/34479

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 18, 2022
@dnfield
Copy link
Contributor

dnfield commented Jul 18, 2022

Thank you!

@auto-submit auto-submit bot merged commit e481e6d into flutter:main Jul 18, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 19, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2022
@engine-flutter-autoroll
e481e6d Fix sample video frame rate and reenable ExternalTextureTests (…
464385e 
…flutter/engine#34479)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 19, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2022
@engine-flutter-autoroll
e481e6d Fix sample video frame rate and reenable ExternalTextureTests (…
34a20ee 
…flutter/engine#34479)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 19, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2022
@engine-flutter-autoroll
e481e6d Fix sample video frame rate and reenable ExternalTextureTests (…
ab3db6d 
…flutter/engine#34479)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 19, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2022
@engine-flutter-autoroll
e481e6d Fix sample video frame rate and reenable ExternalTextureTests (…
17357c3 
…flutter/engine#34479)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 19, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2022
@engine-flutter-autoroll
e481e6d Fix sample video frame rate and reenable ExternalTextureTests (…
4643c90 
…flutter/engine#34479)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@dnfield dnfield dnfield approved these changes

@jason-simmons jason-simmons jason-simmons approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Re-enable ExternalTextureTests

5 participants

@ds84182 @skia-gold @chinmaygarde @dnfield @jason-simmons