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

Vertical Mode #113

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

Vertical Mode #113

wants to merge 14 commits into from

Conversation

phileastv
Copy link

@phileastv phileastv commented Dec 17, 2023

Hey everyone !

This pull request is to propose initial changes for the integration of vertical snippets.

I'm not a pro programmer, so I hope my code will be good enough and help you :)

Description

  • Creating the option to create "vertical profiles" in the profile page

    • The vertical profiles have a portrait icon next to them. Same for horizontal ones.
    • To make thoses profile persistent, i've added the _vertical suffix to them when creating the folder in the storage
    • The _vertical suffix is hidden in the profiles page (TODO : hide them when adding a snippet).
    • To make work this, I've splitted the getCurrentProfile into getCurrentProfileObject (which returns an Profile object with the isVertical variable, to easily check if the profile is vertical), and the getCurrentProfileString (which is the old getCurrentProfile method, but didn't manage to change to add the isVertical variable, because all the process of creating folder relies on it).
  • When creating a snippet, if a profile with an isVertical value is set to true, there will be a check for the aspect ratio.

    • If the original video is in 4/3, it will be cropped into 9/16 (this is because lots of devices, like my Pixel can shoot 4/3 vertical videos, if you're on the photo mode for example). I thought it would be better to crop everything to 1080x1920 instead of having black borders that appearing and disappearing, or dealing with multiple vertical formats when the movie was created.

There again a lot more work to do to make this happen but I think this is a good beginning !

Checklist

  • I have added a description of the change in CHANGELOG.md.
  • I have added my name in the CONTRIBUTORS.md file, if it wasn't already present.

* Create a system for creating vertical profiles.
* When you select a vertical profile, create 1080x1920 videos.
* When creating a snipper in vertical mode, checks the aspect ratio. If it's 4/3, crop it to fit 9/16.
Changed the scale command when saving a 4:3 video, to crop into 9:16.
The VideoPlayer in the Calendar page now uses the Aspect Ratio of the selected video.
Dialog for creating profiles now have a text to indicate that the toggle is to activate the vertical profile option. When toggling it on, showing an snackbar explaining the mode.
@Hkllopp
Copy link

Hkllopp commented Dec 22, 2023

Merci ! Tu gères !

@phileastv
Copy link
Author

phileastv commented Dec 23, 2023

Hi !

@Hkllopp : De rien, trop hâte que tout fonctionne, je m'amuse bien à faire ça ! 🙂

Little update on this. This basic integration works already well ! I'm currently adding all of my 2023 vertical snippets with it.

I've reworked the profile creation dialog to be more clear, and tested to create a movie : it works ! 🎉

Because when you save a snippet, it issues a different "scale" command to ffmpeg, wether you have a vertical or horizontal profile choosen, we're not supposed to have any 16/9 videos into 9/16 profiles. BUT I tested it anyway by moving a 16/9 file intro a vertical profile to see what it does.

In this situation, the concat command works anyway and the movie is created. But interestingly, the file produced is with mixed resolution : when I move it on my computer, the window size of VLC/IINA changes during the playback, I've never seen that before !

Anyway, I'm reluctant to add a check that all of the snippets are in the same format, to avoid any issues. But given that to check the format of a video file you need to run ffprobe for each file, I'm worried that this check slows down the process. And because even if there were different file formats, the movie generation works, would it really be useful? I'm not sure.

Maybe I can add this check when the app retrieves the video in the profiles folder ?

Anyway, I will continue to work on this, but I'm already happy with theses results !

@phileastv
Copy link
Author

Do you need help to finalize this release @KyleKun ?

@KyleKun
Copy link
Owner

KyleKun commented Jan 15, 2024

Do you need help to finalize this release @KyleKun ?

Actually there is one thing I was checking, if the locale string is too long, it will overlap the date (if it's on the bottom). Maybe we should render the locale below the date?

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