AviSynth 2.5.6 RC 2 Release Candidate [October 7th] [Archive] - Page 3

IanB

10th May 2005, 05:56

Originally posted by neuron2
Again, I ask, why was this alignment change made? Is the person who did it willing to fix all the broken applications? Why wasn't this discussed at all so that affected people could have their say?

The alignment change was requested so SSE code would work on the chroma planes. Increasing the luma alignment from 16 to 32 should not effect any correctly written code, it should just occasionally waste a little more memory, obviously this is not quite the case.

The issue has been discussed in http://forum.doom9.org/showthread.php?s=&threadid=90527 It is only in developement, if it doesn't work out, we can always change it.

IanB

iNFO-DVD

10th May 2005, 10:45

BTW, I just tried CPU=0, the image was the same.....

Guest

10th May 2005, 12:41

All versions of mpeg2dec and its derivatives have this "incorrectly written" code, which happens to work fine until the alignment rules are changed. Now I'd like to know who is going to fix all of those filters, or are you happy to leave them behind?

Isn't there some way to provide polymorphism, so that existing filters can operate in the old way without requiring changes?

Wilbert

10th May 2005, 13:16

I'm also worried about it. It might affect other filters too. I think we can't take that risk.

IanB

10th May 2005, 14:07

Don,

The fact is the filters are broken already, you can't feed a frame that has been cropped or is not mod 16 width into any of the mpeg2dec stuff. With mod 32 alignment the restriction becomes a little tighter at mod 32 width.

Version 2.5.6 is the current development branch and this is where we need to push the envelope a little to see what is possible, what is feasible and what breaks.

Fine if mod 32 turns out to be an insurmountable problem then we leave it out in the final released version and try again in 2.5.7, just like we will probably have to do with the "Evaluate: (Un)Recognised exception." SEH problem with XPsp2.

Mod 16 aligned chroma is actually very important if we want to use SSE code. Somewhere down the track I'd like to see an SSE2 resizer core and I'd like to have the infrastructure in place and well tested before I get a round tuit:sly:

If I can find time I'll have a proper look at the code and see what can be done. At first glance it looks like just the conv4xxto4xx series engines just need to be taught to use pitch and/or GetRowSize(planar_aligned)instead of width.

IanB

Nic

10th May 2005, 14:33

@IanB: If you could let me know (either here, or by PM) exactly what I'm looking for with the SEH XP SP2 problem, I'll look into it for you.

I'm sure making DGIndex/Decode use your new alignment will be a pain but probably for the best. But I think the ramifications could run to old unsupported filters too...So could there be some switch/pragma/regkey for turning the alignment back to 16 (8 for chroma)?

Because, technically Ian your dead right, the filters are broken to an extent, however they do work perfectly well for a lot of people and to take that away would be quite bad...

-Nic

Edit:
ps. I have to use VC6 SP6, so no Pro Pack 5. So i'll have to make minor changes...Plus, where do _aligned_malloc/free come from, they're not in any platform SDK I use, but maybe I'm out of date.
@Mods: If this is all a bit too off topic, I'll continue it in the development thread.

Guest

10th May 2005, 14:53

Originally posted by IanB
The fact is the filters are broken already, you can't feed a frame that has been cropped or is not mod 16 width into any of the mpeg2dec stuff. mpeg2dec, etc., are source filters; you don't feed frames into them.

sh0dan

10th May 2005, 20:29

mpeg2dec3 could simply request NewVideoFrame(-8) - that should force mod 8 aligned chroma, and not necessarily mod 16 (I don't assume this has changed).

But you need to include encapsulate it in the AlignPlanar class. That class was specificly made for filters that do not always return frames with pitches that are what Avisynth requires. The definision is in avisynth.h, and I just moved the implementation to src/core/alignplanar.cpp for easier use in plugins.

The reasons I didn't push for mod 16 chroma:

* Main reason: A pure SSE2 filter is not useful - you need fallback to at least iSSE, so switching to fallback routine on unaligned pitches should be an option.

* Do it in the Win64 port. All filters would need recompile anyway.

My personal opinion: Move it to 2.5.7 and get 2.5.6 out the door without breaking stuff. With ALL mpeg2dec versions/forks having this problem there will be too big problems for people when upgrading.

Cyberia

10th May 2005, 20:37

Originally posted by neuron2
mpeg2dec, etc., are source filters; you don't feed frames into them.

But the original vob does feed frames into mpeg2dec...

I believe IanB means that if the source video (vob, etc) has a non-mod-16 width, this problem will happen with MPEG2Dec (and derivatives), regardless of the AviSynth version. Does anyone have a vob like that that we could test this theory against?

Guest

10th May 2005, 20:40

@Cyberia

MPEG2 widths are always a multiple of 4 so they're guaranteed to be 16-aligned.

IanB

11th May 2005, 05:04

Originally posted by neuron2 MPEG2 widths are always a multiple of 4 so they're guaranteed to be 16-aligned. Come again mod 4 width implies mod 4 (32 bit) alignment.

I just knocked up a 712x480 Mpeg2 (TMpg would only do mod 8) and just about every piece of Mpeg2 software I have fell out of the sky, only PowerDVD survived and it upsize the image to 720x480 with a 8 pixel black border on the right, so only half marks.mpeg2dec, etc., are source filters; you don't feed frames into them.True for MPEG2Source, but all the auxilary filters; LumaFilter, YV12toRGB24, YV12toYUY2, BlindPP & DeBlock; expose the alignment challenged internals nicely.Originally posted by sh0dan
* Main reason: A pure SSE2 filter is not useful - you need fallback to at least iSSE, so switching to fallback routine on unaligned pitches should be an option.Pure SSE2 can handle any alignment, it just takes a big hit with non-mod 16 alignment, however it is still faster than the equivalent MMX code especially when some of the fancy fangled intructions are used.

Fallback code should be for supporting lesser machines, not poor programming. And yes I admit to doing this just like everybody else, I just should know better. :sly: * Do it in the Win64 port. All filters would need recompile anyway.Ah em rewrite I hear you say :angry: f... Micro$oft, Softwire may yet save the day;)

I think leaving the current alignment in until we are nearer to release to shake any other gremlins loose, then take it out until 2.5.7 to save unconfortable problems in a released 2.5.6 version is the go.

IanB

Guest

11th May 2005, 05:47

@IanB

Why can't you just make the default be 8-byte chroma alignment with the new mechanism? Then old stuff would still work but you could ask for 16-byte chroma alignment if you want it.

Leak

11th May 2005, 07:45

Originally posted by neuron2
Why can't you just make the default be 8-byte chroma alignment with the new mechanism? Then old stuff would still work but you could ask for 16-byte chroma alignment if you want it.

Well, I guess the idea is to make sure most stuff uses 16-byte chroma alignment as soon as possible (i.e. from the get-go) as otherwise you'd again have to blit the frame to re-align it, probably several times in a filter chain...

np: Ricardo Villalobos - Theogenese (Alcachofa)

IanB

11th May 2005, 10:41

Originally posted by neuron2
Why can't you just make the default be 8-byte chroma alignment with the new mechanism? Then old stuff would still work but you could ask for 16-byte chroma alignment if you want it. Leak has got it in one. ;)

8 byte chroma, 16 byte luma is the current way. The future way want to be 16 byte chroma, 32 byte luma.

Some history
In Kassandro request he wanted most child filters returning mod 16 aligned chroma planes and I agree with him it is a worthwhile enhancement. My first thought was to just realign the chroma planes but all the baked in code enforceing chroma must be half luma in avisynth.h stopped that idea cold, a full recompile of the world is not practical and I don't want to be lynched. Next thought was bump FRAME_ALIGN=32 in avisynth.h again the baked in nature of the constant meant somethings would still slip thru the cracks and non-planar formats would be wasting memory and squandering cache lines. As I researched the internals of AVS for a better answer I found that by shifting the definition of planar alignment from the luma plane to the chroma plane in NewVideoFrame(), Kassandro could have his cake and eat it to. There is no baked in component, no recompiling and it works straight away with a new version. Only problem is there is code out there that thinks it knows about frame buffer layout. :mad:

IanB

P.S. Like the debug build 64 byte guard regions I added recently to shake out bounds error I'm starting to think it might be a good idea in debug builds to randomize frame alignment somehow to shake out any smartalec code lurking. Thorts? :D

P.P.S. :sly: I have an idea for a script verb to set the alignment for the next filter to overcome these legacy problems, just got a small chicken before egg problem with source filters like MPEG2Source(), let me ponder upon it a while, I see some illumination.

tsp

11th May 2005, 12:02

Is there a way to get the current alignment at runtime?

so something like this softwire instruction from XtoRGB would work right( it will work anyway because RGB24 is still 16 bytes aligned right?)

cmp( edx,(width*3+15)/16*16/3*3);

IanB

11th May 2005, 13:35

(int)(frame->GetReadPtr())
frame->GetPitch(PLANAR_Y)
frame->GetPitch(PLANAR_U)Then check the bottom bits for zero.

:confused: Why check the width. The pitch is what is interesting.:confused:

This is where Mpeg2dec has gone wrong it assumes width == pitch.

To present aligned frames AVS rounds the width up to a multiple of Alignment, this is the pitch. It mallocs a memory buffer pitch*height+(a little bit) it then calculates an offset from the beginning of the buffer such that the address of buff[offset] is appropriatly aligned in memory, this address is what is returned by frame->GetReadPtr(), (a little bit) is the maximum value of offset.

IanB

tsp

11th May 2005, 17:42

IanB: Put what if the frame only contains zeros. One way to do it might be to reqeust a frame of size 1x1 and see how big it is. Also the softwire code is only called in the constructor(where the assembler code is generated) so the pitch is rather useless in the test because it can change from frame to frame but the single instruction does use the pitch instead of the width. This part calculates the minimum pitch if it's 16 Byte aligned
(width*3+15)/16*16
and the last part /3*3 rounds it down to the nearest pixel

[edit]
so the optimal code should like this:

cmp( edx,(width*3+ALIGNMENT-1)/ALIGNMENT*ALIGNMENT/3*3);

tritical

11th May 2005, 18:06

Question about setcachehints using CACHE_RANGE. If I am requesting frames that are in the range of n-4 to n+4 assuming n is the current frame... then I should call SetCacheHints with (CACHE_RANGE, 4). In the setcachehints method it doubles this to 2*4+1, but then in the GetFrame method under the CACHE_RANGE handling I see this:

// Check if it is out of scope
else if (abs(h_video_frames[i]->frame_number-n)>h_radius) {

That should be:

// Check if it is out of scope
else if (abs(h_video_frames[i]->frame_number-n)>=h_total_frames) {

shouldn't it? Let's say current frame is 16 and so I start by requesting 12. Then with >h_radius it deletes cached frames 17, 18, and 19, which I am about to request. And when I go up to requesting frames 16+ it starts dropping frames off the other end! That doesn't seem to be correct.

I changed it to >=h_total_frames and tested, it made a major difference in speed :).

IanB

12th May 2005, 09:56

@tsp,

I am a little confused as to what you are actually asking so I'll answer both ways. If I still have it wrong PM me to set me right.

1a. For an input frame the worst alignment you can get will be a minimum pixel unit; 32bits for RGB32 & YUY2, 24bits for RGB24, 16bits for YV12 luma & 8 bits for YV12 chroma; This can be caused by say a 1 unit left Crop().

1b. Also for input frames the pitch can be many times the width. Again with a little help from our friend say Crop(most_of_the_width).

2a. For Env->NewVideoFrame(vi, align), for "align" >= 0 this will provide a guaranteed pitch aligned >= "align" Note! >= i.e. Ask for 4 get 16, ask for 32 get 32. And default for "align" is FRAME_ALIGN (currently 16)

2b. However if "align" is < 0 the pitch is force aligned to "-align" i.e. "align"=-1 pitch==rowsize, "align"=-3 pitch==rowsize+2/3*3. Rowsize is Width*BytesPerPixel. And it is generally unfriendly to force wierd alignments (but RGB24 is already unfriendly)

@tritical,

Originally posted by tritical
// Check if it is out of scope
else if (abs(h_video_frames[i]->frame_number-n)>h_radius) {

That should be:

// Check if it is out of scope
else if (abs(h_video_frames[i]->frame_number-n)>=h_total_frames) {

shouldn't it?Yeap it's wrong.

I am halfway thru reviewing the cache, I have done a slight pretidy up and built and installed some infrastructure I will need. I just need a large lump of time and to psyche myself up for it. Currently CACHE_RANGE stops tracking frames that go out of scope so even if the frame hasn't been reused yet it can't be found.

IanB

Backwoods

14th May 2005, 02:22

In the current beta, this is happening:

http://forum.doom9.org/showthread.php?s=&threadid=90042&perpage=&pagenumber=2

function TimeDenoise(clip clip, float "strength", bool "interlaced"){
strength = default(strength, 1.0)
interlaced = default(interlaced, true)

order = (clip.getparity == true) ? 1 : 0

clip = (interlaced == true) ? clip.ConvertToYV12(interlaced = true) : clip.ConvertToYV12()
clip = (interlaced == true) ? clip.TDeint(mode=1,order=order,type=2) : clip

clip = clip.FFT3DFilter(sigma = strength / 2, bt = 2, bw = 48, bh = 48)

backward_vectors4d = clip.MvTools_MVAnalyse(blksize = 16, isb = true, lambda = 1000, delta = 4, idx = 1, pel = 2)
backward_vectors3d = clip.MvTools_MVAnalyse(blksize = 16, isb = true, lambda = 1000, delta = 3, idx = 1, pel = 2)
backward_vectors2d = clip.MvTools_MVAnalyse(blksize = 16, isb = true, lambda = 1000, delta = 2, idx = 1, pel = 2)
backward_vectors1d = clip.MvTools_MVAnalyse(blksize = 16, isb = true, lambda = 1000, delta = 1, idx = 1, pel = 2)
forward_vectors1d = clip.MvTools_MVAnalyse(blksize = 16, isb = false, lambda = 1000, delta = 1, idx = 1, pel = 2)
forward_vectors2d = clip.MvTools_MVAnalyse(blksize = 16, isb = false, lambda = 1000, delta = 2, idx = 1, pel = 2)
forward_vectors3d = clip.MvTools_MVAnalyse(blksize = 16, isb = false, lambda = 1000, delta = 3, idx = 1, pel = 2)
forward_vectors4d = clip.MvTools_MVAnalyse(blksize = 16, isb = false, lambda = 1000, delta = 4, idx = 1, pel = 2)
clip = clip.MvTools_MVDenoise(backward_vectors4d, backward_vectors3d, backward_vectors2d, backward_vectors1d,
\ forward_vectors1d, forward_vectors2d, forward_vectors3d, forward_vectors4d, tht = int(strength * 6),
\ thsad = int(strength * 800), thSCD1 = int(strength * 1000))

clip = clip.ConvertToYUY2()
clip = (interlaced == true) ? clip.separatefields().selectevery(4,0,3) : clip
clip = clip.VagueDenoiser(threshold = strength, wavelet = 3, method=1, nsteps=16 ,chromaT=0.8, Wiener = false,
\ percent = 80, interlaced = false)
clip = clip.LimitedSharpen(ss_x = 2, ss_y = 2, strength = 255, wide = true)
clip = clip.Unfilter(int(strength * 4), int(strength * 4))
clip = (interlaced == true) ? clip.Weave() : clip

return clip
}

This function is no longer working.

Wilbert

14th May 2005, 13:33

You know the drill. Post the most simple script which is not working.

IanB

14th May 2005, 14:05

Dear All,

I've just commited changes into CVS to add a SetPlanarLegacyAlignment(clip, bool)] function. It attaches itself to the child->GetFrame() chain so it works backwards up your script, i.e. you put it after the plugin that is making illegal assumptions about memory layout.
Mpeg2Source("test.d2v")
SetPlanarLegacyAlignment(True)
SuperSSE2ChromaFilter()
OtherSSE2AwareFilter()
SetPlanarLegacyAlignment(False)
BlindPP()
SetPlanarLegacyAlignment(True)

@Backwoods,

Try this when a build becomes available.

IanB

Guest

14th May 2005, 15:10

OMG, you've got to be kidding. You know people think Avisynth is hard to use already. Wait until average users have to understand planar alignment.

Fizick

14th May 2005, 20:19

IanB,
Probably in this case you must use different greeting, not:
Dear All,

but:
Dear neuron2
:D

Backwoods

14th May 2005, 22:01

Originally posted by Wilbert
You know the drill. Post the most simple script which is not working.

I posted the function which was not working, which I figured would be enough since the script is just calling on the function. The script was also in the link. Either way, here is the script and I will test it again once a new build is up.

LoadPlugin("C:\Program Files\AviSynth 2.5\yea\fft3dfilter.dll")
LoadPlugin("C:\Program Files\AviSynth 2.5\yea\mvtools.dll")
LoadPlugin("C:\Program Files\AviSynth 2.5\yea\unfilter.dll")
LoadPlugin("C:\Program Files\AviSynth 2.5\yea\Tdeint.dll")
LoadPlugin("C:\Program Files\AviSynth 2.5\yea\MaskTools.dll")
LoadPlugin("C:\Program Files\AviSynth 2.5\yea\VagueDenoiser.dll")
Import("C:\Program Files\AviSynth 2.5\yea\LimitedSharpen.avs")
Import("C:\Program Files\AviSynth 2.5\yea\TimeDenoise.avs")

AVISource("otto.avi")
TimeDenoise(1.0,interlaced=false)

Wilbert

14th May 2005, 23:06

most simple script ...

So, start with TDeint, FFT3DFilter, MvTools_MVAnalyse, separately ...

Backwoods

14th May 2005, 23:36

Originally posted by Wilbert
most simple

So, start with TDeint, FFT3DFilter, MvTools_MVAnalyse, seperately ...

UnFilter and MvTools_MVAnalyse make the test script crash.

AVISource("otto.avi")
MvTools_MVAnalyse(blksize = 16, isb = true, lambda = 1000, delta = 4, idx = 1, pel = 2)
AVISource("otto.avi")
UnFilter(HSharp, VSharp)

Guest

15th May 2005, 00:40

Originally posted by Fizick
IanB,
Probably in this case you must use different greeting, not:
Dear All,

but:
Dear neuron2
:D I don't get it. Please explain.

IanB

15th May 2005, 02:57

@Neuron2,

Don, I think Fizick, might be tugging our chains, just a little :p
you might be petulant and I might be condesending or is it the other way round:D :D :D :D :D

OMG, you've got to be kidding...

I am, in reality I expect that most people still in need of legacy alignment to just stick one SetPlanarLegacyAlignment(true) at the end of their script and say dang it. I also hope pluggin's with problems are repaired by the time 2.5.6 is finished.

I was just demonstrating how you can turn it off and on throughout a script and that it is not a simple global switch.

IanB

Guest

15th May 2005, 03:00

@IanB

Got it, thank you. :cool:

Fizick

15th May 2005, 10:37

neuron2,
I am sorry for too short post.
I tryed to tell a joke (almost ;) ), but my English is not so good, and it was late of night here.

Now i try write longer.

I really wanted to say that:
1. IanB really makes the things hardly understandable to wide public.
2. Probably you are one of a few who could fully understand it.
3. But you could be more kind to his innovations (in beta version).
May be suggest some solutions. (I can not do it due to lack of serious programming experience).

So, I was (and I am) for world peace. :)

IanB,
thanks for explanation. :)
I really want to say, that you could be more respectual to Neuron2,
and then he will permit you to do anything you want. :)
Sorry for offtopic.
Now I have a real question. Currently we try to translate Avisynth doc to Russian language.
IanB, I see your recent update of Avisynth doc on CVS. Is the current CVS documentation up-to-date with your latest Avisynth changes?
When you suppose v.2.5.6 will be released? We want to finish the translation to that time and put it to installation package.

Guest

15th May 2005, 14:41

Originally posted by Fizick
I really want to say, that you could be more respectual to Neuron2,
and then he will permit you to do anything you want. :)
Please don't speak for me. I feel quite adequately respected by IanB.

Don't impute motives to the forthright expression of opinions. Speaking one's mind about technical issues does not constitute a lack of respect.

Wilbert

15th May 2005, 15:07

UnFilter and MvTools_MVAnalyse make the test script crash.

AVISource("otto.avi")
UnFilter(HSharp, VSharp)

AVISource("otto.avi")
MvTools_MVAnalyse(blksize = 16, isb = true, lambda = 1000, delta = 4, idx = 1, pel = 2)

Unfilter: you did substitute numbers for HSharp and VSharp right (at least in the short script above)? You also checked with AviSource("otto.avi").Info returns indeed YV12 (needed in both scripts)? So, try

AVISource("otto.avi")
ConvertToYV12()
UnFilter(4, 4)

MVAnalyse returns a bunch of numbers, so that's why it fails. Try

source = AVISource("otto.avi").ConvertToYV12()
vectors = source.MVAnalyse(isb = false)
source.MVShow(vectors)

If that works try

AVISource("otto.avi")
ConvertToYV12()
FFT3DFilter(sigma=3)

Backwoods

15th May 2005, 22:24

Ok, for one thing, I'm an idiot. I thought my AVI was already YV12 with Langarith but it was YUY2.

works:
AVISource("otto.avi")
ConvertToYV12()
FFT3DFilter(sigma=3)

works(but wouldn't the YUY2 AVI work too? I thought the new DLL supported YUY2, either way):
AVISource("otto.avi")
ConvertToYV12()
UnFilter(4, 4)

works:
source = AVISource("otto.avi").ConvertToYV12()
vectors = source.MVAnalyse(isb = false)
source.MVShow(vectors)

TimeDeNoise with ConvertToYV12() still does not work and shouldn't I be getting an error as to why it crashes instead of just a close out from VirtualDub?

Fizick

16th May 2005, 04:55

Backwoods,
please re-post your problem with fft3dfilter (if any) to its thread with more detail.
http://forum.doom9.org/showthread.php?s=&threadid=85790&highlight=fft3dfilter

sh0dan

16th May 2005, 11:38

Looking at this thread (http://forum.doom9.org/showthread.php?s=&threadid=94184) I seem to get worried.

I think messing with pitches is a generally bad idea. You create a bunch of problems.

* Initial byte alignment cannot be guarranteed to be 16 byte aligned, unless every crop is bitblitted. This is required for fast SSE2.

* SSE2 is not so widespead that you don't need to have fallback anyway.

* Only 20-40% of all images will not have 16 byte aligned chroma. (Rough estimate)

* Chroma data isn't that much data.

* Only planar data is currently guaranteed to have this alignment.

* SSE2 filters could just create their aligned frames and do the bitblit themselves. The blit will just be placed elsewhere - it's inavoidable in most cases (MPEG2, AVI, DSS input).

* Memory footprint will unnecessarily (for 95% of all filters) be slightly larger.

kassandro

16th May 2005, 13:31

Originally posted by sh0dan
Looking at this thread (http://forum.doom9.org/showthread.php?s=&threadid=94184) I seem to get worried.

Well, in this code the assumption "width=pitch" was made and you never should do this. Unfortunately, this problems seems to stem from mpeg2dec3, which I still have to use, because I use mostly .d2v files created by ProjectX. Unfortunately in DGDecode a cheap fix was done by replacing the new default alignment with the old one, i.e. NewVideoFrame(vi, -16). That may easily lead to an inhomogenuous pitch (the pitch may change from frame to frame), which slows down some filter like my RemoveDirt quite a bit and may easily crash quite a few filters, which tacitly make the assumption of homogeneous pitch.

* Initial byte alignment cannot be guarranteed to be 16 byte aligned, unless every crop is bitblitted. This is required for fast SSE2.

I don't understand this at all.

* SSE2 is not so widespead that you don't need to have fallback anyway.

For more than 2 years Intel produces only SSE2 capable chips and AMD sole non-SSE2 compatible chips are at the very low end. Probably, in 2006 no CPU will be sold, which is not SSE2 compatible.

* Only 20-40% of all images will not have 16 byte aligned chroma. (Rough estimate)

This is an extremely rough estimate! For me almost 100% of all frame would have no 16 byte aligned chroma. I always start with 720x576 YV12 frames and crop at the very end. Thus if each filter would use the default NewVideoFrame(vi), frames will never have a 16 byte aligned chroma.

* Chroma data isn't that much data.

After all its 50% of all data and later on you complain about wasting a tiny little bit of memory with the improved alignment.

* Only planar data is currently guaranteed to have this alignment.

because we only need it there.

* SSE2 filters could just create their aligned frames and do the bitblit themselves. The blit will just be placed elsewhere - it's inavoidable in most cases (MPEG2, AVI, DSS input).

As I already mentioned above, this would easily lead to an inhomogoenoeus pitch. On the other hand, if all filter would creat their frames with the default NewVideoFrame(vi) then we would always have a homogeneous pitch unless we use Crop or SeperateFields. As I rarely use SeperateFields and nearly always crop at the end of the script, I would always have a homogeneous pitch, if each filter uses the default alignment. Actually, I would never allow another alignment. Then the "dirty programming problem" in DGDecode/Mpeg2dec3 and in other filters would have to be fixed properly.

Guest

16th May 2005, 13:41

Originally posted by kassandro
Unfortunately in DGDecode a cheap fix was done This guy likes to try to hammer me at every opportunity, because he imagines I have wronged his buddy 'mf'. It's getting tiresome.

I did not make any fix in DGDecode. I made available an experimental version to *test* operation under the new beta. I even named it DGDecodeX so that nobody would confuse it with a sanctioned version. Tests such as this help to expose all the issues that we would have to address if this alignment change goes forward.

@sh0dan

The -8 sets up 8-alignment on the chroma and 16-alignment on the luma under the new beta.

kassandro

16th May 2005, 14:35

Originally posted by neuron2
This guy likes to try to hammer me at every opportunity, because he imagines I have wronged his buddy 'mf'. It's getting tiresome.

This has really nothing to do with this old story. You are very easy to hammer. The not yet prohibited word "cheap" seems to be enough.

I have already mentioned one drawback of changing the default alignment: inhomogeneous pitch. There is a second one. Make an MPEG2 encoding of a video with width 712 or any other width not divisible by 16. Then, even with the old 2.5.5 alignment, you won't have pitch=width for the luma and probably have the same troubles. The "expensive" fix would be to remove the pitch=width assumption at all places in the source code. You will probably agree, that your fix of enforcing pitch=width is very cheap compared such an expensive fix, but in my opinion the expensive fix is the only one, with which one can really sleep well.

sh0dan

16th May 2005, 16:58

Originally posted by kassandro

* Initial byte alignment cannot be guarranteed to be 16 byte aligned, unless every crop is bitblitted. This is required for fast SSE2.
I don't understand this at all.

To be able to use MOVDQA, the load adress must be 16 byte aligned.
Besides, adding a "process the remaining 8 bytes, if there are any" after your loop shouldn't be a problem. You could even do it in C for that matter.

For more than 2 years Intel produces only SSE2 capable chips and AMD sole non-SSE2 compatible chips are at the very low end. Probably, in 2006 no CPU will be sold, which is not SSE2 compatible.

Not really as relevant, as what's out there. ISSE could be acceptable minimum now, but SSE2 - no way.

Then the "dirty programming problem" in DGDecode/Mpeg2dec3 and in other filters would have to be fixed properly.
I'm talking about end-user-perspective, not programmer-perspective.

Making thousands of already installed MPEG2DEC3/DgDecode explode when users install 2.56 is NOT the way to go, IMO. We've had enough of (unintentional) problems, with filters breaking because of an AviSynth update, and I don't think breaking the most popular is going to help anyone.

sh0dan

16th May 2005, 17:06

Originally posted by neuron2

The -8 sets up 8-alignment on the chroma and 16-alignment on the luma under the new beta.
@IanB: If this is true, you need to assure that AviSource and DSS behaves corectly. Both of these create VideoFrames that emulate the alignment which both of these are supposed to give. Do a search for "NewVideoFrame(" in the sources.

kassandro

16th May 2005, 18:31

Originally posted by sh0dan

To be able to use MOVDQA, the load adress must be 16 byte aligned.
Besides, adding a "process the remaining 8 bytes, if there are any" after your loop shouldn't be a problem. You could even do it in C for that matter.
[/QUOTE]
I now understand, what you meant. If you want to use MOVDQA always, you must bitblt if you crop the left side by an amount, which is not a multiple of 16. I didn't even think of this possibility and therefore I always check alignment and use an unaligned routine if necessary. As croping on the left side should only be done once and usually at the end, where you can also avoid the bitblt with a special option. Vertical cropping may be used earlier, to remove big black bars at the top and the bottom, but there no bitblt is necessary. Thus your idea makes a lot of sense.
The advantage of 16 byte alignment goes well beyond MOVDQA. For instance, if esi, edi, ebx are multitples of 16 you can do

__asm movdqa xmm0, [esi]
__asm pavgb xmm0, [ebx]
__asm movdqa [edi], xmm0

If esi, edi, ebx are not multitples of 16 you must do

__asm movdqu xmm0, [esi]
__asm movdqu xmm1, [ebx]
__asm pavgb xmm0, xmm1
__asm movdqu [edi], xmm0

Not really as relevant, as what's out there. ISSE could be acceptable minimum now, but SSE2 - no way.

CPUs without SSE2 are not effected by the alignment changes at all.

Making thousands of already installed MPEG2DEC3/DgDecode explode when users install 2.56 is NOT the way to go, IMO. We've had enough of (unintentional) problems, with filters breaking because of an AviSynth update, and I don't think breaking the most popular is going to help anyone.
There is nothing intrinsically bad about raising the alignment. In fact, the 2.5.6 beta did exhibit a programming bug in MPEG2DEC3/DgDecode, which of course must be fixed before 2.5.6 is released. As I pointed, this bug is already there with the old alignment, if the widths of the mpeg2 video is 712. If we would now return to the old alignment we simply would surrender to lousy programming. When we move to 64 bit, we certainly want the chroma to be better aligned. It is therefore better to have these troubles now than later, when we have a lot of other troubles as well.

EDIT: It must be a multiple of 32 - not 16 !