Dilema - Crop restriction - Fieldbased clips

[IanB]

In Avisynth 2.5 crop() suffers from a new series of even crop restriction for
YUY2 field based clips, these restriction apply even if the clip field height is odd.

Clip examples:-

a) AssumeFrameBased().Crop(0,1,0,0) # succeeds

b) Weave().Crop(0,1,0,0) # succeeds

c) AssumeFieldBased().Crop(0,1,0,0) # fails

d) SeparateFields().Crop(0,1,0,0) # fails

The even crop restriction only need apply to an interlaced frame based clip
there is no problem with individual fields being cropped by an odd amount.

Similarly for YV12 there is a general even height restriction and divisible by 4
restrictions for field based clips are likewise not appropriate.

Problem is there is insufficient information to distinguish between an interlaced
frame clip and a field seperated clip. Interestingly the RGB formats are currently unrestricted.

Thoughts people????

IanB

[hakko504]

Your examples a) and c) looks alright to me. But as you say, weave should produce fieldbased video, thus the crop should fail, and separatefields should produce framebased video thus crop should be OK. I think the problem here is not the actual crop, but rather weave/separate fields not setting the frame/field-based flag correctly (or indeed at all). And as you say, RGB ought to have the same restrictions of vertical crop as YUY2 has.

(Background information on why the crop restrictions were added can be found here in the Gordian Knot forum (http://forum.doom9.org/showthread.php?s=&threadid=42708) and at Avisynth.org (http://www.avisynth.org/index.php?page=DataStorageInAviSynth), FAQ Section3:Filters and Colorspaces (http://www.avisynth.org/index.php?page=Section+3%3A+Filters+and+colorspaces))

[IanB]

Okay, I have had a really good read of the literature and the source :eek: and
am rapidly coming to the conclusion this whole frame/field based is seriously
misunderstood as meaning Progressive/Interlaced.

Hakko:- but rather weave/separate fields not setting the frame/field-based
flag correctly (or indeed at all)

No! I believe this is about the only place where things actually are consistant and
make some sense. Besides this code is pretty much still as Ben wrote it.

Lets try for some definitions from what I have read :-

FrameBased - Video samples are whole frames, either progressive or interlaced.

FieldBased - Video samples are individual fields. Alternate samples are top, bottom fields.

This is obviously at odds with most of the YV12 inspired modifications made so far. The
YV12 code appears to make the following assumption :-

FrameBased==Progressive, FieldBased==Interlaced

This is inconsistant with all the previous versions of AviSynth, and it appears we are
trying to extract desperately needed information from an inappropriate place.

Probably some better definition of progressive/interlaced is needed.


In Avisynth.h

lines 153-157
enum {
IT_BFF = 1<<0,
IT_TFF = 1<<1,
IT_FIELDBASED = 1<<2
};

lines 171-174
bool IsFieldBased() const { return !!(image_type & IT_FIELDBASED); }
bool IsParityKnown() const { return ((image_type & IT_FIELDBASED)&&(image_type & (IT_BFF||IT_TFF))); }
bool IsBFF() const { return !!(pixel_type & IT_BFF); }
bool IsTFF() const { return !!(pixel_type & IT_TFF); }


BUG Alert!
The IsBFF()/IsTFF() functions should be masking image_type not pixel_type. And
nothing ever sets the IT_TFF/IT_BFF flags, so IsParityKnown() always returns False.
AVISource() etc, AssumeTFF() and AssumeBFF() should all update these bits!

I would like to propose the following definition :-

Progressive video samples,

IT_FIELDBASED bit set or both IT_BFF and IT_TFF bits clear

i.e Is FieldBased or a Parity is not Specified

Note : Separated individual fields probably should be treated the same as
progressive frames.

Interlaced video samples, (Needing "even" height protection/processing)

IT_FIELDBASED bit clear and either IT_BFF or IT_TFF bit set

i.e. is Framebased and a Parity is specified

The following code would probably need changing :-

In Avisynth.h (damn!)

lines 172-174
bool IsParityKnown() const { return !!(image_type & (IT_BFF||IT_TFF)); }
bool IsBFF() const { return !!(image_type & IT_BFF); }
bool IsTFF() const { return !!(image_type & IT_TFF); }
bool IsInterlaced() const { return ((image_type & (IT_BFF||IT_TFF)||!(image_type & IT_FIELDBASED)); }


Field.cpp needs to be taught about IT_BFF and IT_TFF bits

Most occurances of .IsFieldBased() need to be vetted.

Opinions!

IanB

[mb1]

Here's an excerpt from the mpeg2 specs, too:
picture_structure -- This is a 2-bit integer defined in the Table 6-14.
Table 6-14 Meaning of picture_structure
picture_structure Meaning
00 reserved
01 Top Field
10 Bottom Field
11 Frame picture
When a frame is encoded in the form of two field pictures both fields must be of the same picture_coding_type, except where the first encoded field is an I-picture in which case the second may be either an I-picture or a P-picture.
The first encoded field of a frame may be a top-field or a bottom field, and the next field must be of opposite parity.
When a frame is encoded in the form of two field pictures the following syntax elements may be set independently in each field picture:
• f_code[0][0], f_code[0][1]
• f_code[1][0], f_code[1][1]
• intra_dc_precision, concealment_motion_vectors, q_scale_type
• intra_vlc_format, alternate_scan

top_field_first -- The meaning of this element depends upon picture_structure, progressive_sequence and repeat_first_field.
If progressive_sequence is equal to ‘0’, this flag indicates what field of a reconstructed frame is output first by the decoding process:
In a field picture top_field_first shall have the value ‘0’, and the only field output by the decoding process is the decoded field picture.
In a frame picture top_field_first being set to ‘1’ indicates that the top field of the reconstructed frame is the first field output by the decoding process. top_field_first being set to ‘0’ indicates that the bottom field of the reconstructed frame is the first field output by decoding process
If progressive_sequence is equal to ‘1’, this flag, combined with repeat_first_field, indicates how many times (one, two or three) the reconstructed frame is output by the decoding process.
If repeat_first_field is set to 0, top_field_first shall be set to ‘0’. In this case the output of the decoding process corresponding to this reconstructed frame consists of one progressive frame.
If top_field_first is set to 0 and repeat_first_field is set to ‘1’, the output of the decoding process corresponding to this reconstructed frame consists of two identical progressive frames.
If top_field_first is set to 1 and repeat_first_field is set to ‘1’, the output of the decoding process corresponding to this reconstructed frame consists of three identical progressive frames.

Mpeg encoders like Tmpeg and CCE handle picture structure frame only.
Canopus Procoder supports picture structure frame, picture structure field and picture structure frame/field mixed.
It is highly recommended for interlaced DV to use the mixed picture structure for superior quality.
Unfortunately Procoder has up to now problems with that. Those streams aren't correctly written.
Canopus works on it (next release should work).

Just as an extra information which is maybe of interest.

[sh0dan]

@IanB: Changing avisynth.h is not a problem - still a stupid bug though. But when the next version will be released the bug should be gone. Nice spot!

Anyway - the handling of interlaced material is quite problematic. The main problem is, that we cannot rely on fielsbased/framebased flags too much anyway.
I must admit I jumped very much around it, since I couldn't find any useful information or reverse ingeneer exatly how getparity is supposed to work.

Yet another issue is that getparity cannot send information about progressive/interlaced frames - only tff and bff (I think).

VideoInfo has the main problem that it cannot change when the graph has been created.

Being able to attach custom properties to single frames, that are passed down through the chain seems like a _very_ welcome thing.

[neuron2]

Originally posted by sh0dan
Being able to attach custom properties to single frames, that are passed down through the chain seems like a _very_ welcome thing. Ah, now you see why I have been "harrassing" you about this. :)

[sh0dan]

Inspiration comes from many sources :D

(However it isn't possible with the current architecture, without nasty hacks (passing property references to VideoInfo and other nasty stuff)

[neuron2]

Originally posted by sh0dan
(However it isn't possible with the current architecture, without nasty hacks (passing property references to VideoInfo and other nasty stuff) I accept your judgement on that. I'm using my personal extremely ugly hack right now. You don't even want to know about it, believe me. :)

[IanB]

1. Pre-apology, I'll be gone interstate for a week so I'll have to let this issue simmer
until I can get online (unlikely) or get back home.

Originally posted by sh0dan
@IanB: Changing avisynth.h is not a problem - still a stupid bug though. But when the next version will be released the bug should be gone. Nice spot!

Currently this is only the tip of the problem, the real problem is that the flags are not
updated, so there is need for code. However as the feature is actually unimplemented
we have the opportunity for some good designing here. Done right this can be really
clean and painless, done wrong and we end up with more incomprehensible getparity()
trickey code.

Anyway - the handling of interlaced material is quite problematic. The main problem is, that we cannot rely on fieldbased/framebased flags too much anyway.

You can't rely on them at all for determining a video streams interlaced status. This
is the "Dilema", we need extra information independant of this flag.

I must admit I jumped very much around it, since I couldn't find any useful information or reverse ingeneer exatly how getparity is supposed to work.

Understanding GetParity() just about fused my brain. It uses a really nasty OO trick of
repeatadly overlaying the the GetParity() method in each child object that changes the
"field"ness of the clip. Translating AviSynth script into pseudo normal "C" activity code
I hope is the leg up one needs to understand what is happening.

AssumeFrameBased() . . . GetParity(int n) { return False; }

AssumeFieldBased() . . . GetParity(int n) { return !!(n & 1); }

ComplementParity() . . . GetParity(int n) { return !child->GetParity(n); }

AssumeTFF() . . . . . . . GetParity(int n) { return (IsFieldBased() && !(n & 1))



Yet another issue is that getparity cannot send information about progressive/interlaced frames - only tff and bff (I think).

Nor is it supposed to. GetParity() is only to keep track of the top/bottom field status
of samples (frames) in a field separated clip i.e. IsFieldBased()==True

VideoInfo has the main problem that it cannot change when the graph has been created.

And it does not need to. In Avisynth Script space a clips attributes are fixed, filters
create a new clip with new attributes (fixed). Translated into In C++ space a clip
is a VideoInfo object.

Being able to attach custom properties to single frames, that are passed down through the chain seems like a _very_ welcome thing.

Although not needed to distinguish the progressive/interlaced state of a Clip (VideoInfo
object) this would be a very welcome addition.

A neat implementation could be to add 2 stub methods to possibly VideoInfo or
a parent.

1. SetTag(int framenumber, char *Key, (void *)object);

2. (void *)GetTag(int framenumber, char *Key);

Key is used to identify the instance of Tag data the current filter wants so multiple
objects can be stashed per frame and the appropriate one retrieved. It could be
as simple as an int or a complex object (class) instead of a string.

Internally it could be some sort of list (linked?) of arrays indexed by framenumber,
but it doesn't matter as the methods could be overlayed by appropriate code for
a given instance of the say current VideoInfo object. The trick is keep it simple
and generic. The base code could even do nothing and just return NULL

IanB

P.S.

Last minute inspiration from MB1's mpeg post. Use IT_TFF and IT_BFF flags as follows


IT_TFF IT_BFF State
=======================
False. . . False. . . Interlaced Status Unknown (Protect even height just to be safe)
False. . . True . . . Interlaced Clip, Bottom Field First
True . . . False. . . Interlaced Clip, Top Field First
True . . . True . . . Progressive Clip

And need a new script verb - AssumeProgressive() cloned from a fixed AssumeTFF()

Also some useful derived methods like IsProgressive(), IsInterlaced(), etc ...

P.P.S. It just hit me. Sh0dan, Neuron2, are you suggesting individual frames have a
Progressive/Interlaced state? If so I am not sure I can cope with the concept.:sly:

[Bidoche]

A neat implementation could be to add 2 stub methods to possibly VideoInfo or a parent.

1. SetTag(int framenumber, char *Key, (void *)object);

2. (void *)GetTag(int framenumber, char *Key);
That's more or less, what I was thinking of, but I was placing them directly in VideoFrame, not in VideoInfo.
You could look into avisynth.h/videoframe.h/.cpp in the struct_mod branch of the cvs if you want to see how I plan to do it (comments welcomed)


P.P.S. It just hit me. Sh0dan, Neuron2, are you suggesting individual frames have a Progressive/Interlaced state?
I would like that, at least I'd nderstand the GetParity stuff like it..

[sh0dan]

In general - assigning properties to frames could be very useful - we have before talked about conditional filtering - something like:


video = something
if (detectscenechange(video,10)) {
if (!fieldbased(video)) {
blur(video,1.0)
}
} else {
temporalsoften(video,3,5,10)
}


But again - we are far out in the future - neverless worth considering for the next major update.

As neuron would say - "Less talk - more code!" :)