pinterf / masktools

MaskTools v2 fork
Other
48 stars 11 forks source link

mt_merge no error but just crash #16

Closed TbtBI closed 3 years ago

TbtBI commented 3 years ago

Hi,

When mask clip has only one plane and mt_merge(x, y, z, u=3,v=3) is used there is no error that mask should have chroma planes for u=v=3 but mt_merge just crash.

realfinder commented 3 years ago

this work fine for me

ColorBars(width=640, height=480, pixel_type="yv12")
converttoy8
mt_merge(last, last, last, u=3,v=3)
pinterf commented 3 years ago

Well, seeing the code, it does not crash when the input clip is greyscale.

     // PF: for greyscale: graceful fallback to chromaless operation
      if (plane_counts[C] == 1) {
        use_luma = false;
        operators[1] = operators[2] = operators[3] = NONE;
      }

When the mask clip is greyscale, input clip is not greyscale and luma is false, well, this case is not covered.

pinterf commented 3 years ago

What should be the proper behaviour if luma=false (default) but only a greyscale mask is provided? I suppose it should act as luma=true automatically. EDIT: no. Because - as a rule - luma=true implies chroma processing as well, and this may not be what we want. Next proposal: when providing a greyscale mask + clips are not greyscale + luma==false, then error message is thrown if mode u or v is set to 'process', EDIT2: what about this one: Error: "mask clip is greyscale and 'u' or 'v' or 'a' processing is set with luma=false"

realfinder commented 3 years ago

Next proposal: when providing a greyscale mask + clips are not greyscale + luma==false, then error message is thrown if mode u or v is set to 'process', EDIT2: what about this one: Error: "mask clip is greyscale and 'u' or 'v' or 'a' processing is set with luma=false"

seems ok

off-topic but can you please take a look at the last 2 issues since about 2 months here? https://github.com/pinterf/TIVTC/issues

pinterf commented 3 years ago

off - sorry, no time for those in the near future

TbtBI commented 3 years ago

EDIT2: what about this one: Error: "mask clip is greyscale and 'u' or 'v' or 'a' processing is set with luma=false"

It looks ok to me too.

pinterf commented 3 years ago

O.K., fixed in https://github.com/pinterf/masktools/commit/041146640e6a0f3a547f8237d74b4c7a34f40834

TbtBI commented 3 years ago

Thanks.