AviSynth / AviSynthPlus

AviSynth with improvements
http://avs-plus.net
962 stars 73 forks source link

ShowY/U/V behavior #258

Open Reel-Deal opened 2 years ago

Reel-Deal commented 2 years ago

Hi again,

So I was updating the rst docs for ShowY/U/V and I always like to try it out to make sure the docs match the actual behavior. It seems there are some inconsistencies with the behavior of ShowY/U/V and the docs.

http://avisynth.nl/index.php/Extract


ShowY(clip, string pixel_type)
ShowU(clip, string pixel_type)
ShowV(clip, string pixel_type)

    clip   =

        Source clip. Accepts 8-16 integer bit depths, but not Float. 
        If specified plane does not exist, an error is raised. 

    string  pixel_type = (same as clip)

        Set color format of the returned clip. 
        Cannot convert bit depths or YUV↔RGB, but can convert from 4:4:4 to 4:2:0 etc. 

1st problem, the default for pixel_type is RGB32 or RGB64, as shown in layer.cpp#L779, not the same color format as the input clip.

2nd problem, it says that it accepts 8-16 bit clips, but that is not the case since pixel_type defaults to RGB and only 8 or 16-bit clips are allowed unless pixel_type is explicitly set to the correct YUV format. Only then are 8 through 16-bit clips allowed.

Here's the script I use to test the behavior:

ColorbarsHD()
ConvertBits(10) # omit this line and pixel_type and the resulting clip is rgb
ShowY(pixel_type="YUVP10") # if you omit pixel type, you get an error

So my question is, are the docs wrong or is the behavior wrong?

pinterf commented 2 years ago

How it behaved in classic Avisynth? I suppose originally the default output was RGB32.

It seems I wanted to keep the logic that the output must be a packed RGB format. But only RGB64 existed.

This choice unfortunately eliminated the 10-14 bit and 32 bit float variants.

A possible solution would either be incompatible (all goes to planar RGB including 8 bit) or inconsistent (8 bit to RGB32 all others to planar RGB)

Reel-Deal commented 2 years ago

There is no ShowY/U/V in classic avs. Only ShowRed,Green,Blue,Alpha. And since the format was already RGB the default pixel_type was "RGB32", but it could be overwritten by choosing a YUV format as the pixel type, the extracted plane goes to Y and the chroma set to neutral. I think that's the reason Raffriff42 wrote the documentation for ShowY/U/V as it is (extract the plane and keep the same color format unless explicitly set to something else, provided that is the same bitdepth and YUV).

pinterf commented 2 years ago

Then this behaviour must be fixed: ShowY/U/V would be nice to keep the format. Obviously ShowU for a 420 format would sometimes fail: since a subsampled U or V goes into Y then the new neutral U and V planes must be valid: the Y width must fulfill the mod2 height and width rule. What happens to a 10 bit planar RGB?

Reel-Deal commented 2 years ago

It throws an error: ShowY: plane is valid only with YUV(A) source

Yeah, ShowU/V would be prone to that since the limitations of subsampled colorspaces. At least there's no problem with most common dimensions found in consumer formats.

I'll get back to you later on. It's my bedtime :)

Reel-Deal commented 2 years ago

Ok, so to recap the proposed behavior change for ShowY/U/V:

I'm sure you have a way that you want to implement it, feel free to ignore what I said. My feelings wont be hurt :)

pinterf commented 2 years ago

Sounds good.

Reel-Deal commented 2 years ago

Sounds good pinterf. Right, I meant for ShowU/V not ShowY. I agree about RGB32/64. I will wait till the next test version to finish documenting it.

I created and closed 2 issues for some previous fixes just to have as a future reference. If this is frowned upon let me know and won't do it again.

pinterf commented 2 years ago

(Regarding the generic "rgb" parameter value: once there was a request for CombinePlanes (?) to accept "RGB" or "YUV" for output format without further format and bit depth setting, do you remember it? - it was forgotten and was not implemented)

Reel-Deal commented 2 years ago

Yes I remember asking for something like that. It was a while back so I forgot my exact usage case but it was something like this:

# Convert YUV420 to half-size YUV444 without resizing chroma
function Convert420to444Test(clip input, string "cplace") 
{
    cplace = default(cplace, "mpeg2")
    Assert(cplace == "mpeg1" || cplace == "mpeg2" || cplace == "topleft", "Only mpeg1, mpeg2, topleft allowed")
    Assert(Is420(input), "Only YUV420 allowed")

    src_left = (cplace == "mpeg1") ? 0.0 : -0.50
    src_top  = (cplace == "topleft") ? -0.5 : 0.0
    Y = ExtractY(input).Spline36Resize(input.width/2, input.height/2, src_left, src_top)

    bits = BitsPerComponent(input)
    csp  = BuildPixelType(family="YUV", bits=bits, chroma=444)

    CombinePlanes(Y, input, input, planes="YUV", source_planes="YUV", pixel_type=csp)
}

I ended up using BuildPixelType to do what I wanted but I thought that it would be "friendlier" if CombinePlanes accepted YUV or RGB as a valid pixel type and then map internally to the corresponding pixel type. VapourSynth's ShufflePlanes does exactly that:

The only thing that needs to be specified is colorfamily, which controls which color family (YUV, RGB, GRAY) the output clip will be. Properties such as subsampling are determined from the relative size of the given planes to combine.

pinterf commented 2 years ago

It's a bit more difficult. I think frame properties (Matrix, ChromaLocation, maybe ColorRange?) must be deleted in the resulting clip. I do not want to check zillions of possible combinations when keeping the properties makes sense. Mostly it makes no sense. E.g. ShowU of a 420 clip targeted into an RGB or 444 clip?

Reel-Deal commented 2 years ago

I agree, the only property that may have any use is ColorRange. ChromaLocation won't matter since the chroma planes will be neutral anyways. As far as for Matrix, I hope no one would want to convert an extracted planes to RGB* since it makes no sense to do that. I have not checked what happens to the properties now when a plane is extracted onto a YUV or RGB clip.

* YUVclip.ShowU(pixel_type=YUV4xx).ConvertToRGB()

pinterf commented 2 years ago

So far all properties of source were copied. (Yesterday I finally started working on it and turned out that this ShowX family is a rather ugly copy-paste-modify-a-little part in source, I've got to simplify it first, so you have time if any request emerges)

Reel-Deal commented 2 years ago

I started to work on the docs for the ShowRed et al filters and there it also behaves exactly like the ShowYUV filters. I thought I had read that the ShowRGBA filters only worked with the interleaved formats. But that is not the case, they also accept all of the planar formats. And just like the YUV counterpart, only 8-16 bits by default since pixel_type defaults to "RGB32/64". So if the input clip is RGBP14 you will get an error unless you set the pixel_type to YUVP14 or RGBP14. I don't think any of this has been mentioned before. The ShowYUV/RGBA filters should behave similar I think, always return the same format as what the input clip is unless explicitly set to something else. And as you suggested "RGB" means PlanarRGB, if the user wants a interleaved format it must be set explicitly to RGB24/32/48/64 as appropriate with the bitdepth of the input clip.

Edit: maybe make the new default be "" - an empty string meaning the same color format as the input. With the only exception being RGB24. In classic avs the ShowRGBA filters always return RGB32 for RGB24 unless pixel_type="RGB24". This special case can be handled internally and it keeps the behavior compatible with how things were.

pinterf commented 2 years ago

I've checked the docs and it is not clear. http://avisynth.nl/index.php/ShowAlpha It says: string pixel_type = "RGB32" but really it's "RGB", not a big deal, the user does not see how it is translated internally.

Anyway the working mode after my modification:

Default pixel_type is adaptive. If none is given for pixel_type then target format is

When 'rgb' is given for pixel_type then then target format is

When 'yuv' is given (new option!) for pixel_type then then target format is

Also a new option when pixel_type is still not exact, and is given w/o bit depth. pixel_type which describes the format without bit depth is automatically extended to a valid video string constant: y, yuv420, yuv422, yuv444, yuva420, yuva422, yuva444, rgbp, rgbap e.g. 32 bit video and pixel_type 'y' will result in "Y32" 16 bit video and pixel_type 'yuv444' will result in "YUV444P16" 8 bit video and pixel_type 'rgbap' will result in "RGBAP8"

Reel-Deal commented 2 years ago

Default pixel_type is adaptive. If none is given for pixel_type then target format is

  • YUV 420 when source is Y, YUV or YUVA

What happens when the dimensions are not compatible?

Other than that, it sounds good. I will document the ShowX filters as such.

pinterf commented 2 years ago

Hmm. No reason for 420, dunno why I chose it. Let it be 444.

pinterf commented 2 years ago

Done, I hope. Can you build the project yourself or should I make a test build?

Reel-Deal commented 2 years ago

Test build please. I've never built avs before and I'm not feeling adventurous at the moment 😆

pinterf commented 2 years ago

Here you are: 3.7.2 test 4 https://drive.google.com/uc?export=download&id=1jUyiBVeb0j5uszXVSw9LtzBg2PtRNWi7

Reel-Deal commented 2 years ago

Thanks pinterf, I will test it out and get back to you if I find any bugs.

Reel-Deal commented 2 years ago

I'm finally getting around to testing the new features (I know, wrong timing).

One thing I noticed is that the alpha is not copied for YUVA/RGBAP when both input and output are alpha capable formats. For example:

ColorBars(pixel_type="YUV444P8")
AddAlphaPlane(128)
ShowY("YUVA444")
ShowAlpha() # alpha is now 255

# or

ColorBars(pixel_type="RGBP8")
AddAlphaPlane(128)
ShowRed("RGBAP") # alpha is now 255
ShowAlpha()

But works correctly with:

ColorBars(pixel_type="RGB24")
AddAlphaPlane(128) # now RGB32
ShowRed("RGBAP")
ShowAlpha()
pinterf commented 2 years ago

Then I suppose this ShowU case must be checked before, because the original alpha cannot be copied over, because new clip dimension (thus its A plane) is smaller in size than the original Alpha. (tried but not crashed, maybe I was just lucky with memory pointers :) )

ColorBars(pixel_type="YUV420P8")
AddAlphaPlane(128)
ShowU("YUVA444")
ShowAlpha() # ?? alpha cannot be copied because new clip dimension (thus its A plane) is smaller in size than the original Alpha?
Reel-Deal commented 2 years ago

Wow, I didn't even think about that. In the case that ShowU/V are used and the source and output format are alpha capable and it's not 444 then alpha should be discarded, or maybe not even copy the alpha at all for ShowU/V. Your call, I can documented either way :)

pinterf commented 2 years ago

Error message will be given. I'm preparing a test build soon. here you are 3.7.2. test13, x64 only https://drive.google.com/uc?export=download&id=1qjndY_bqGCdLyREMMfPlJLU6IMHlgZzw

Reel-Deal commented 2 years ago

Thanks, I'll keep on testing.