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

Frame properties #32

Open
TbtBI opened this issue Feb 19, 2022 · 18 comments
Open

Frame properties #32

TbtBI opened this issue Feb 19, 2022 · 18 comments

Comments

@TbtBI
Copy link

TbtBI commented Feb 19, 2022

Hi. After TDeint and tfm/tdecimate the frame property "_FieldBased" isn't changed (2/1->0).

@FranceBB
Copy link

I can confirm this.

@pinterf
Copy link
Owner

pinterf commented Feb 19, 2022

True, no frame property handling at all. Neither has such feature in TIVTC nor in AviSynth+.
And there are some hidden data-encoded-in-lower-bits method for other data which we also have to get rid of and replace with frame properties. VS port of TIVTC (but not TDeint) did some initiatives on using frame propertiesm this is all I know of the topic.
AviSynth environment has an additional compatibility drawback that it must also match with the existing clip properties.

@realfinder
Copy link

now we got 2 issues with same content #18

@realfinder
Copy link

realfinder commented Feb 26, 2022

some hidden data-encoded-in-lower-bits method for other data which we also have to get rid of and replace with frame properties.

I think "get rid" is not a good solution as a start, I think it's better to see if there are frame property or not, if no frame property then hints hack is used as usual

also same as http://avisynth.nl/index.php/ConvertStacked , we may need "hints hack <> frame property" converter, there are MergeHints() in tivtc and some independent plugins like http://avisynth.nl/index.php/TelecideHints already

@pinterf
Copy link
Owner

pinterf commented Feb 27, 2022

Last year TIVTC was ported to VapourSynth, they have already made the complete transition to frame properties. It has to be 'only' backported while maintaining compatibility with old bit-hint stuff.

@pinterf
Copy link
Owner

pinterf commented May 3, 2022

Just for the record:
In VapourSynth port - https://github.com/dubhater/vapoursynth-tivtc which was based on my version frame properties are handled fine, The old bit-hint hack is completely removed of course.
Some handled properties appear in TFM::putFrameProperties, others are set in various places.
List of handled properties: https://github.com/dubhater/vapoursynth-tivtc/blob/master/src/internal.h#L23

Relevant commits:

dubhater/vapoursynth-tivtc@6c6424a
dubhater/vapoursynth-tivtc@42ee261
dubhater/vapoursynth-tivtc@f6886a4
dubhater/vapoursynth-tivtc@e967d3f

Unfortunately (for us) other things were modernized/rewritten as well, so a one-by-one backport is no longer possible.

@pinterf
Copy link
Owner

pinterf commented Sep 15, 2022

Please look at the first test build, which includes frame properties used by the VS port.
https://github.com/pinterf/TIVTC/releases/tag/v1.0.27test1

@realfinder
Copy link

realfinder commented Sep 15, 2022

seems ok, I always love how your test build work ok even if you said that you didn't test it at all :)

but with https://www.mediafire.com/file/q2q0a64yosh79rz/INTRO.demuxed.m2v/file there are small diff

D2VSource("INTRO.demuxed.d2v")
tfm.TDecimate
a=last

LoadPlugin("TIVTC.dll")
D2VSource("INTRO.demuxed.d2v")
tfm.TDecimate
mt_makediff(last,a,u=3,v=3)
Mt_lut("x 128 = 128 255 ?",u=3,v=3)
#~ propShow(8)

not in all frames but with some like 2322 (after ivtc)

edit: I don't think it's a bug (rather than difference due to updates) but it might be, tfm part are 100% same aside from hints diffs in the top left, also I will be glad to see updates for #26 and #27

@realfinder
Copy link

btw, the op problem ("_FieldBased" isn't changed (2/1->0)) still there

@pinterf
Copy link
Owner

pinterf commented Sep 16, 2022

I simply didn't have time to test, but of course I tried not to release a version which I knew there are fully unfinished parts (there was a couple of them which I left open; that's why I had some work with it last week and that was the reason I had not released this test earlier). I appreciate your help in testing, you have much broader insight in the topic than I have.

TDeint is still unmodified, since it was not ported along with TIVTC to VapourSynth, so I didn't have ready-made modifications which I could simply pull and integrate into Avisynth code. If the property handling in TIVTC filters work, then credits go to dubhater, I just tried to backport the code properly and mix it with the existing hint handling. If there is a difference though, it must be investigated whether it is a backport bug or the result of a bug which was meanwhile fixed.

@realfinder
Copy link

yes I know that TDeint still not updated, but I think tfm should at least in most cases change _FieldBased to 0

@pinterf
Copy link
Owner

pinterf commented Sep 16, 2022

Now _FieldBased is only read, and if exists, it gets priority over Avisynth's GetParity function.
I don't know how and when can we set it to any value for sure. The property is not written is VapourSynth port.

@realfinder
Copy link

maybe the VapourSynth port not complete yet? from what I see it manly only removed hints hack and replace it with props

anyway it's safe to make tfm write _FieldBased as 0 (since we got _Combed for pp 0 and 1), same for TDeint

and I will ping @videoh @dubhater @myrsloik to see if they got another opinions

@myrsloik
Copy link

myrsloik commented Sep 16, 2022

Technically you should probably only set _FieldBased=0 after adjusting the chroma for the slightly different position. 99.99% of users won't care so setting _FieldBased=0 is correct too I guess.

@realfinder
Copy link

realfinder commented Sep 16, 2022

that only in 60i sections (_Combed=1) with 4:2:0, right? https://forum.doom9.org/showthread.php?p=1849482#post1849482 edit: I think the 30i/p sections also affected

the pure hard telecined part affected? or it depends on the source and how they did the telecine?

@realfinder
Copy link

ok, I did read about chroma case, it seems kinda rare case and it can be anything from what I asked above, so it safe to set _FieldBased=0 in non 420 chroma, also in case of 420 and _ProgressiveFrame=1 frames

@pinterf
Copy link
Owner

pinterf commented Sep 19, 2022

Sorry, I'm ignorant and did not follow developments in other areas. Namely I need help on the _ProgressiveFrame.
And another question, should this property be deleted or set (if not exists) after the filter (and which filter)?

@realfinder
Copy link

Asd-g/MPEG2DecPlus#9 (comment) it tell how the frame was encoded in the stream

if there are no _ProgressiveFrame then tfm should act as usual (as there are no _ProgressiveFrame), you can delete it in tfm output but just in case better to leave it or delete it in TDecimate

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

No branches or pull requests

5 participants