-
-
Notifications
You must be signed in to change notification settings - Fork 828
Add support for non 16x9 aspect ratios #1354
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
base: main
Are you sure you want to change the base?
Conversation
For non 16x9 content to fit properly
|
Thanks for resubmitting. Do you have sample files I can download from some place to try it out? This will facilitate testing and eventually merge. Thanks |
|
I don't have sample files for all of those aspect ratios. But here are a few I found through a quick online search, that offer free (and royalty-free) downloads, without any sort of sign-up requirements. Most are available to download at various resolutions, except the first anamorphic one, which is a non-standard high-resolution video that would be an interesting test. |
|
I've added 3 of the videos linked above to demo.mediacms.io and also to a local installation with the changes proposed here. I've noticed that
I'll let it open and give it a try soon, once the videojs migration is finished (which we are working since a few months now). On the meanwhile if there's any feedback from others as far as these changes is related, it would be interesting to see. Because I'm not sure what benefit this brings, and also the fact that result files end up larger is not great. I'll have to retest using the same environment most probably, eg try out in X videos in branch main, note the size of each resolution that was created, then delete the videos, apply the changes, restart celery_worker and repeat the process. |
|
I actually just tried the methodology on the last comment and the result size is the same. So ignore that comment :) |
The benefit of this is that media can be shared at appropriate resolutions and aspect ratios relative to the original content rather than shrinking too far, the wrong way. Let's take the example of 4096 x 1716, which is the official DCI resolution for 4K scope (and is a common resolution that many movies are filmed in). This video has a vertical resolution of 1716 pixels and is wider than the default 16x9. The existing method looks only at height and would see that 1716 is less than 2160 (UHD); and it would then look for the next smaller container resolution. The next default profile is 1440, which is a resolution of 2560x1440. So now, your 4096x1716 video is contained within a much smaller 2560x1440. And not only is this a digital container resolution issue, but since this is not a perfect multiple of this profile, this requires subpixel downscaling which further degrades rendered resolution. This is fine for being one of many options; but it should not be the only option. But if look at both the height and width, you will see that the width is closer to (and contained within) UHD (= 3840 x 2160) than the height is to 2560x1440; so you can re-encode to 3840x1609. Much closer to the original resolution than the 2560x1440 of the old method. Further, if you expand to the container size rather than contain both dimensions within it, you will at minimum preserve the original resolution. So there is a benefit; and the question shouldn't be what benefit this brings; but why the existing system doesn't provide these options. To reiterate, here is the existing code: And to translate the above:
Note that both conditionals are identical but with different outputs. So these 2 lines contradict each other. So the question becomes: what benefit is there to having this contradiction? This correction changes to the following:
The next line changes the default from fit within a container to expand using the containers size to deal with the aforementioned downscaling issue. This part is more optional or subjective--this is basically a question of will people ever be able to stream the original resolution of the video? I'd argue the answer should be 'yes.' As far as file sizes go, that is not an issue of resolution: it is an issue of bitrates. You can have a higher-resolution 2160 video that has a smaller file size than a lower-resolution 1080p video, because resolution has nothing to do with file size. If you happened to start by uploading a high resolution + low bitrate video and it then re-encoded by using a default profile that has a larger bitrate, you will end up with a larger file size. This will happen even if you reduced the video's resolution, as long as the bitrate is larger. This has nothing to do with the above changes, which only deal with resolutions and do not deal with bitrates. I hope this helps and clarifies things. |
MediaCMS seems to do ok in terms of scaling when videos are presented at a 16x9 aspect ratio using common consumer resolutions such as UHD (3840x2160) or HD (1920x1080); however, there are many more aspect ratio standards; and this is where MediaCMS does not scale appropriately.
For context, the 16x9 standard itself was designed to be a compromise for wider cinema footage (2.4:1) and common television footage (4:3). 16x9 is the geometric mean between these two aspect ratios; and as such, 16x9 was designed to maximize viewing area by placing horizontal black bars for wider cinema content; and vertical black bars for taller television content, to be displayed on a single monitor (consumer televisions).
Today, in addition to 16x9, we have multiple common aspect ratio standards for video content. Examples:
The issue was that when MediaCMS tried to fit content within a 16x9 area, it was only using the vertical resolution to scale content. This logic was even repeated on 2 consecutive lines, which may have been an error, since the 2nd line appeared to reference width but was still using height (see code changes). (Note that previously, rows 542 and 543 had the same boolean comparison of image-width/image-height, even though only the first was scaling height. This made the boolean condition wasteful and irrelevant, since the condition was always the same).
So in the 4K DCI letterbox example above, suppose one was trying to fit that into a common UHD resolution (= 3840 x 2160). MediaCMS was previously using only the vertical resolution (1716 px), realizing it doesn't quite meet the vertical resolution for UHD (2160 px), and then scaling down to the next resolution, which might be 1920 x 1080. This would result in content that is a maximum of 1920x804 pixels, even though the original content could be larger than 3840x2160 (with a resolution of 4833x1716); or at minimum, fit within 3840x2160 (with a resultant resolution of 3840x1608). Either of these would be appropriate scalings; but since MediaCMS was comparing only vertical resolutions and the content is extra-wide letterbox, the resultant resolutions were far smaller.
This fix is to correct for that--both height and width are compared to the area in order to choose the appropriate resolution scales.
(This pull request is a new version of this previous request: #836 )