horizontal stripes after upgrading from v2.58 to v2.60

[zee944]

I've upgraded from Avisynth v2.58 to 2.60, and when I opened my project horizontal stripes appeared on the image that weren't there before. Here's a picture. See those stripes on the right side of the orange credits, around the heads of the couple and the very left side of the image? That shouldn't be there.

http://i179.photobucket.com/albums/w286/zee944/TDavisynth260raw.png

Here's the same frame taken from an encode made with v2.58 (it's heavily tweaked but still proves that no stripes are supposed to be seen):

http://i179.photobucket.com/albums/w286/zee944/TDavisynth158tampered.png

The code part that's responsible for the problem is simple as this:


LoadPlugin("Decomb[5.2.2].dll")
MPEG2Source("TDUV.d2v")
AssumeTFF()
Telecide(guide=1)
Decimate(cycle=5)
AssumeFPS(23.976)

Actually, it is the Telecide() command that triggers the problem. I'm in a middle of a big-big project, I'd rather stick with Decomb if this can be fixed a less risky way. If the replacement plugin would do just a slightly different job IVTCing, that would led to catastrophic results and possibly wouldn't be noticed until it's too late.

I could go back to Avisynth 2.58, but I had to switch to 2.60 because of some new features needed for the very same project. I've tried both 2.60 alpha 1 and alpha 3 with the same result. The new feature I needed was ConvertToYV12's 'chromaresample' parameter. I have to do several colorspace conversions while avoiding chroma shifts, and this parameter is a key for archieving that. Discussed here: http://forum.doom9.org/showthread.php?t=164737, Gavino's posts.

What can I do?

[Didée]

Try

...
Telecide(guide=1)
SetPlanarLegacyAlignment(true)
...

[zee944]

:wiping sweat from forehead:

Ah, thanks. Works fine. :)

[Guest]

Is this something I could fix in Telecide()? What exactly is going on here?

[Didée]

The problem isn't about chroma pitch (http://forum.doom9.org/showthread.php?p=1544645#post1544645), or is it?

[IanB]

People have been notified since 2.57 to not assume any relationship between memory attributes of planes. Yet we still have plugins that do foolish things like pitchUV=pitchY/2; and even more foolish srcV=SrcY+src->GetPitch()*vi.height; (and this one dies with all versions after a crop).

As noted there is always the SetPlanarLegacyAlignment(true) compatibility mode and/or Crop(... align=true) for plugins that cannot be corrected.


@neuron2,

I appreciate you did not write the offending mpeg2source pp code, but you did assume the role of custodian, you did accept the chalenge to fix the bug and did suceed. I have pointed to offending lines of code and offered ammended code but the relevant posts seem to have avoided your notice. Not so sure about Telecide.

[TheFluff]

Telecide has always made unsafe assumptions about pitch, and there has been quite a bit of hurf durfed regarding this (http://forum.doom9.org/showthread.php?t=161437), so I'm a bit baffled by neuron2's apparent surprise and attempt to blame Avisynth 2.6 for breaking things. As IanB points out, assuming things about pitch is a recipe for disaster and it has never been the correct thing to do, so blaming Avisynth 2.6 for "breaking" things when what it did was stopping to work around silly bugs is sorta like trying to sweep your problems under somebody else's rug IMO.

[Guest]

I have pointed to offending lines of code and offered ammended code but the relevant posts seem to have avoided your notice. Not so sure about Telecide. Can you please point me again to these things?

[Chikuzen]

Can you please point me again to these things?

http://forum.doom9.org/showthread.php?t=150428

[Guest]

Thanks, but those changes were already made. I'd like to hear about the ones I apparently didn't notice.

[Guest]

As noted there is always the SetPlanarLegacyAlignment(true) compatibility mode and/or Crop(... align=true) for plugins that cannot be corrected. Is there anyway to enforce that from my plugin or is it something that can only be done in the script?

[StainlessS]

Could perhaps just duplicate functionality from 2.6a3

avisynth_2-6\src\filters\debug.cpp

class PlanarLegacyAlignment : public GenericVideoFilter
{
private:
const IScriptEnvironment::PlanarChromaAlignmentMode mode;

public:
PlanarLegacyAlignment( PClip _child, const bool _mode, IScriptEnvironment* env )
: GenericVideoFilter(_child),
mode(_mode ? IScriptEnvironment::PlanarChromaAlignmentOff // Legacy PLANAR_Y alignment
: IScriptEnvironment::PlanarChromaAlignmentOn) // New PLANAR_UV priority alignment
{ }

PVideoFrame __stdcall GetFrame(int n, IScriptEnvironment* env)
{
const IScriptEnvironment::PlanarChromaAlignmentMode
oldmode = env->PlanarChromaAlignment(mode) // Set the PLANAR alignement mode
? IScriptEnvironment::PlanarChromaAlignmentOn
: IScriptEnvironment::PlanarChromaAlignmentOff;

PVideoFrame src = child->GetFrame(n, env); // run the GetFrame chain

env->PlanarChromaAlignment(oldmode); // reset the PLANAR alignement mode

return src;
}

static AVSValue __cdecl Create(AVSValue args, void*, IScriptEnvironment* env)
{
return new PlanarLegacyAlignment(args[0].AsClip(), args[1].AsBool(), env);
}

};


Above, remembers required mode in "mode" in constructor (is inverted for chroma).
In GetFrame(), switch to required mode using constructor set mode
using env->PlanarChromaAlignment(mode) and remember original returned in "oldmode".
Get child PVideoFrame with the set alignment, finally switch back to "oldmode".

Does not take into consideration the mentioned crop alignment.

[IanB]

As noted there is always the SetPlanarLegacyAlignment(true) compatibility mode and/or Crop(... align=true) for plugins that cannot be corrected.
Is there anyway to enforce that from my plugin or is it something that can only be done in the script?
As pointed out by StainlessS you could flip legacy mode around your GetFrame calls, but that means all the child filter chain runs with potentially unaligned chroma planes. Of course any child sse2 filters that don't properly check address alignment may crash and burn. At least one filter author has suggested they might be inclined to just throw an error if the alignment is not to their liking which is just as useful as a crash and burn, just with a "nicer" error message. Some filter defer to a slow C implementation when an unsuitably aligned source is supplied, better ones use a movdqu version, which is still slower.

As for the Crop(... Align=True) that is just a blit to new suitably aligned frame, so sure you could do that as well, but it costs you an extra set of blits.

The PlanarLegacyAlignment mode only effects the behaviour of the env->NewVideoFrame(vi); calls, so you probably need to wrap them as well, but then you end up feeding your parent filter with potentially unaligned output, so the crash and burn target moves down the script a few lines.

Anyway the PlanarLegacyAlignment mode only applies to YV12 frames, so to support new formats like YV16 and YV24 you are going to have to use the correct pitch values for the chroma planes.


Oops, I have previously looked at fixes for your Decomb package, but it seems I never got around to sending them to you or posting about them, life distractions are such a pain, particularly when you remember the intent instead of the action :o so no there is no post about Decomb only the one about PP in Mpeg2Source. (damn the forum search is lame, thank God for Google's site: modifier)

[StainlessS]

Perhaps I'm not terribly bright (It's not up for discussion) but,
why would (( x + 15) / 16) * 16, not be mod 16 aligned ?
(think mentioned in linked thread rather than this one).
EDIT: Here:- http://forum.doom9.org/showthread.php?p=1337605#post1337605

[Gavino]

Perhaps I'm not terribly bright (It's not up for discussion) but,
why would (( x + 15) / 16) * 16, not be mod 16 aligned ?
(think mentioned in linked thread rather than this one).
EDIT: Here:- http://forum.doom9.org/showthread.php?p=1337605#post1337605
(( x + 15) / 16) * 16 clearly is, by construction, mod 16 aligned.

However, what IanB said was:
there is obviously still some recalcitrant code that assumes UVpitch = YPitch/2; and/or YPitch = (Rowsize+15)/16*16
The problem there is not the mod16 alignment, but the erroneous asumption that the pitch is derived simply from the source width, which is wrong if a Crop has been used earlier in the chain.

[StainlessS]

Arh yes Of course, maybe it was the 'and/or' that threw me.
Thank you Big G.

EDIT: I would always use ((x + 15) & 0xFFFFFFF0) rather than ((( x + 15) / 16) * 16), except in script.

[IanB]

Okay, I spent a little time fixing Telecide to correctly handle pitch.

[Guest]

Sweet, thanks very much!

[Guest]

Hi IanB,

I'm ready to test this but when I search for Avisynth 2.6 to download all I find is installer EXE packages. Can I get just the plain DLL somewhere?

[manolito]

AFAIK 7zip can open the installer EXE and it lets you extract the dll just fine...


Cheers
manolito

[IanB]

Run the installer and chose "Standalone, Unpack Files only", this will unpack the files into the directory specified in the next tab. No registry updates are applied, the .dll's and a template .reg file (%INSTALLDIR% needs replacement) are deposited in the target directory.

Flipping between versions by using an installer version to overwrite the existing install is the approved method.

[Guest]

Thanks, guys.

@IanB

When you were looking at changes for Telecide did you have a chance to look at FieldDeinterlace and Decimate? Pushing my luck here. :)

I really appreciate your contribution here, it's clear you did a lot of work on it. Thank you. I will credit your contribution.

[IanB]

FieldDeinterlace.cpp and Decimate.cpp seem to be later generations of coding style and have better discipline regarding pitch. I did not see a need to change them.

FieldDeinterlace uses a style with variables like pitch and dpitch that swaps the intent among the planes in action at the time. It seems to do GetPitch(PLANAR_[YUV]) calls at appropriate times.

Decimate.cpp uses a style with variables like pitchY, pitchUV, dpitchY and dpitchUV and seems correct and consistent in a cursory check pass for usage. This style seems to be the most mature of the three samples.

Without fully reviewing all the code in detail I cannot be 100% sure, but they did pass a pitch torture test script that totally mangled chroma and eventually gpf'd telcide version 5.2.3.


One coding style issue for the future is the use of class statics for variables that really only need local scope. These kill any chance of multithreading the filters. I did start a bit of conversion to local variables in telecide but there is still much to do.

[Guest]

Thank you, IanB, your assistance is greatly appreciated.

[zee944]

I encountered another problem related to this. I have a script that worked fine on v2.58. Now in v2.60 when I put in the "SetPlanarLegacyAlignment(true)" line the script didn't work anymore. VirtualDub dropped the following error when I wanted to encode:

Avisynth read error:
Softwire: caught an access violation at 0x088c723d(code+357),
attempting to read from 0xffffffff

It's not a VirtualDub issue, it doesn't work with avs2avi either. I tried several times. I also restarted the computer, still the same. It seems constant. When I remove the "SetPlanarLegacyAlignment(true)" line it works fine, except the red stripes of course which I don't want to see. Why's that?

[IanB]

SetPlanarLegacyAlignment(true) screws with the default plane alignment for all the filters above it in the script. You should only set around unfixed filters. You can use Crop(..., align=true) to fix output frame alignment....Source
GoodFilter1()
SetPlanarLegacyAlignment(false) # Turn mod 16 alignment back on.
BadFilter()
SetPlanarLegacyAlignment(true) # Turn legacy alignment on
# Crop(0,0,0,0, True) # Repair output alignment of BadFilter(), if GoodFilter2() need mod16 alignment
GoodFilter2()

Post #17, decomb524src.zip, (http://forum.doom9.org/showthread.php?p=1602273#post1602273) above does have a compiled Decomb.dll in the decomb524src/Release directory of the .zip archive.

[zee944]

Thanks IanB, it helped. It's interesting though that why my script part needed to set this switch on and off, as there are only Trim(), Subtitle() and two numeric fuctions in it.

SetPlanarLegacyAlignment(true) # Turn legacy alignment on
GenerateField(18228, 18227)
SetPlanarLegacyAlignment(false) # Turn mod 16 alignment back on.

Function GenerateField(clip src, int replacethis, int replacetothat)
{#
src
frac(float(replacetothat)/2) == 0 ? \
Trim(0, replacethis-1)++bobbed.Trim(replacetothat*2,replacetothat*2).subtitle("one")++Trim(replacethis+1,0) :\
Trim(0, replacethis-1)++bobbed.Trim(replacetothat*2+1,replacetothat*2+1).subtitle("another")++Trim(replacethis+1,0)\
Return(last)
}


Anyway, it works now :)

[IanB]

Hint :- Rather than using frac(float(replacetothat)/2) == 0 which can fail due to inexact float representation for some numbers, try the modulo operator, replacetothat % 2 == 0, to select odd versus even processing.


Note:- Because SetPlanarLegacyAlignment functions up through the GetFrame chain it works backwards up the script. i.e. You place a False call before the bad filter and a True call after it, possibly with a Crop(..., True) if required.

:script: Also post your entire script, for the good of the whole community the filter causing the problem needs to be identified and if possible a fixed version provided.

[Guest]

FieldDeinterlace.cpp and Decimate.cpp seem to be later generations of coding style and have better discipline regarding pitch. I did not see a need to change them. I've released Decomb 5.2.4 with your changes for pitch handling in Telecide. Thank you once again for your assistance. I credited your contribution as "IanB". Let me know if you want me to use your real-life name also.

[zee944]

Oh, I see now. For some reason I switched the two lines, so they didn't affect the commands and function I mentioned above, but everything before and after them :)

MPEG2Source("TDUV.d2v")

# ---------------- fix blended fields --------------------
global bobbed=last.Bob(0.0, 1.0).SeparateFields().AssumeFPS(59.940)
SeparateFields()
FreezeFrame(17947, 17947, 17949)
FreezeFrame(17952, 17952, 17954)
FreezeFrame(17982, 17982, 17984)
FreezeFrame(18099, 18099, 18101)
FreezeFrame(18137, 18137, 18139)
FreezeFrame(18169, 18169, 18171)
FreezeFrame(18229, 18229, 18231)
# Replace fields that has no pairs so they have to be generated from the other field
SetPlanarLegacyAlignment(true) # Turn legacy alignment on
GenerateField(17934, 17935)
GenerateField(18042, 18043)
GenerateField(18098, 18097)
GenerateField(18146, 18147)
GenerateField(18158, 18157)
GenerateField(18184, 18185)
GenerateField(18228, 18227)
SetPlanarLegacyAlignment(false) # Turn mod 16 alignment back on.
Weave()
# ---------------------------------------------------------------------------------------------------

AssumeTFF()
Telecide(guide=1)
SetPlanarLegacyAlignment(true) # Turn legacy alignment on
Decimate(cycle=5)
SetPlanarLegacyAlignment(false) # Turn mod 16 alignment back on.
AssumeFPS(23.976)
RemoveUnneccessaryFrames(7293,1)
RemoveUnneccessaryFrames(7276,1)
RemoveUnneccessaryFrames(7270,1)
RemoveUnneccessaryFrames(7252,1)
RemoveUnneccessaryFrames(7242,1)
RemoveUnneccessaryFrames(7230,1)
RemoveUnneccessaryFrames(7217,1)
RemoveUnneccessaryFrames(7202,1)
RemoveUnneccessaryFrames(7193,1)
RemoveUnneccessaryFrames(7178,1)
Trim(7166, 7305)

Vinverse()

AddBorders(16,16,16,16)
gradfun2db(2)
Crop(16,16,-16,-16)

ConvertToRGB32()

Function RemoveUnneccessaryFrames(clip src, int itsplace, int inbetween)
{#
src
Trim(last, 0,itsplace-1)++Trim(last, itsplace-1+inbetween+1,0)

Return(last)
}

Function GenerateField(clip src, int replacethis, int replacetothat)
{#
src
frac(float(replacetothat)/2) == 0 ? \
Trim(0, replacethis-1)++bobbed.Trim(replacetothat*2,replacetothat*2).subtitle("one")++Trim(replacethis+1,0) :\
Trim(0, replacethis-1)++bobbed.Trim(replacetothat*2+1,replacetothat*2+1).subtitle("another")++Trim(replacethis+1,0)\
Return(last)
}