HaikuArchives / ffmpegGUI

GUI for FFmpeg
MIT License
23 stars 10 forks source link

Move cropping options to separate tab and add cropping preview #112

Closed andimachovec closed 1 year ago

andimachovec commented 1 year ago

The image currently used for the cropping preview is the first frame in the video. I plan to extend that to be able to switch between a few different frames ( again, we don't want ffmpegGUI to become a video editor). At the moment the preview marker only reflects the values selected in the cropping spinners. It can't be changed via mouse. This feature can be added later on if we need it.

Implements #28

@humdingerb , @scottmc , what do you think? ;-)

humdingerb commented 1 year ago

WRT to the CPU pegging, I can only trace it back to the CropView's Draw() function being called continuously. By whom, I don't know.

andimachovec commented 1 year ago

Thanks, that's a good hint. :+1:

humdingerb commented 1 year ago

@andimachovec, you think we can merge this one soon and deal with fixing stuff after? I'd like to merge #113, which I think is ready. I've also played a bit with the crop tab layout I'd like to present you. :)

andimachovec commented 1 year ago

Yep, shouldn't take too long, just a few small things to add. My cousin is here on a visit currently, so it still might take a few days. Happy to hear about your layout idea. Couldn't get the layout for the new buttons to work correctly anyway 😉

humdingerb commented 1 year ago

Entertain your cousin by showing some nice commits... :) BTW, I also looked into dis/enabling those arrow-buttons when they reach the start/end of the preview image index. So, if you haven't yet, that one thing you don't have to add.

andimachovec commented 1 year ago

Hi @humdingerb Back in Haiku-Land again ;-) Boundary checking for the preview image switching buttons is implemented. I don´t disable them when they reach the boundaries but rather switch to the other end, so the buttons can be used in a cyclic way. If you prefer the buttons to be disabled just let me know, it' s no big effort to change the behaviour. The problem with the cpu peg out is also fixed. I called Invalidate() from Draw() which causes a redraw itself and causes Draw() to be called. Definitely not the thing to do ;-)

We´re slowly getting there ;-)

humdingerb commented 1 year ago

Welcome back! :) Tell me when this PR is ready to merge. My "jobs" branch has been waiting so long, it'll soon burst into blossom, now that it's spring... :)

andimachovec commented 1 year ago

My "jobs" branch has been waiting so long, it'll soon burst into blossom, now that it's spring... :)

Well that´s great, maybe a nice cherry colour. Would fit well with Haiku´s Japanese connection. ;-)

Nevertheless, I´ve updated this branch to a state where it´s nearly ready from my point of view. Could you take care of the placement of the Reset and Left & Right buttons in the Cropping tab? That would be great, my attempts to place them next to the image but still keep their size where not really successful. But that would be best done after merging this PR, right?

humdingerb commented 1 year ago

Yes, better we merge this first and do some layout fiddling afterwards. Let me know when you're ready to merge.

andimachovec commented 1 year ago

It's ready. Let's merge! :-) There`s a bug I've discovered in MainWindow::_ToggleCropping() which enables the cropping tab and option controls even if no source file is set. But I'd rather fix this in a separate PR.

humdingerb commented 1 year ago

Very good! I'll prepare a PR for the crop layout suggestion tomorrow.