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

Images to video generation cli command tests added (Resolves #11) #13

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rutvik110
Copy link

Images to video generation cli command tests added (Resolves #11)

This PR adds tests for a ffmpeg operation that takes in a group of images and an audio file and outputs a video file.

@rutvik110 rutvik110 self-assigned this Feb 10, 2024
@@ -113,6 +113,42 @@ void main() {
),
);
});

test("converts a group of images to a video", () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Notice that other tests in this file test technical details of the package. This test is really more of an example use-case. If we put all use cases in this file then it would be very difficult to find anything. Please put this test in a new file called test/use_cases/use_cases_test.dart.

FfmpegInput.asset("assets/audio/audio.mp3"),
],
args: [
const CliArg(name: 'y'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add comments for why you need/want each of these arguments.

Copy link
Author

Choose a reason for hiding this comment

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

Sure.

test("converts a group of images to a video", () {
final command = FfmpegCommand.simple(
inputs: [
// file%d.png represents images named like file0.png, file1.png, file2.png, etc.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of calling things file - how about we use names that reflect what a real file name for this purpose might look like. For example, I assume these files are screenshots?

Copy link
Author

Choose a reason for hiding this comment

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

yah, they were. Will update the names.

const CliArg(name: 'c:a', value: 'aac'),
const CliArg(name: 'qscale:v', value: '1'),
],
outputFilepath: 'assets/output/file.mp4',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you really name your output file.mp4?

Copy link
Author

Choose a reason for hiding this comment

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

I'll update it to something more appropriate.


expect(
command.toCli(),
const CliCommand(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you run this command to ensure that it does what you expect?

Copy link
Author

@rutvik110 rutvik110 Feb 11, 2024

Choose a reason for hiding this comment

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

yah, works nicely.

@rutvik110
Copy link
Author

@matthew-carroll Latest updates:

  1. Moved use cases tests under use_cases folder.
  2. Separate tests for generating video with/without audio file
  3. Docs updated to include explanations for ffmpeg inputs
  4. An additional input parameter added to make videos compatible with QuickTime and most other players due to pixel formatting issue.
    More info here

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.

Make Flutter image to video generation work with this package
2 participants