VidCoder icon indicating copy to clipboard operation
VidCoder copied to clipboard

Don't disable cropping fields

Open FCrane opened this issue 1 year ago • 11 comments

Feature details

Hi!

Since all cropping automatic modes sometimes still select incorrect values (I already posted this on the HandBrake forum, but the developers don't seem to understand or care), I always need to adjust the values that are preselected.

E.g. I use "Conservative", it scans the video and selects 132, 132 for top and bottom, but the best values would be 132, 130 (my player calculates the correct values). So what I have to do in VidCoder is to switch to manual cropping (because the crop fields are disabled in conservative mode), adjust the values, add the file to the queue and then NOT forget to switch back to conservative, so that the next file will be treated to preselect values.

This is annoying and time consuming! It would be much better if the edit controls for the cropping values would NOT be disabled in any mode, so that adjustments can be made quickly and easily. So when any cropping mode is selected and VidCoder scans a file, it should preset the cropping values according to the mode, but then leave the edit controls enabled so that the user can change the values without having to change the cropping mode...

Thanks!

FCrane avatar Feb 23 '24 07:02 FCrane

I get where you're coming from here, though this is tough to fit in the current model of presets. If you change the cropping values manually, it's hard to tell if the user meant for them to persist to the next encode or not. And we already have two layers of preset: the saved preset and unsaved changes. Having just the cropping values revert back to automatic would mean there's a third layer here; which is going to complicate the already complex preset handling code even more. Like, what if you change the values, then close VidCoder and re-open the same video? Should these values stay? I think you'd expect them to stay, but unless you did some special handling, it would try and reset them every time it loaded in a video.

One thing I could see doing is allowing you to edit the cropping values, but changing the mode to Custom when you do. Then you could use the "revert" feature to bring it back to Conservative for the next video.

RandomEngy avatar Feb 28 '24 05:02 RandomEngy

When scanning a new video or reopening the program, I'd expect the currently selected cropping mode to be applied, thus discarding my manual changes. I think this is pretty intuitive and consistent! Whenever VidCoder would "set" the cropping values itself, it should do so according to the selected mode (which should always stick). When I make quick adjustments, they should be discarded the next time a video is scanned, etc. I guess this would not be confusing for the user.

Switching to Custom whenever I adjust a value and not switching back at the next scan would be even more problematic, because I'd most likely forget it even more often resulting in completely wrong cropping values...

But as I said: when a new video is scanned (or the program is opened), VidCoder could simply use all values of the saved preset. Adjusting any value should be possible (not only the cropping values) without having the new values stick for the next scan. Maybe you can add this mode as an option? I'm sure many users would benefit, because this way you can easily adjust values for a single encode without switching back and forth presets, having to revert, etc. You can still use the * as an indicator that some values are adjusted, but next time a video is scanned, the * can be cleared.

Thanks!

FCrane avatar Feb 28 '24 06:02 FCrane

The * indicator already has a meaning that applies to the whole preset. And there is already a "revert" button. If it stayed on the automatic "Conservative" mode when you edited the values there would need to be two layers of "revert". It would be a little confusing and messy to code. Some of the most difficult debugging I've had to do has been from the preset system - problems updating UI when switching presets, reverting, etc. I'd like to avoid making that even more difficult. It would also be completely unique in behavior among all of the different encoding options, and I'd like it to behave consistently. That is: the values you've chosen in the UI apply to any source that you load.

I've thought about switching over to a system with autosave and undo and removing the "save" and "revert" features on the presets themselves. If I ever got that done, I could see maybe adding some kind of explicit override/revert per-encode thing, but it's just too messy right now.

RandomEngy avatar Feb 28 '24 06:02 RandomEngy

Maybe you could add a checkbox "Allow override", that simply keeps the edit fields enabled? That should be easy and gives the user all flexibility. The override selection should stick, of course, so that i don't need to enable it every time...

FCrane avatar Feb 28 '24 07:02 FCrane

And changing the values manually in override mode should not trigger the * indicator, because the profile technically is still unchanged. I think this would be super comfortable, logical and consistent! And easy to implement...

Thanks!

FCrane avatar Feb 29 '24 08:02 FCrane

Is it not possible to add this small feature which would make the program so much better and much more usable?

Regards!

FCrane avatar May 20 '24 09:05 FCrane

I've thought about switching over to a system with autosave and undo and removing the "save" and "revert" features on the presets themselves. If I ever got that done, I could see maybe adding some kind of explicit override/revert per-encode thing, but it's just too messy right now.

RandomEngy avatar May 20 '24 14:05 RandomEngy

Save and revert is fine, no need to change that! Just add "Allow override" which keeps the cropping fields enabled - nothing else would be needed...

FCrane avatar May 20 '24 14:05 FCrane

I have heard your view here. The only way you could make this happen faster is by submitting a PR that adds undo and removes the save/revert system for encoding presets and pickers.

RandomEngy avatar May 20 '24 15:05 RandomEngy

?? I don't understand - what does undo and save/revert have to do with keeping the cropping fields enabled? Nothing... these are 2 completely different things. Again: just a simple option to keep the cropping fields enabled would be enough. No changes to anything else needed to stay consistent. Everything should work as it does now, just the actual cropping values in the cropping fields should be used (as they are now, when prefilled by "Automatic", etc.)

FCrane avatar May 20 '24 16:05 FCrane

Because as I have said before, two stacking layers of "revert" is too complex.

RandomEngy avatar May 20 '24 16:05 RandomEngy