search

HaikuArchives / ffmpegGUI

GUI for FFmpeg
MIT License
23 stars 10 forks source link

Changing aspect ratio possible? #107

Open humdingerb opened 1 year ago

humdingerb commented 1 year ago

Using a "Custom resolution" will not change the aspect ratio, just use a (reduced) horizontal/vertical resolution.

But what to do when a file has an incorrect apect ratio? I've been searching the web for a bit, but all suggestions found didn't seem to work.

There's SAR = Sample Aspect Ratio being the actual pixel size, and DAR = Display Aspect Ratio that's telling the media player how to scale the pixels (if the software supports that). Changing SAR requires re-encoding, changing DAR should be doable by doing no encoding (stream-copy) and just changing the DAR meta data of the container.

The parameter "-aspect 16:9" doesn't work with stream-copy or re-encoding. There's "-bsf:v "h264_metadata=sample_aspect_ratio=16/9", but we don't support h264 at all. There's "-vf scale=iw*0.3333:ih" while re-encoding, doesn't change anything either. All checked with "ffprobe" that show SAR and DAR.

Just leaving that here, in the hopes someone more familiar with ffmpeg shows us the way...

andimachovec commented 1 year ago

I'll take a look at it in the next few days

humdingerb commented 1 year ago

Thanks! ffgui-window.cpp is uncomfortably large. Do you mind if I try a bit more re-organizing of the code? Though I have to admit I never did that before, so I dunno if I'll be successful... :) Just don't want to disrupt any changes you may have.

andimachovec commented 1 year ago

ffgui-window.cpp is uncomfortably large.

Yes, it's gotten quite big. I don't see that much code in it that I think would make sense to put in separate classes (and / or files) though. The CodecOption and ContainerOption classes could of course easily put into preferably one extra file for both of these classes. Apart from that I'd rather reorganize the code into smaller functions within the ffguiwin class. The constructor is really big and also a few cases in MessageReceived.

Since we now have our own spinner classes you could move the SetSpinnerMinsize() functions or the respective code there. These functions come from a time where we used standard BSpinners.

Do you mind if I try a bit more re-organizing of the code?

No, not at all. Just let me know if you want to do it now or rather before the next release.

humdingerb commented 1 year ago

Please see #109. I first also moved the SetSpinnerMinsize code into the Spinner class, but when testing things, I turns out the code isn't needed at all. The spinner size is the same with and without it...

I haven't looked any further into what could be improved in MessageReceived(). Leaving that for another time, or you if you have ideas about that. :)

andimachovec commented 1 year ago

This is nice, thumbs up from me :+1:

I first also moved the SetSpinnerMinsize code into the Spinner class, but when testing things, I turns out the code isn't needed at all.

I wrote that before we put the spinners into grid layouts and the numbers above 3 digits didn't fit in. Good that you removed it.

I haven't looked any further into what could be improved in MessageReceived().

The code for the M_ENCODE_FINISHED is a bit long, but it's the only one. And it's not too bad, I think we can leave that for now.

humdingerb commented 1 year ago

Good, let's merge that and let this issue get back on-topic. :)