vo_placebo

[haasn]

All of the commits will be squashed before merging. But I wanted to get some testing/reviews in, since it's pretty much feature-complete for the most part.

Major missing components: (won't be part of this PR)

• Support for OpenGL contexts
• Support for hardware decoding
• Support for blend-subtitles=video

Options which cannot (currently) be supported due to having no libplacebo analog:

• interpolation-threshold
• tscale=linear Edit: libplacebo's rewritten interpolation logic makes tscale=linear redundant, as tscale=bilinear is already as efficient as tscale=linear could possibly be - thus invalidating its reason for existence
• scale=oversample
• --alpha=blend-tiles
• yuv420pf

libplacebo exclusive options that I still need to hook up:

• disable_builtin_scalers
• preserve_mixing_cache
• allow_delayed_peak_detect
• loading custom LUTs

Known issues:

• buffer offset crash on pixelart0.png when using DR
• missing DOCS/interface-changes.md for new option

Other TODO:

• use mp_image directly instead of the AVFrame helpers
• add support for VOCTRL_SCREENSHOT
• pass stats/timers

Some stuff could also probably be done way better. Like the source crop stuff. But it works for now. Also, as of right now this requires a pretty high version of libplacebo (>=4.130), which we should probably split up and relax to some capacity to allow vo_gpu's vulkan backend to continue working on lower versions. Edit: Done splitting up the check.

[haasn]

Bug:

• black screen after exact seek, also when starting with --pause

Edit: Fixed.

[haasn]

Frame-stepping still behaves weirdly when transitioning from a paused playback frame to a frame-stepped frame. I suspect this is partly due to how framestepping is implemented in mpv as basically pausing-and-unpausing playback, and I'm not sure what a good solution is.

[haasn]

As long as that isn't so, things like making vo_placebo somehow reuse parts of vo_gpu speaks against this PR.

Can you clarify which parts of vo_placebo is reusing vo_gpu components in a way you don't like? Do I understand correctly that this is about the reuse of ra_ctx to get a window/swapchain?

There is also some scary stuff like vo_placebo making mpv dependent on a libplacebo that in turn depends on ffmpeg (libavutil) due to AVFrame usage, which is something I'm 100% opposed to.

This is actually not the case. <libplacebo/utils/libav.h> is a single header library containing a set of static helpers. libplacebo does not have a link-time dependency on ffmpeg, for exactly this reason. (Note: ffmpeg itself also depends on libplacebo, and re-uses those same helpers. It would be a cyclic dependency otherwise.)

Edit: It's also worth pointing out that the re-use of the AVFrame helpers is only to avoid me having to write more code than necessary to get this PR off its feet. The existing AVFrame helper already deals with a lot of the intricacies of proper format mapping, side data (ICC profiles, mastering metadata) etc., and rewriting that all would, if anything, be a minor optimization (save some memcpy).

[haasn]

The only way that would be acceptable would be merging the source of libplacebo into mpv, or at least developing it in exactly the same place (such as same github org)

If I understand this part correctly: I'd be fine with moving haasn/libplacebo to mpv-player/libplacebo. I already accept issues and merge requests on both code.videolan.org and github.com, and I don't think this causes me any extra developer burden.

(As for what it means to be "developed in a place", other than a github repo existing and accepting issues/MRs, I'm not sure)

[haasn]

Also all the options and some utility functions. You could say vo_placebo vampirizes vo_gpu.

Well what would the resolution be? Moving the options to some common file, maybe as part of options.c? They stopped being vo_gpu suboptions for a reason, there's no reason they should be exclusive to vo_gpu either.

Edit: Also, I don't see any reused utility functions besides update_auto_profiles, and for that I agree it could probably be moved somewhere better. Maybe part of whatever ra_ctx turns into.

For what?

vf_libplacebo. (Although it's not merged into master yet, still blocked by some vulkan-related API changes in libavutil)

Abstractions like mp_image translate FFmpeg's model of how everything should work to mpv's, and isolates it from any stupidity FFmpeg might have.

For what it's worth, libplacebo's abstractions are also designed for the needs of libplacebo. Since you perhaps seem to have gotten that impression, it's not a candidate for a mp_image replacement. For example, pl_frame (the closest analog) doesn't even contain data pointers. The planes are already assumed to be in pl_tex form, which the API user has to obtain separately (either via mapping hwdec frames, wrapping an existing VkImage, or uploading it using something like pl_upload_frame). It also even isn't allocated - it's just a struct to group together identifying metadata with the plane textures it applies to.

It sounds like the technical resolution here is to just bite the bullet and write upload_image which takes an mp_image and uploads it using libplacebo APIs (pl_upload_frame). That's more or less what the pl_upload_avframe helper already does internally, and doing it this way avoids any further dependency on AVFrame or the FFmpeg API (if that's your goal?).

In terms of code complexity, having a line of code taking mp_image metadata (e.g. ICC profiles) and forwarding it to pl_frame is no less "duplicate maintenance burden" than having code in vo_gpu that takes that same metadata and forwards it to the relevant shader function calls. struct pl_frame just happens to be the API I settled on for doing that metadata forwarding.

Rather than mp_image, the more annoying thing to keep up would actually be duplication of csputils.h. When I first started work on libplacebo I was actually planning to convert all of mpv to use the new PL_COLOR_* enums as well. Since libplacebo's enum's are currently a superset, I can still see this being a reasonable path forward to avoid double-maintenance of colorspace-related stuff.

Edit: It's also worth pointing out that <libplacebo/colorspace.h being a direct replacement for csputils.h, and <libplacebo/filters.h> being a direct replacement for filter_kernels.h are the only two headers I can think of that could reasonably replace any equivalent code in mpv. Almost everything else is closer to the contents of gpu/ and has no business in the rest of mpv.

Video rendering is one of mpv's "crown jewels", and moving that to a separate project might inhibit development.

This is an understandable concern, since I fully appreciate that you want to avoid outsourcing such a big component to something you don't have the ability to synchronize refactors with. That is indeed a technical problem with no immediate solutions. But I also don't think this is as big a deal as you make it out to be.

The amount of backwards-incompatible API changes I've had to make the libplacebo API over the years was, to me, astonishingly small given the scope of the refactors I've undergone since then. Most of the API is designed in a way that permits obvious extensions of functionality (e.g. new struct members, for which I, as a rule, insist on making {0} be the default behavior) without breaking existing users. So I've basically never felt burdened by e.g. needing to maintain compatibility with VLC.

To perhaps make a point: VLC's vout_placebo is currently backwards compatible down to libplacebo v1.7 (one of the earliest releases, dated 2018). This is the same vout_placebo that supports almost all of the features I've added since then. Sure, it has a several #if PL_API_VER >= N surrounding new features that have been added since then, but nothing more invasive than the obvious guarding of a few lines for forwarding an option parameter. No alternate code paths, no changes in logic, etc.

(The one single breaking change in this period of time was a switch from pl_rect2d to pl_rect2df in one struct field.)

Could I just cowboy a change into libplacebo?

Out of curiosity, what kinds of cowboy changes to vo_gpu do you have in memory as you write this?

[haasn]

For example, making a change in libplacebo will probably have to go through being challenged by testing with a quadrillion buggy drivers, and even if not, someone WILL complain.

FWIW, right now this is more so the other way around. mpv has the larger user base of people with a quadrillion buggy drivers than libplacebo. So not only is this concern also relevant for vo_gpu (thus raising the question of what you even have to lose here), I'm quite frankly expecting vo_placebo to help me find bugs in libplacebo that can only be found on said drivers.

[haasn]

It could use its own options, maybe a single one, like --vd-lavc-o etc.

That seems like an awful UI, given the number of options. Personally my goal is for vo_placebo to be a drop-in replacement for vo_gpu. It's not very drop-in if you have to replicate half your config file into a single thousands-of-characters-long line that contains nested lists inside lists.

If anything, the only way this would make sense is for libplacebo's option to be prefixed, like --placebo-glsl-shaders= rather than --glsl-shaders=. But if we're doing that, then I expect vo_gpu's options to be namespaced as well. Otherwise the design makes no sense. The lack of a namespace implies, to me, that the options should apply to all VOs capable of supporting them.

I saw at least parts of the lcms being changed too.

Oh, yeah, that. To be honest the better way to design that would be to have the parse_3dlut_size somehow be part of the option itself, so the consumer of the opts struct just gets the int[3].

I think what I'll probably do is lift gl_video_opts out of gpu/video.c and into the common VO code, call it mp_render_opts, and also do the 3DLUT size parsing and so on there.

And if it makes you happy, I could also split ra_ctx off of the rest of ra more cleanly.

[...]

The AVFrame helpers are just helpers. I'm not sure why you keep stressing on about mp_image vs AVFrame as though it were a big deal, given that from libplacebo's PoV both are alien and require mapping code. The only design consideration here is that mapping code already exists for AVFrame but still needs to be written for mp_image. We're talking about a grand total of 50-100 lines of code that differ between the two designs.

But anyway, I'll update the PR to use mp_image directly, which should alleviate the concerns you raised (and perhaps clear up any misunderstandings).

I too was hesitant to do anything advanced to vo_gpu, because what if it just gets replaced by libplacebo? For me it's a messed up conflict with no good solution.

OTOH, if vo_placebo exists as a quasi-equivalent to the current vo_gpu, that could also free you up to freely change vo_gpu as much as you want without worrying about removing anything people would complain about.

Personally, I always thought you'd be happy to delete ~20k lines of complexity and baggage from mpv?

they bump the version on every API or ABI (not sure, probably both) change.

Ah, yes. When I mean API break, I mean API break - not ABI break. I encode the API version into the SONAME as a deliberate choice. When I speak about API backwards comapatiblity, I mean the ability to recompile the same code against newer versions of libplacebo without modification.

But mpv currently uses libplacebo only as vulkan "driver".

I meant after vo_placebo is merged.

[haasn]

My goal is that it isn't.

What exactly are you worried about will happen if vo_placebo takes the same options as vo_gpu? User confusion?

Not in exchange for an even larger library based on code stolen from mpv and out of any control.

The last mpv-licensed code was removed from libplacebo with version v3.120, except for a few helper functions (e.g. dither generation code) that I have received permission from the respective original authors to use under a permissive license.

So the only sense in which anything in libplacebo could even remotely be considered "stolen" is in the sense that I have re-used my own algorithms and designs, for which I don't remember signing any CLA that would prevent me from doing exactly that.

Because it is a big deal, as I have explained before. Besides, mapping 2 times is still better than mapping 3 times.

I've written (not yet pushed) a version without any AVFrame dependency. Do you have any remaining objections?

Edit: Pushed

[haasn]

Besides, mapping 2 times is still better than mapping 3 times.

Something I noticed while implementing plane_data_from_imgfmt is that this is not actually necessarily the case. In particular, AVFrame currently supports quite a number of features that mp_image does not (due to mpv limitations), such as AV1 grain metadata, new colorspaces (e.g. BT.2100 ICtCp) and extra components of the mastering metadata struct.

The libplacebo<->AVFrame helpers map all of this metadata - everything that libplacebo supports, really - whereas the libplacebo<->mp_image function only maps the subset that's also supported by mp_image.

Now, if somebody were to extend the mp_image structs in the future to add these missing members, they have to update both the mp_image<->AVFrame mapping and the mp_image<->libplacebo mapping functions. Re-using the AVFrame mappers would eliminate this unnecessary duplication of code inside mpv.

Anyway, not a huge deal, just pointing it out.

[Akemi]

Why do you want libplacebo to be in mpv anyway? It already got a renderer. You can use your libplacebo knowledge to improve vo_gpu, if you think the former is better.

one point is probably to minimise the work that has to be done. otherwise it would be twice the work.

[haasn]

Why do you want libplacebo to be in mpv anyway? It already got a renderer. You can use your libplacebo knowledge to improve vo_gpu, if you think the former is better.

1. So I can test libplacebo by essentially using it myself.
2. So I don't have to fix bugs in both vo_gpu and libplacebo.
3. It already supports some features that make it more attractive for script authors (and I plan on adding more)

I have no interest in extending vo_gpu in the same ways. I wrote libplacebo for a reason, and it was because I wanted to rewrite vo_gpu from-scratch to fix some of the major design issues I had with it. libplacebo is that from-scratch rewrite. Trying to backport the changes would defeat the purpose.

I might as well cp -r libplacebo into the mpv repository, which is apparently what you want me to do, anyways?

The first commit admits it [..] But then deflects by calling it a joke.

It is obviously a joke, at least that was obvious to me. In the same commit you can see clearly that the LICENSE file explained which parts were taken from mpv, and copied the relevant parts of the mpv licensing agreement. By any legal system I know of, this is completely fair, and entirely the point of free software. If that constitutes "stealing" in your worldview, then perhaps you should consider relicensing mpv under a nonfree EULA.

And anyway, that's beside the point, because I went out of my way to rewrite all of the code that I took from mpv initially.

Well, some seem to have the opinion that the "ship of theseus" question applies to software copyright as well.

By that same argument mpv should be GPLv2 licensed and you are therefore engaging in willful copyright infringement by claiming to have relicensed it under LGPLv2. So I think we can both agree that it's a retarded argument and probably doesn't apply in Europe. And besides, by the draconian US copyright laws I would be physically unable to write libplacebo because I have seen mpv code and therefore my brain is permanently tainted with knowledge of its algorithms. So yeah, I don't see that discussion getting anywhere.

Also you seem to have forgotten my points about why using AVFrame is bad.

What do you mean? I pushed a commit that avoids the usage of AVFrame. How is that "forgetting" about your points? Is this not evidence of me taking your points into consideration?

[haasn]

From mpv's point of view, maintenance becomes much harder.

From the point of vo_gpu being currently unmaintained, this argument is absurd. A big reason to add vo_placebo is precisely because I have an interest in maintaining the latter but not the former.

Why does it need to replace vo_gpu for that?

For that point, it doesn't. Notice that this PR does not remove vo_gpu. As I understand it, this is still an argument primarily about whether or not it should be allowed to have the same options syntax or not. (And incidentally, had it not, testing it would have been a lot more annoying)

Making an external project wasn't a requirement to rewrite vo_gpu yet again.

I didn't claim it had to be.

Would a lawyer agree? If anything, it's clear that you want to keep it legal, but it was also clear that you were taking a huge chunk of mpv code with the intention to replace it eventually.

I disagree with both halves of that statement. Not only did I not take a "huge chunk of mpv code" (that commit mostly consists of talloc, bstr, and matrix/filter math - hardly components central to libplacebo's functionality), but the intent at the time also wasn't to eventually replace it.

The idea of rewriting tainted code came up years later, when I realized how small a part of libplacebo that actually applied to. (It was still essentially talloc and bstr that had to be replaced - neither of which are particularly core to libplacebo's functioning.)

(As a side note, you'll also notice that libplacebo is still LGPLv2.1, so what a lawyer thinks about it only matters in as much as determining whether or not you referring to my code as "stolen" technically qualifies as defamation or not.)

All of the original authors were asked to relicense, and agreed. (A very low number didn't agree or didn't reply, and their code was replaced in the way you mentioned.) What you did was take existing code to "get started", replace it bit by bit with your own code, and then declared that it was free from the original code. Whether that really is the case is rather questionable.

I'd say the second half of that describes mpv more accurately than libplacebo, but whatever.

In the interest of coming to a conclusion here (lest we spend the rest of this thread listening to your rationalizations about why everything not under your direct control is shit), do I sense correctly that your remaining issue with this PR is of political, not technical, nature? In which case, a resolution is impossible, and the only possible outcome ends in either you being re-removed from the mpv organization or in me forking it for good?

[haasn]

How about we say: you're rejecting my proposed solution out of political, not technical natrure.

Your proposed solution being to hide all of the vo_placebo options behind a suboption namespace, the only justification for which is that it will prevent vo_placebo from replacing vo_gpu?

[haasn]

My preferred proposal was putting libplacebo development under mpv control.

As you yourself pointed out, what you want (i.e. the ability to freely refactor) is impossible because libplacebo already has other users besides mpv. You'd still need to maintain API compatibility even if it was under mpv control.

If your issue is merely about keeping the mpv code base in sync with changes to libplacebo, git submodules could solve that.

[haasn]

I don't know why you think that's a problem. Formally speaking, libmpv's API has been stable since 2014, which is longer both since the last libplacebo API break, and libplacebo's existence.

I specifically don't think it's a problem. I am mirroring your concern from earlier with regards to not wanting to lose the ability to make API changes without making sure to keep VLC/FFmpeg/etc. happy.

If that's not the concern, then what exactly is the advantage you hope to gain from having the code in a different place on your hard drive?

Also you can't tell me your OpenGL format table rewrite is copyright-safe.

Which part are you concerned about? The OpenGL format list, and the corresponding extensions that enable them, are baked into the OpenGL specification, so the end result is necessarily similar. I'm not sure how I could possibly come up with a different table of formats without violating the OpenGL spec.

For what it's worth, my implementation even chooses an entirely different mechanism of choosing which formats to enable - splitting them up into a list of formats provided per-extension, rather than having a big list with extension bits.

(Also, for what it's worth, even had I gone through with relicensing libplacebo, I wouldn't have started with the opengl subsystem, merely because that's the part I'm also the most worried about - alongside colorspace.c)

Why even do this when libplacebo is LGPL?

Perhaps to avoid giving people the impression that libplacebo is "stolen code", when the reality is that I essentially authored >95% of it, if not more - including the code in vo_gpu that it was based on.

Clearly I was unsuccessful.

There might be more - maybe I'll dig further if it starts mattering.

While your copyright expertise is certainly appreciated, dare I ask - out of curiosity - why you care so much, especially given your own history of relicensing others' code so you can monetize it? (Something I haven't actually done)

(Or is this business about copyright just a ploy to detract the conversation away from anything that would undermine your ability to NAK this PR on technical grounds?)

[haasn]

But my main concern is simply that development is made harder and less flexible.

As somebody who has written much of (if not the majority of) the code in vo_gpu over a good 5 years or so, I feel like my opinion on the matter should have some amount of weight to it, when I say that I think libplacebo in its current state makes development easier, not harder - separate repo or not.

I don't know, progressively removing any contributions I might have had in this thing at all just rubs me the wrong way and I'm trying to find reasons, and uh, what you just said is certainly odd.

You're placing me in a very strange double bind. If I preserve your contributions, it's "stealing code". If I remove your contributions, it's discrediting you?

But anyways, it seems we've reached the point of fully venting out whatever emotional disagreements we had about the state of libplacebo/vo_gpu development, or at least not wanting to continue this pointless debate / insulting about meta-issues that miss the point.

While I appreciate that you're frustrated by your lack of involvement in the project, you surely also appreciate that I think it's very reasonable not to delete a project I've been working on for 3 years just to merge it into a different project run by a benevolent dictator whose decisions I clearly have little ability to overturn.

Feel free to merge libplacebo, as long as it's in the priority list below every other VOs and doesn't overlap with vo_gpu in the slightest way.

Well, I feel like lifting the gpu/video.c options out of vo_gpu - something I already committed to - would accomplish this?

And I'm fine with it being low priority. The whole intent of this PR was mostly to get testing and feedback in.

[haasn]

I feel like my opinion on the matter should have some amount of weight to it.

Well if we agree that everybody's opinion deserves weight, why can't we settle this with a vote?

Purely on the technical side you might be right, though I didn't check. But what about the organizational side?

I don't know, as I don't really have much to compare. The few contributions I've received to libplacebo have gone over smoothly, so I don't have a negative precedent for what a more invasive change could look like.

I do read ill intent out of some of these changes.

It's possible to read ill intent into anything. It's also possible to do the opposite. The choice on that particular matter is yours.

Personally I'm confused about what this "timeline" you're talking about is referring to. Perhaps you also understand the libplacebo history better than I do?

While I appreciate that you're frustrated by your lack of involvement in the project

The point your oh-so-clever copy/paste of my argument completely misses is that there is a difference between a lack of involvement and a lack of an ability to become involved. I am clearly frustrated by my lack of an ability to have a voice in the mpv project. As far as I can tell, you have not even attempted to make any changes to libplacebo, so the reverse does not even make sense. This is an asymmetric situation, one in which you appear primarily worried about what could happen.

But in either case, I'm also not enjoying any second of needing to convince a needlessly antagonistic maintainer, especially given that this is not the first time we've had this sort of argument, and I don't see the cycle ever stopping.

can you stop ruining my new found fun (as I have thought) with this project? Go do your own thing.

If that's the way it'll be, I suppose I really do have no choice but to go do my own thing (again). I'm sorry to have wasted your precious time.

an "epic win" for the mpv project, as I'm sure the peanut gallery will agree

[pigoz]

Don't shoot the messenger 🔫

<Quote removed by effective request of @decent-name >

[haasn]

Oh well, that actually went differently from what I've hoped. I apologize and I'm hopefully gone forever.

Now that it's a new day and I'm less underslept, I wouldn't mind re-opening discussion to seek a different resolution.

Can you please clarify - what is it you were hoping would happen?

Edit: Never mind, I guess. I just saw your decision to leave (as well as your kind words about me) and all I have to say, I'm... confused. I already said I wouldn't try getting this merged further. So I'm not just compromising, I'm effectively giving you exactly what you want - the ability to dictate mpv without my influence.

Given that, I would still like some clarification on what exactly it is you were hoping would happen, since apparently even me being gone isn't enough to make you happy.

Edit 2: Oh, I just realized you can't reply to the issue, as it has been locked (not by me).

[BlueSwordM]

haasn, I think you should continue working on libplacebo integration in mpv. Your work in the multimedia community has been truly excellent, and getting broader libplacebo support in mpv would be great.

I wish you all the good luck of the world in this regard! :D

[haasn]

Found some bugs related to subtitles:

• some sort of memory leak related to subtitles
• alpha blending issues with subtitles
• target position related validation error for some subtitles

[haasn]

Minor annoyance:

• Lots of warnings about discontinuous source PTS jumps when seeking forwards by small amounts. Investigate.

[haasn]

Used this for about a week now with no more issues in my own testing.

Except that earlier today I managed to trigger a mixer-related assertion (fidx > 0). I made a change upstream that I think eliminates this particular crash (though the triggering coredump still seems a bit suspicious in other ways).

Anyway, I'm happy merging as-is, so we can get more testing in, unless somebody objects.

[haasn]

Hmm, the bug fix to the last issue seems to have introduced a regression when seeking backwards. I'll look into it tomorrow. (Though this should be easy to fix)

Edit: Oops, its was trivial, I just accidentally omitted the p->last_pts = 0.0; line when refactoring the queue resetting code. With that in place it works again.

[haasn]

The deed is done. It is time to atone for our sins.

[v-fox]

The deed is done. It is time to atone for our sins.

By the way, it seems there is an additional benefit to this placebo/gpu-next vo: legacy 'gpu' vo triggers some kind of bug (which upstream just ignores for years) on firmware or driver of my Radeon RX580 which makes it stuck on max voltage until reboot but with gpu-next it seems fine. mpv is the only app that triggers it, that I know. Therefore, in fact, you have already atoned for that sin ;]