CrendKing / avisynth_filter

DirectShow filters that put AviSynth and VapourSynth into video playing
MIT License
108 stars 8 forks source link

Aspect ratio #79

Closed flossy83 closed 2 years ago

flossy83 commented 2 years ago

Environment

Describe the bug

Aspect ratio of a 720x576 square pixel video is slightly squashed from 1.25:1 → 1.22:1 (test clip in last section)

To Reproduce

https://forum.doom9.org/showthread.php?p=1977687#post1977687

CrendKing commented 2 years ago

Could you please move the feature request to a separate issue for better tracking?

CrendKing commented 2 years ago

Please try https://github.com/CrendKing/avisynth_filter/actions/runs/3424393572. I tested with some crop and resize functions without problem.

flossy83 commented 2 years ago

Thank you, that seems to have resolved it.

But there is still a problem slightly related to this -- a lot of my scripts perform custom cropping and scaling which assumes square pixels for all videos, since that is how standalone Avisynth normally behaves.

But with your plugin forwarding on DAR metadata to the renderer, this breaks those scripts since the renderer no longer thinks it's getting square pixels.

Is it possible to have an option to disable all DAR processing in your Avisynth filter and just treat all videos as square pixels? For the purpose of retaining backwards compatibility with scripts.

flossy83 commented 2 years ago

Actually, if I set manually _SARNum & _SARDen to 1 & 1 in my script, will Avisynth pass that on to renderer?

edit: it seems the SAR values are ignored by Avisynth filter when I set them with

propSet("_SARNum", last.width) propSet("_SARDen", last.height)

It is definitely setting them though according to propShow()

CrendKing commented 2 years ago

DAR metadata to the renderer

I'm not sure if I get your concern correctly. DAR is part of VIDEOINFOHEADER2, which all filters preserves, including LAV Filters.

On the other hand, since you mentioned "square" pixels, I assume you are actually talking about pixel aspect ratio (PAR), not display aspect ratio (DAR)? AFAIK, feel free to correct me if I'm wrong here, there is no metadata in DirectShow that sets PAR directly. According to wiki, PAR = DAR / SAR (SAR being storage aspect ratio here). Since we have both DAR and SAR, we can always calculate PAR. Our filters already calculate PAR and set _SARNUM and _SARDen properties (SAR being sample aspect ratio, which is the same as PAR) VPSF shows the SAR for this particular video being 1:1, as shown below.

Clipboard 1

What I changed in https://github.com/CrendKing/avisynth_filter/commit/62a7cc56a44ebca36133bb543f3fe2babe5bbc2a is fix the calculation so that if synth script does any crop or resize to change video dimension, we preserve the original PAR (usually being 1:1), and update DAR in VIH2. For example, the input DAR is 5:4, SAR is 720:576, thus input PAR is 5:4 / 720:576 = 1:1. If the script resizes to 600:300, DAR will be updated to 600:300 * 1:1 = 2:1.

If it is broken for your script, could you post a dumb-down version of your script, test video and expected/actual result so I can repro?

CrendKing commented 2 years ago

Oh, forgot to mention that I also fixed the unsimplified SAR numbers in AVSF. The previous 2880:2880 should be now 1:1.

flossy83 commented 2 years ago

The problem is that normally when we use Avisynth on its own without your Directshow filter, there is no Directshow metadata like dwPictAspectRatioX/Y being passed through from the source filter to the renderer. eg. for a 720x576 16:9 video we have to manually stretch this in Avisynth like eg. BicubicResize(1024, 576) to get the correct aspect (1024/576 = 16/9) otherwise the aspect would be 720/576=1.25. That test pattern is an outlier that just happens to be natively 720x576 1.25:1 but no video exists like that in the real world.

Test clip 720x576 16:9 https://drive.google.com/file/d/1inJLUTbTf01YRCVBqENdoc-gjXL05soy/view?usp=share_link)

Standalone Avisynth (just opening my .avs file directly in MPC-HC without your filter enabled) https://i.ibb.co/BrbrM17/1.png

Avisynth Filter (opening the mkv in MPC-HC with your filter enabled) https://i.ibb.co/WF3yRvm/2.png

It's actually a very good thing that your filter passes the aspect metadata through to renderer, it's just that it's causing this inadvertent issue where we have to re-do all our aspect handling in our scripts.

I guess I'm looking for a way to force Avisynth filter to behave as if dwPictAspectRatioX/Y is null or not set?

CrendKing commented 2 years ago

I can repro the screenshot you attached, and I think what you propose is to have an option to always set output PAR to 1:1, regardless of the input PAR. For instance, the new video's input PAR is 16:9 / 720:576 = 64:45. If output PAR is 1:1, you want the output DAR to be exactly 5:4 (same as SAR).

Of course, this is doable, but based on my experience of video playing this kind of request is very rare, is it not? There are seldom new videos nowadays still not 16:9. And if there is, playing it on 16:9 screen without stretching is just wrong (as your screenshot shows). If you'd like to archive these old AR videos in their pristine form, go for it. However, since AVSF is mostly for video playing, why don't you resize these videos to 16:9 for playing in a batch and be done with it, rather than handling AR in real-time again and again? Am I missing something?

flossy83 commented 2 years ago

Yes, it is definitely "ideal" and "best practices" to not have to manually stretch to 1024x576 and let the renderer do that based on the aspect flags. But in Avisynth world it's always been square pixels and calculations are done based on that: http://avisynth.nl/index.php/Aspect_ratios

Seem to have another issue with overlays interfering withe aspect, eg. LanczosResize(1440, 576) Subtitle( \ "\nWidth = " + String(last.width) \ + "\nHeight = " + String(last.height) \ + "\n_SARnum = " + String(propGetInt("_SARNum")) \ + "\n_SARden= " + String(propGetInt("_SARDen")) \ , font="courier", text_color=$ffff00, size=32, align=4, lsp=0)

Result: https://i.ibb.co/gd5qrRh/4.png

Removing the Subtitle() fixes the issue, but I need that for debug purposes.

CrendKing commented 2 years ago

in Avisynth world it's always been square pixels

OK, but AVSF is in a limbo area between AviSynth world and DirectShow world. In fact, I think it is more lean towards the DirectShow world. The two worlds have fundamentally different requirements and I can only satisfy one without bringing in yet another option.

So, if you really want this, please create a separate issue as feature request, and I can consider introducing a hidden option to always output DAR as SAR.

What's the "interference" in the screenshot?

EDIT: I noticed that if I have this script

    LanczosResize(1440, 576)
    Subtitle(String(propGetInt("_SARNum")))

the debug build will fail with an assertion. It is due to the script clip's video info having width remaining at 720, not the correct 1440. If I remove the Subtitle, the problem goes away. That's probably a bug in AviSynthPlus itself.

flossy83 commented 2 years ago

Initially I was using standalone Avisynth with DirectShowSource() as the source filter, which uses LAV Video decoder as that happens to have highest merit on my system. In this configuration I get square pixels -- it appears the source filter doesn't pass any AR flags to renderer. Same result with FFmpegSource2() and LWLibavvideosource(), but these 2 don't use DirectShow.

In MPC-HC, unticking "Internal filters > Avisynth" uses Windows own generic AVI splitter ("AVI/WAV File Source") to open .avs files as that happens to have highest merit on my system (otherwise it uses MPC-HC's internal LAV splitter). Again square pixels for both.

Purely for the sake of backwards compatibility, it would be nice if we could force square pixels with something like:

propSet("_SARNum", last.width) propSet("_SARDen", last.height)

(these should really be called _PARNum and _PARDen -- Avisynth wiki says they are "Pixel (sample) aspect ratio". Normally SAR means storage aspect ratio.

edit: actually I like your suggestion of managing it in avisynth_filter.ini. So if you don't mind me cluttering up your repo with all these issues I'll add a new issue.

What's the "interference" in the screenshot?

The edges are cropped off. The second overlay is MadVR's debug screen which thinks it's still receiving 720x576 16:9 from Avisynth filter.

Also keep in mind in Avisynth we are doing all kinds of crazy things like SeparateFields(), SelectEven(), nnedi3() upscale field 1 to double height, slight chroma shifting with Spline64Resize() etc. etc. and all this has to be handled with the exact same geometry which might get interfered with if Avisynth filter is performing its own adjustments behind the scenes but I haven't tested this yet, maybe it works perfectly and it's only the final scale that gets affected by the dwPictAspectRatioXY flags.

CrendKing commented 2 years ago

So if you don't mind me cluttering up your repo with all these issues I'll add a new issue.

Please do.

The edges are cropped off.

That's what I noticed too. As I stated above, it seems to be AviSynth+ bug related to Subtitle. Nothing I can do.

If there's no other issue, help me close the issue, OK?