VideoFrame should provide its color format

[Enyium]

A VideoFrame should bring with it the necessary data to handle it. You can already query its width (advancement- and validity-wise) as well as height. You should also be able to query the information what exactly the bytes describe, i.e., the color format.

Often, you also have access to the IClip to get the color format from. But when retrieving a VideoFrame with env->propGetFrame(), there is no defined relationship to an IClip.

Not being able to get the associated color format when getting a frame out of a frame prop is a problem for my Rust crate.

[pinterf]

True. Actual format is a higher level information, VideoFrame is not capable to store that info. I'm not seeing a compatible way to do that while still keeping VideoInfo pixel_type in a parallel way. You must probably stuff the format info into a frame property as well.

[Enyium]

Why would VideoFrame not be capable to store that info? Just add another private field and a public getter.

I'm not seeing a compatible way to do that while still keeping VideoInfo pixel_type in a parallel way.

What do you mean by compatible and parallel? Where would be the problem if VideoFrame received a copy of the associated IClip's VideoInfo's pixel_type? Could the VideoFrame's color format ever change, once instantiated? (We're not talking about VideoFrameBuffer, which is recycled.)

[pinterf]

Unfornately there are cases where the same VideoFrame is changing its pixel_type format.
Personally, I consider this habit as an abuse, though technically possible. Within a complex filter chain replacing these with a proper frame copy is possibly not slowing down the process.

Cases which I remember of:

• ConvertToDoubleWidth and ConvertFromDoubleWidth (external AviSynth+ filter) which does nothing just overrides the VideoInfo format. This one even changes the bit depth as well.
• CombinePlanes special case: 4:4:4 YUV(A) and planar RGB(A) formats can be converted into each other by a simple "typecast", changing the VideoInfo pixel_type. Since the frame property alteration (RGB cannot have YUV specific frame properties) there is already a MakeProperyWritable which makes a necessary copy as a side effect, this part can also be rewritten to a proper frame copy with NewVideoFrame with the new pixel_type.
• ConvertBits: I think there is a special case when using "true_range" parameter one can simply convert a YUV420P10 into YUV420P16 by similar typecast, without any data conversion (I don't think this feature is used by anybody, it was put into action in the very early high-bit-depth development phase, when there 10 bit data in 16 bit contained could occur. It must be very rare, I'm happily sacrifice this typecast-instead-of-copy speed gain and make a proper plane copy with the new video format.

The other cases:

• format is changed when calling env->SubFramePlanar on a format with alpha plane.
  See in PVideoFrame RemoveAlphaPlane::GetFrame(int n, IScriptEnvironment* env)
  You must convert the underlying VideoFrame pixel_type to the alpha-less format (e.g. YUVA420P16 to YUV420P16) within SubFramePlanar

• format is changed when calling env->SubFrame on a format with more that one plane.
  Tipically this is a simple plane extraction for planar formats and result in an Y8..Y32 format, keeping the original bit depth.
  You must convert the underlying VideoFrame pixel_type to the alpha-less format e.g. (YUV444P8 to Y8, RGBPS to Y32) within SubFrame.

Other notices:

• C interface AVS_VideoFrame must be extended according the new field
• (a request), please try to separate syntax and editor autoformat changes from the actual content change, sometimes it's hard to revise what has been really changed
• (a request) you don't necessarily use the new DEFAULT_PLANE enum instead of nothing, historically this is how we use it. E.g. packed RGB and Y are simply more readable (and consistent across plugins) when you see frame->GetRowSize() instead of frame->GetRowSize(DEFAULT_PLANE)

With these remarks, it probably will break nothing and can be kept pixel_type of VideoInfo consistent with of VideoFrame.
I still have to go through a lot of plugin sources, whether they are affected or not.

[Enyium]

(This discussion should've been in the PR.)

I complied with every point.

CombinePlanes... there is already a MakeProperyWritable which makes a necessary copy as a side effect

CombinePlanes hat a change to be made, but I didn't find a MakeProperyWritable() call there.

env->SubFramePlanar... env->SubFrame

These functions themselves weren't the best places to change this behavior. I did it in the VideoFrame::Subframe() overloads.

I still have to go through a lot of plugin sources, whether they are affected or not.

Only for those not in the codebase, if my approach to change plugins was sufficient:

• I searched for the regex (\.|->)pixel_type ?=(?!=) in the whole codebase.
• For every filter with an occurence, I went to its GetFrame() method.
• I highlighted return and checked the PVideoFrames whether they needed an AmendPixelType() and an accompanying MakeWritable().

[pinterf]

My bad. Error in commit 1b70e45 text. Not #317 but #337.