The scaler code only checks whether or not the FBOFMT is storable,
but in the case of e.g. draw_overlays, we have to check the actual FBO
(provided by the user) instead.

Solve this by just checking both. (Note: In theory, if the FBO is
storable but FBOFMT is not, this would be a false negative. But
whatever.)

This is a major overhaul of the target rendering code, and the renderer
code in general (to keep things organized and understandable).

The motivation comes from vf_libplacebo, which wants to let users render
out subsampled YCbCr content directly. Unfortunately, we're still pretty
much forced to upconvert to RGB internally, if only for the sake of
things like tone mapping, linear scaling, and so forth. Plus, the entire
pl_renderer architecture is very much designed around the idea that
there's only one plane currently being rendered. While it's not entirely
impossible to change this, it'd be _way_ outside the scope of this
already massive change.

Due to the major change of a core API, I decided to make the old `pl_tex
*fbo` interface merely deprecated, rather than removing it entirely. Due
to the increasing unification between `pl_image` and `pl_render_target`,
I may choose to merge these into a single `pl_frame` at some point in
the future, but that would also imply dropping this bit of backwards
compatibility, so I'll postpone that refactor until after the next major
release.

Pick up a few bugs related to the dithering/compute indirection code
while we're at it.

Added a new test case to make sure it round trips, even for oddly-sized
planes.

Turns out we also need to clamp negative values, specifically for the PQ
function. I'm not sure if both clamps are strictly needed, but I guess
it's better to be safe than sorry.

We can go all the way back to 55.74 by dropping our reliance on
AVCOL_PRI_EBU3213. Fortunately, this is not needed, because the
backwards compatible name AVCOL_PRI_JEDEC_P22 is identical to it.

This includes libavutil-dev, so we can run the libav test.

I have no idea how this is even possible, given that sig is already
clamped to be smaller than sig_peak, but whatever. This brute-force
fixes the issue.

This replaces `int components` by a new `uint8_t component_mask`, which
is more flexible. I decided not to deprecate the original field, because
it doesn't really cause any harm to continue supporting, and in most
cases you don't need the flexibility provided by a mask, so it's just a
hassle.

Made for the sake of allowing us to more efficiently render sub-sampled
planes without needing to waste GPU time scaling unnecessary components
like luma.

The old code tried printf'ing a small float using %f, which resulted in
a rounded literal that was quite far from 1/scale. This led to the image
getting brighter than it should be, globally. The effect was very
significant for higher bit depths.

Fixed this, and improved the precision, by printing exact integer
literals, which get turned into exact float literals by the shader
compiler.

I never seem to get a break with this logic. Anyway, what happened
*this* time was that I noticed `rx`, `ry` were being incorrectly
calculated (they're supposed to be floats, but I was dividing two ints).

I also realized while working through a paper example that using `rx`
for the plane shifts was incorrect to begin with. Fortunately, for most
real-world content, `rx` and `rrx` are identical. For cases where they
aren't, the earlier answer ended up effectively cancelling out this
error, at the cost of an off-by-one (resulting in tiny, probably
unnoticed chroma misalignment).

The correct logic is to use rrx/rry for both. Fixed, and tested. (In
particular, by making sure a YCbCr image survives being converted to
RGB and back to YCbCr by the renderer)

This is technically a hack, or at the very least, some sort of
ID/UD-type behaviour. It works in practice (for good reasons), but
juuust in case, it would really be a smart idea to allow disabling it.

Also swap the order of the error messages to make sure checks happen in
a more sensible order.

After having managed to contrive in my head a scenario in which the luma
plane somehow ends up with luma in an index other than 0 (uploading UYVY
to an rgba8 texture and aliasing it as rg8, the luma information is now
in the G channel), I realized it's a good idea to support this.

It's not really a horrendeous break of API, because the backwards
compatible behaviour is, handily, what you get if you just ignore this
new field's existence.

We load this, but forgot to add it to the recommended_extensions list.

This is a 3DLUT, not a 4DLUT. The alpha channel doesn't exist,
practically.

The av_pix_fmt_desc_get can return NULL which would cause a
NULL-pointer dereference for the has_alpha assignment.

This shader isn't merged with anything else, so no point in forcing it
to be unique. This avoids a double-compilation for planar chroma.

Some API users might erroneously create textures with usage modes that
can't actually be reasonably used, either because the libplacebo
capability is blacklisted, or because of specific cases like storage
support being masked due to lack of unformatted shader storage.

Inlining all of the functions suppresses -Wunused-function, and the
int cast suppresses one -Wsign-compare.

This makes the header "safe" to compile under -Wall -Wextra.

As the comments at the top say, this is extremely shitty and rushed.
Proper path for refactors:

- Split up into separate files, modularize the GL/VK components and load
  the correct one at runtime.

- Split up rendering onto a separate rendering thread, make it take into
  account timing etc. properly.

- Move it into its own directory structure and give it a meson.build.

Once this is done, we could use this as a test-bed for testing e.g.
interpolation, and finally get that patchset moving along.

An array of pointers to pointers? Complete rubbish and overkill. An
array of pointers is fine. So what if users have to group their plane
textures together, big deal. It's way more convenient this way, than
having to explicitly separate the plane texture by plane and somehow
provide them separately as an array.

Not even gonna bump the API version for this because 1) nobody uses this
API yet, and 2) it's single-header garbage anyway, so no risk of ABI
break.

These structs and enums are designed in not-quite-compatible ways, and
the mapping is not at all intuitive or obvious. This should hopefully
make life a billion times easier for people wanting to use libplacebo
together with FFmpeg/libav* (i.e., basically the default use case).

Also includes an ultra-high-level wrapper that just takes an AVFrame *
and straight up uploads it to the GPU, in `pl_image` format, ready to be
consumed by `pl_renderer`.

This entire set of helpers is done as a single-header library to avoid
depending on libavutil, which would be a *really* terrible idea since
libavfilter also wants to depend on libplacebo. This single-header
nonsense is the easiest way to avoid a cyclic dependency. I promise I
won't make it a habit. Besides, it's annoying having to do things this
way, because it means I can't re-use any of the fancy logging functions
from pl_ctx. Oh well. I hope the user knows what they're doing.

Add these for alignment with the AVColorPrimaries enum. New primaries
are cheap and trivial to add, since they only need the (straightforward)
definition of their Yxy coordinates.

To avoid bugs like the previous missing <stddef.h>, add a set of
brute-force compile tests to check that all installed headers compile.

These are needed to be able to include these headers in isolation
without build failures.

Oops. It's a small miracle this managed to pass the round trip test
inside `tests/colorspace.c`. Maybe we should also include some sort of
test to make sure the values aren't too far out of the range we'd
expect?

Niklas Haas authored 4 years ago and Niklas Haas committed 4 years ago

This allows the use of uniform buffers and storage buffers,
respectively. The interface is similar to //!TEXTURE. Currently, the
only supported directive is //!STORAGE, which marks it as a readwrite
storage buffer, and //!VAR <type> <name> which declares a variable
(similar to writing `type name;` in GLSL).

Also add a rudimentary test case, that doesn't actually test anything.

Closes #110

Niklas Haas authored 4 years ago and Niklas Haas committed 4 years ago

This allows shaders to have persistent state across images. We
piggy-back the storage texture support off the existing texture support,
by simply adding a new directive //!STORAGE which marks the image as
a storage image. (It's still bound using the existing //!BIND, which now
simply causes the resulting texture to get bound as an image2D instead)

This still has the limitation that the texture size needs to be static,
whereas ideally it would be cool if we could specify the storage size as
an RPN expression, but it's an easy way to implement what we want. (And
in practice, you can just make the texture larger than it needs to be)

Partially implements #110

Niklas Haas authored 4 years ago and Niklas Haas committed 4 years ago

This function was annoying and served no real purpose, because it lives
in the same file as the only place that needs it. It's a left-over
design quirk from mpv.

In addition to deleting this function, the custom_shader_tex has been
replaced by the more generic pl_shader_desc, which allows us to create
and attach more resource types than just textures, via the same
mechanism.

This commit is non-functional, it's a pure refactor.

In particular, this fixes compile failures on the version shipped by
ubuntu 20.04.

I have no idea why this fails, but the error message makes no sense. It
doesn't appear to be an error on our side either, EGL_BAD_ALLOC implies
some sort of internal driver failure or OOM condition, which makes zero
sense since running malloc() ourselves works fine.

I give up. I'm just blacklisting this test from the CI instance.

Printing a human-readable string here is infinitely better than printing
a 0x%x.

Note that this switch/case doesn't technically cover all the
possibilities under the sun (as implied by e.g. extensions), but I
couldn't be bothered enumerating error codes for extensions we don't use
anyways.

Instead of dumping the raw error code.

It can be a bit hard to find the right place in the log otherwise.

This can help make errors clearer, by allowing us to access the relevant
error message string.

Oops.

As pointed out by @ValZapod, this now differs from the matrix for
BT.2100 PQ in the second revision of BT.2100.

I computed the exact inverse using WolframAlpha for the same reason as
given by 1533f7c0.

This generates a PL_WARN message, which is ugly and unnecessary.