Asd-g / MPEG2DecPlus

Modified for Japanese. original: http://rationalqm.us/dgmpgdec/dgmpgdec.html
GNU General Public License v2.0
23 stars 1 forks source link

_AspectRatio better to replaced with _SARDen and _SARNum #10

Closed realfinder closed 2 years ago

realfinder commented 2 years ago

since all others use _SARDen and _SARNum not _AspectRatio

or better have both?

Asd-g commented 2 years ago

You essentially want the variables FFSAR_NUM, FFSAR_DEN be set as frame properties too?

videoh commented 2 years ago

I noticed that too. They're called _SARNum and _SARDen in Avisynth+. I think we can retain both ways. If Asd-g agrees, I'll fix it in DGMPGDec.

real.finder never misses a thing. ;)

realfinder commented 2 years ago

You essentially want the variables FFSAR_NUM, FFSAR_DEN be set as frame properties too?

never check the variables but I think yes

I noticed that too. They're called _SARNum and _SARDen in Avisynth+.

and in the vapoursynth too

Asd-g commented 2 years ago

I'm ok if we retain _AspectRatio or even if we drop it because it seems not reserved frame property. I have one script that use it but it can be calculated easily in the script.

videoh commented 2 years ago

Let's keep it to not break existing scripts.

videoh commented 2 years ago

/ AR / env->propSetData(props, "_AspectRatio", d.Aspect_Ratio, strlen(d.Aspect_Ratio), 0); unsigned int sar_num, sar_den; (void)sscanf(d.Aspect_Ratio, "%d:%d", &sar_num, &sar_den); env->propSetInt(props, "_SARNum", sar_num, 0); env->propSetInt(props, "_SARDen", sar_den, 0);

I'll slip it into DGMPGDec 2.0.0.7.

realfinder commented 2 years ago

I did test the last update with 4:3 dvd vob and note _SARDen=3 and _SARNum=4 but in LSMASHSource and ffms2 it's _SARDen=9, _SARNum=8

Edit: this is how avspmod do it https://github.com/gispos/AvsPmod/blob/3eab8e9364f9f64cb7a7f16b8155c74ff5a8b320/tools/encoder_gui.py#L259

videoh commented 2 years ago

Yeah, we went through this for DGIndexNV. It's about the difference between SAR and PAR. Everybody wanted PAR to be reported but the property names say _SARNum/_SARDen. I buckled and gave them PAR in the properties.

Theory:

There are two meanings of SAR: "Sample Aspect Ratio" (connected to mpeg4-avc terminology) and "Storage Aspect Ratio" (connected to mpeg2 terminology).

DGIndexNV now uses SAR in the meaning of "Sample Aspect Ratio" which is in fact PAR (from mpeg2 terminology) which means "Pixel Aspect Ratio" and defines "the shape of the pixel".

SAR as "Storage Aspect Ratio" defines "the frame size" and in that meaning SAR = DAR / PAR.

DAR is "Display Aspect Ratio" and defines "the ratio between the width and the height".

I will go ahead and change it to use PAR for _SARNum/_SARDen and post the changes here.

videoh commented 2 years ago

Now I want to get rid of _AspectRatio to avoid confusion. Also Asd-g pointed out that it is not a reserved property.

These changes may require changes to DGIndex and the D2V file format. I will try to avoid that if possible.

videoh commented 2 years ago

OK, please test this:

https://rationalqm.us/misc/DGDecode_test.zip

If it is OK, let me know and then I will give y'all the code. It affects only DGDecode.dll.

videoh commented 2 years ago

And now, do we also need to change the info=1 display and the Info dialog in DGIndex? Or don't bother?

Asd-g commented 2 years ago

Btw the variables FFSAR_NUM/FFSAR_DEN will give the same values as to what ffms2/lsmash gives for _SARDen/_SARNum. So if one wants PAR he can use the variables.

videoh commented 2 years ago

That's OK I guess for your stuff, but I don't have any FF* in DGMPGDec.

Asd-g commented 2 years ago

There is in AVISynthAPI.cpp line 244 but it's not quite the same as the one here

videoh commented 2 years ago

I'll check when I get out of the bathtub. :)

videoh commented 2 years ago

Ah, must have picked that up from the port without noticing. I'm deleting it now. I don't want to set any environment variables, especially FF ones.

realfinder commented 2 years ago

OK, please test this:

https://rationalqm.us/misc/DGDecode_test.zip

If it is OK, let me know and then I will give y'all the code. It affects only DGDecode.dll.

this give 11 for _SARDen and 10 for _SARNum

FFSAR_NUM are 4 and FFSAR_DEN are 3 so both not ok too

videoh commented 2 years ago

11/10 is correct. If you want to dispute it, provide the stream.

realfinder commented 2 years ago

any ntsc 4:3 dvd like this https://www.mediafire.com/file/5v5wopo7w28bwwm/INTRO.demuxed.m2v/file

btw, the FFSAR_NUM and FFSAR_DEN in last MPEG2DecPlus are ok, so maybe copy how they work to _SARDen and _SARNum?

videoh commented 2 years ago

I spent a lot of time on the code I ported from DGIndexNV and nobody has ever claimed any issues with it. I'm happy to re-visit it, but please keep an open mind.

videoh commented 2 years ago

See here:

https://www.afterdawn.com/glossary/term.cfm/pixel_aspect_ratio

The notation is a little different, but it supports my way. Happy to explain it all again if needed.

realfinder commented 2 years ago

See here:

https://www.afterdawn.com/glossary/term.cfm/pixel_aspect_ratio

The notation is a little different, but it supports my way. Happy to explain it all again if your mind is open.

you right, you get the PAR

SAR = DAR/PAR

videoh commented 2 years ago

Thank you. Now MPEG2DecPlus needs to re-consider. I will post my code by tomorrow morning. I'm binge-watching something right now. :)

I need to correct FrameProperties.txt to indicate that _SARNum/_SARDen are the "pixel size" PAR.

videoh commented 2 years ago

Full project:

http://rationalqm.us/misc/DGDecode64_2007.zip

Asd-g commented 2 years ago

Thanks for the sharing.

It seems DGDecode set MPEG-4 PAR values (didn't check the code, just tested with PAL 4:3, NTSC 4:3/16:9) for _SARNum/_SARDen while MPEG2DecPlus/ffms2/lsmash set Generic PAR values - https://forum.doom9.org/showthread.php?p=1058927#post1058927 From the same author posted that doom9 post - https://encodingwissen.de/hintergrund/videobild/anamorph/itu-r-bt601/ (translated version https://encodingwissen-de.translate.goog/hintergrund/videobild/anamorph/itu-r-bt601/?_x_tr_sl=de&_x_tr_tl=en&_x_tr_hl=en)

videoh commented 2 years ago

Yes, I implement the MPEG-4 PAR, which I consider the most useful. It's in-between generic and ITU, has nice easy numbers, and is most suitable for encoding with modern codecs.