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

Changing aspect ratio possible? #107

Open
humdingerb opened this issue Feb 25, 2023 · 6 comments
Open

Changing aspect ratio possible? #107

humdingerb opened this issue Feb 25, 2023 · 6 comments

Comments

@humdingerb
Copy link
Member

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
Copy link
Contributor

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

@humdingerb
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

This is nice, thumbs up from me 👍

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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants