There is no good reason to do so, both zimg and swscale supports full
range output. If downstream something expects limited range yuv always,
it needs to be comunicated in different way.
c784820454 introduced a bool option type
as a replacement for the flag type, but didn't actually transition and
remove the flag type because it would have been too much mundane work.
This mapping isn't actually relevant until we have the Vulkan interop
merged, and it requires a newer version of libavutil than our minimum
requirement. So I'm going to remove it from master and put it in the
interop PR.
Fixes#10813
Wayland VO that can display images from either vaapi or drm hwdec
The PR adds the following changes:
1. a context_wldmabuf context with no gl dependencies
2. no-op ra_wldmabuf and dmabuf_interop_wldmabuf objects
no-op because there is no need to map/unmap the drmprime buffer,
and there is no need to manage any textures.
Tested on both x86_64 and rk3399 AArch64
vaapi allows for implicit conversion on upload, which has some
relevance as the set of supported source formats is larger than the
set of displayable formats. In theory, this allows for offloading the
conversion to the GPU - if you have any confidence in the hardware
and/or driver's ability to do the conversion.
Today, we actually track the 'input' and 'output' upload formats
separately all the way up until the point we do a check as to whether
the original source format is an accepted 'output' format and then
reject it if it is not.
This means that we're essentially ignoring all the work we did to track
those 'input' formats in the first place. But it also means that it's a
simple change to compare against the 'input' format instead. The logic
is already in place to do best format selection on both sides.
I imagine that if I read through the history here, wm4 tried to
implement all of this properly and then gave up in disgust after seeing
vaapi mangle various conversions.
This is particularly interesting for vo-dmabuf-wayland where it is only
possible to display the subset of valid vaapi formats that are
supported by the compositor, yet all playback has to go through vaapi.
Users will then be able to take advantage of all possible vaapi formats
to avoid having to do software format conversion.
A few years ago, wm4 got sufficiently annoyed with how vaapi image
format support was being discovered that he flipped the table and
introduced the shit list (which just included vaapi) to hard-code the
set of supported formats.
While that might have been necessary at the time, I haven't been able
to find a situation where the true list of supported formats was unsafe
to use. We filter down the list based on what the vo reports - and the
vo is already doing a thorough testing of formats, and if a format
makes it through that gauntlet, it does actually work.
Interestingly, as far as I can tell, the hwdec_vaapi probing code was
already good enough at the time (also written by wm4), so perhaps the
key difference here is that the driver side of things has improved.
I dug into this because of the support for the 422/444 high bit depth
vaapi formats I added to ffmpeg. These are obviously not in the hard
coded list today, but they work fine.
Finally, although it's positioned as a vaapi thing, it's really just
Intel specific, as the AMD vaapi driver has never exposed support for
anything except the formats used by the decoder/encoder profiles.
Certain combinations of hardware formats require the use of hwmap to
transfer frames between the formats, rather than hwupload, which will
fail if attempted.
To keep the usage of vf_format for HW -> HW transfers as intuitive as
possible, we should detect these cases and do the map operation instead
of uploading.
For now, the relevant cases are moving between VAAPI and Vulkan, and
VAAPI and DRM Prime, in both directions. I have introduced the IMGFMT
entry for Vulkan here so that I can put in the complete mapping table.
It's actually not useless, as you can map to Vulkan, use a Vulkan
filter and then map back to VAAPI for display output.
Historically, HW -> HW uploads did not exist, so the current code
assumes they will never happen. But as part of introducing Vulkan
support into ffmpeg, we added HW -> HW support to enable transfers
between Vulkan and CUDA.
Today, that means you can use the lavfi hwupload filter with the
correct configuration (and previous changes in this series) but it
would be more convenient to enable HW -> HW in the format filter so
that the transfers can be done more intuitively:
```
--vf=format=fmt=cuda
```
and
```
--vf=format=fmt=vulkan
```
Most of the work here is skipping logic that is specific to SW -> HW
uploads doing format conversion. There is no ability to do inline
conversion when moving between HW formats, so the format must be
mutually understood to begin with.
Additional work needs to be done to enable transfers between VAAPI
and Vulkan which uses mapping, rather than uploads. I'll tackle that
in the next change.
Today, lavfi filters are provided a hw_device from the first
hwdec_interop that was loaded, regardless of whether it's the right one
or not. In most situations where a hardware based filter is used, we
need more control over the device.
In this change, a `hwdec_interop` option is added to the lavfi wrapper
filter configuration and this is used to pick the correct hw_device to
inject into the filter or graph (in the case of a graph, all filters
get the same device).
Note that this requires the use of the explicit lavfi syntax to allow
for the extra configuration.
eg:
```
mpv --vf=hwupload
```
becomes
```
mpv --vf=lavfi=[hwupload]:hwdec_interop=cuda-nvdec
```
or
```
mpv --vf=lavfi-bridge=[hwupload]:hwdec_interop=cuda-nvdec
```
If we want to be able to handle conversion between hw formats in filter
chains, then we need to be able to load hwdec_interops from filters, as
the VO is only ever going to initialise one interop, based on its
configuration. That means that in almost all situations, only one of
the required interops will be loaded at the time the filter is
initialised.
The existing code has some assumptions that new hwdec_interops will not
be loaded after the vo has picked one to use. This change fixes two
instances:
* Refusing to load a new hwdec_interop if there is at least one
loaded already.
* Not recalculating the set of formats known to the autoconvert
filter when a new output format shows up. This leads to autoconvert
not knowing that a new format is supported when the hwdec interop is
lazily loaded.
It turns out that it's generally more useful to look up hwdecs by image
format, rather than device type. In the situations where we need to
find one, we generally know the image format we're dealing with. Doing
this avoids us having to create mappings from image format to device
type.
The most significant part of this change is filling in the image format
for the various hw interops. There is a hw_imgfmt field today today, but
only a couple of the interops fill it in, and that seems to be because
we've never actually used this piece of metadata before. Well, now we
have a good use for it.
This was introduced in 04257417 without a clear explanation of the bug
it was solving, so I have no idea if it's still needed (or why it ever
was). And it definitely creates unexpected behavior, e.g. forced
clipping when converting between float and floatp.
I therefore think we should simply remove this logic and see if it
regresses anything else, then fix those other bugs *properly* (if
they're still around).
Fixes#9979
When I introduced the concept of lazy loading of hwdecs by img format,
I did not propagate the probing flag correctly, leading to the new
normal loading path not runnng with probing set, meaning that any
errors would show up, creating unnecessary noise.
This change fixes this regression.
A few years ago, in 25e70f4743, we
disabled the vavpp deinterlacing auto-filter on the basis that it
caused crashes on _some_ hardware with _some_ driver version(s). But
since then, the situation has improved. There is still a limitation
where you can't turn deinterlacing on on the fly with AMD, but it
doesn't crash anymore (That is #7388).
So, given that AMD users have to set up the deinterlacing filter
manually either way, let's re-add the auto-filter for Intel users.
Historically, we have treated hwdec interop loading as a completely
separate step from loading the hwdecs themselves. Some hwdecs need an
interop, and some don't, and users generally configure the exact
hwdec they want, so interops that aren't relevant for that hwdec
shouldn't be loaded to save time and avoid warning/error spam.
The basic approach here is to recognise that interops are tied to
hwdecs by imgfmt. The hwdec outputs some format, and an interop is
needed to get that format to the vo without read back.
So, when we try to load an hwdec, instead of just blindly loading all
interops as we do today, let's pass the imgfmt in and only load
interops that work for that format. If more than one interop is
available for the format, the existing logic (whatever it is) will
continue to be used to pick one.
We also have one callsite in filters where we seem to pre-emptively
load all the interops. It's probably possible to trace down a specific
format but for now I'm just letting it keep loading all of them; it's
no worse than before.
You may notice there is no documentation update - and that's because
the current docs say that when the interop mode is `auto`, the interop
is loaded on demand. So reality now reflects the docs. How nice.
Ever instance of m_obj_list is a constant and for all of them, the field
is true. Just remove the field all together.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
This seems to work on gcc, clang and mingw as-is, but I made it
conditional on __GNUC__ just in case, even though I can't figure out
which compilers we care about that don't export this define.
Also replace all instances of assert(0) in the code by MP_UNREACHABLE(),
which is a strict improvement.
Today, validation is only possible for string type options. But there's
no particular reason why it needs to be restricted in this way, and
there are potential uses, to allow other options to be validated
without forcing the option to have to reimplement parsing from
scratch.
The first part, simply making the validation function an explicit
field instead of overloading priv is simple enough. But if we only do
that, then the validation function still needs to deal with the raw
pre-parsed string. Instead, we want to allow the value to be parsed
before it is validated. That in turn leads to us having validator
functions that should be type aware. Unfortunately, that means we need
to keep the explicit macro like OPT_STRING_VALIDATE() as a way to
enforce the correct typing of the function. Otherwise, we'd have to
have the validator take a void * and hope the implementation can cast
it correctly.
For help, we don't have this problem, as help doesn't look at the
value.
Then, we turn validators that are really help generators into explicit
help functions and where a validator is help + validation, we split
them into two parts.
I have, however, left functions that need to query information for both
help and validation as single functions to avoid code duplication.
In this change, I have not added an other OPT_FOO_VALIDATE() macros as
they are not needed, but I will add some in a separate change to
illustrate the pattern.
Essentially, this lets video.c decide whether to consider a video track
cover art, instead of having the decoder wrapper use the lower level
sh_stream flag.
Some pain because of the dumb threading shit. Moving the code further
down to make some of it part of the lock should not change behavior,
although it could (framedrop nonsense).
This commit should not change actual behavior, and is only preparation
for the following commit.
Do not make resetting the "access" filters reset the queue itself. This
is more flexible, and will be used in a later commit.
Also, if the queue is not in the reset state while the input access
filter is reset, make it immediately request data again. This is more
consistent, because it'll enter the state it "should" be, rather when
the filter's process function is called at an (essentially) random point
in the future. This means the filter graph will resume work on its own
if the queue was not reset before filter reset.
This could affect the only current user of f_async_queue, the code for
the --vd-queue-enable/--ad-queue-enable feature in f_decoder_wrapper.
But it looks like this already uses it in a compatible way.
This is a kind of bad hack (with bad implementation) to paint over other
problems of the filter system. The main problem is that some filters
might be left with pending frames if the filter runner is "paused",
which we don't want. To be used in a later commit.
It's relevant in some obscure corner cases (EDL file that has a segment
without audio). Didn't test what's actually going on (is ad_lavc.c
behaving wrong? is libavcodec behaving wrong or in an unexpected way? is
lavc_process wrong?) and just patched it over with some bullshit, so the
fix might be too complicated, and could be reworked at some later point.
This sure is a real data flow fuckmess.
scaletempo2 is a new audio filter for playing back
audio at modified speed and is based on chromium
commit 51ed77e3f37a9a9b80d6d0a8259e84a8ca635259.
It sounds subjectively better than the existing
implementions scaletempo and rubberband.
This mode drops or repeats audio data to adapt to video speed, instead
of resampling it or such. It was added to deal with SPDIF. The
implementation was part of fill_audio_out_buffers() - the entire
function is something whose complexity exploded in my face, and which I
want to clean up, and this is hopefully a first step.
Put it in a filter, and mess with the shitty glue code. It's all sort of
roundabout and illogical, but that can be rectified later. The important
part is that it works much like the resample or scaletempo filters.
For PCM audio, this does not work on samples anymore. This makes it much
worse. But for PCM you can use saner mechanisms that sound better. Also,
something about PTS tracking is wrong. But not wasting more time on
this.
This sucks, but is helpful for testing.
Obviously, it would be much nicer if there were a way to specify _all_
scaler options per filter (if the user wanted), instead of always using
the global options. But this is "too hard" for now. For testing, it is
extremely convenient to select the scaler backend, so add this option,
but make clear that it could go away. We'd delete it once there is a
better mechanism for this.
When the player core requests new frames from the filter, this is called
external/recursive filtering, which acts slightly differently from when
filters request new data internally. Mainly this is so the external user
doesn't have to call mp_filter_graph_run() just to get a frame. This
causes a number of complications, and the short version is that until
now, mp_filter_graph_run() has unnecessarily returned true in the
current common case, which made the playloop run too often for no
reason.
The problem is that when a mp_pin is read externally, updating the same
mp_pin during recursive filtering flagged external_pending when the
result was written, which made mp_filter_graph_run() return true, which
made the playloop call mp_filter_graph_run() again. This is redundant
because the caller is obviously checking the new state of the mp_pin
immediately.
The only situation in which external_pending really must be set is if
_another_ pin is changed. In theory, we could also unset it if the set
of "external" pins that are not in a signaled state becomes empty, but
we don't track that in a convenient way.
This commit removes the redundant signaling, and avoids running the
playloop an additional time for each video and audio frame (as it
actually was planned from the beginning, but duh).
If a filter receives an asynchronous wakeup during filtering, then
process newly pending filters resulting from that as well, before
returning to the user. Might possibly skip some redundant playloop
cycles.
There is an explicit comment in the code about how this shouldn't be
done, but I think it makes no sense. Filters have no business trying to
interrupt the mainloop, and mp_filter_graph_interrupt() provides a
proper mechanism to do this (though intended to be used by the filter
user, not filters).
After calling the main filter's destroy callback, all child filters are
destroyed. But one of them still tried to access the cache_lock mutex
(which is destroyed in said destroy callback). This actually caused a
crash on Android with _FORTIFY_SOURCE.
Fix this by destroying the child filters first.
As an unfortunate disaster, min/max values use the type double, which
causes tons of issues with int64_t types. Anyway, OPT_BYTE_SIZE is often
used as maximum for size_t quantities, which can have a size different
from (u)int64_t.
OPT_BYTE_SIZE still uses in64_t, because in theory, you could use it for
file sizes. (demux.c would for example be capable of caching more than
2GB on 32 bit platforms if a file cache is used. Though for some reason
the accounting code still uses size_t, so that use case is broken. But
still insist that it _could_ be used this way.)
There were various inconsistent attempts to set m_option.max to a value
such that the size_t/int64_t upper limit is not exceeded. Due to the
double max field, this didn't really work correctly. Try to fix this
with the M_MAX_MEM_BYTES constant. It's a good approximation, because on
32 bit it should allow 2GB (untested, also would probably exhaust
address space in practice but whatever), and something "high enough" in
64 bit.
For some reason, clang 11 still warns. But I think this might be a clang
bug, or I'm crazy. The result is correct anyway.
Change all OPT_* macros such that they don't define the entire m_option
initializer, and instead expand only to a part of it, which sets certain
fields. This requires changing almost every option declaration, because
they all use these macros. A declaration now always starts with
{"name", ...
followed by designated initializers only (possibly wrapped in macros).
The OPT_* macros now initialize the .offset and .type fields only,
sometimes also .priv and others.
I think this change makes the option macros less tricky. The old code
had to stuff everything into macro arguments (and attempted to allow
setting arbitrary fields by letting the user pass designated
initializers in the vararg parts). Some of this was made messy due to
C99 and C11 not allowing 0-sized varargs with ',' removal. It's also
possible that this change is pointless, other than cosmetic preferences.
Not too happy about some things. For example, the OPT_CHOICE()
indentation I applied looks a bit ugly.
Much of this change was done with regex search&replace, but some places
required manual editing. In particular, code in "obscure" areas (which I
didn't include in compilation) might be broken now.
In wayland_common.c the author of some option declarations confused the
flags parameter with the default value (though the default value was
also properly set below). I fixed this with this change.
Before this commit, option declarations used M_OPT_MIN/M_OPT_MAX (and
some other identifiers based on these) to signal whether an option had
min/max values. Remove these flags, and make it use a range implicitly
on the condition if min<max is true.
This requires care in all cases when only M_OPT_MIN or M_OPT_MAX were
set (instead of both). Generally, the commit replaces all these
instances with using DBL_MAX/DBL_MIN for the "unset" part of the range.
This also happens to fix some cases where you could pass over-large
values to integer options, which were silently truncated, but now cause
an error.
This commit has some higher potential for regressions.
Move the "old" mostly command line parsing and option management related
code to m_config_frontend.c/h. Move the the code that enables other part
of the player to access options to m_config_core.c/h. "frontend" is out
of lack of creativity for a better name.
Unfortunately, the separation isn't quite clean yet. m_config_frontend.c
still references some m_config_core.c implementation details, and
m_config_new() is even left in m_config_core.c for now. There some odd
functions that should be removed as well (marked as "Bad functions").
Fixing these things requires more changes and will be done separately.
struct m_config is left with the current name to reduce diff noise.
Also, since there are a _lot_ source files that include m_config.h, add
a replacement m_config.h that "redirects" to m_config_core.h.
The mp_filter_run() invocation blocks as long as the demuxer provides
packets and the queue can be filled. That means it may block quite a
long time of the decoder queue size is large (since we use libavcodec in
a blocking manner; it regrettably does not have an async. API).
This made the main thread freeze in certain situations, because it has
to wait on the decoder thread.
Other than I suspected (I wrote that code, but that doesn't mean I know
how the hell it works), this did not freeze during seeking: seek resets
flushed the queue, which also prevented the decoder thread from adding
more frames to it, thus stopping decoding and responding to the main
thread in time. But it does fix the issue that exiting the player waited
for the decoder to finish filling the queue when stopping playback.
(This happened because it called mp_decoder_wrapper_set_play_dir()
before any resets. Related to the somewhat messy way play_dir is
generally set. But it affects all "synchronous" decoder wrapper API
calls.)
This uses pretty weird mechanisms in filter.h and dispatch.h. The
resulting durr hurr interactions are probably hard to follow, and this
whole thing is a sin. On the other hand, this is a _very_ user visible
issue, and I'm happy that it can be fixed in such an unintrusive way.
p->log has a prefix set that gives some context and distinguishes audio
and video decoders. The "public" wrapper filter didn't use it, which is
a regression since commit a3823ce0e0.
Filtering is integrated into an event loop, which is something the
filter API user provides. To make interacting with the event loop
easier, and in particular to avoid filtering to block event handling,
add functions the event loop code can suspend filtering.
While we cannot actually suspend a single filter, it's pretty easy to
suspend the filter graph run loop itself, which is responsible for
selecting which filter to run next.
This commit shouldn't change behavior at all, but the functions will be
used in later commits.
This was forgotten.
Hardware decoding typically breaks immediately, because many hw decoding
APIs require allocating all surfaces in advance (and/or libavcodec was
not made flexible enough to add new surfaces later). If the queue is
large enough, it will run out of surfaces, fail decoding, and fall back
to software decoding. We consider this the user's fault.
--hwdec-extra-frames can be used to avoid this, if you have enough GPU
memory, and the needed number of frames is lower than the arbitrary
mpv-set maximum limit of that option.
Instead of having f_decoder_wrapper create its own copy of the entire
mpv option tree, create a struct local to that file and move all used
options to there.
movie_aspect is used by the "video-aspect" deprecated property code. I
think it's probably better not to remove the property yet, but
fortunately it's easy to work around without needing special handling
for this option or so.
correct_pts is used to prevent use of hr-seek in playloop.c. Ignore
that, if you use --no-correct-pts you're asking for trouble anyway. This
is the only behavior change.
See manpage additions. This has been a topic in MPlayer/mplayer2/mpv
since forever. But since libavcodec multi-threaded decoding was added,
I've always considered this pointless. libavcodec requires you to
"preload" it with packets, and then you can pretty much avoid blocking
on it, if decoding is fast enough.
But in some cases, a decoupled decoder thread _might_ help. Users have
for example come up with cases where decoding video in a separate
process and piping it as raw video to mpv helped. (Or my memory is
false, and it was about vapoursynth filtering, who knows.) So let's just
see whether this helps with anything.
Note that this would have been _much_ easier if libavcodec had an
asynchronous (or rather, non-blocking) API. It could probably have
easily gained that with a small change to its multi-threading code and a
small extension to its API, but I guess not.
Unfortunately, this uglifies f_decoder_wrapper quite a lot. Part of this
is due to annoying corner cases like legacy frame dropping and hardware
decoder state. These could probably be prettified later on.
There is also a change in playloop.c: this is because there is a need to
coordinate playback resets between demuxer thread, decoder thread, and
playback logic. I think this SEEK_BLOCK idea worked out reasonably well.
There are still a number of problems. For example, if the demuxer cache
is full, the decoder thread will simply block hard until the output
queue is full, which interferes with seeking. Could also be improved
later. Hardware decoding will probably die in a fire, because it will
run out of surfaces quickly. We could reduce the queue to size 1...
maybe later. We could update the queue options at runtime easily, but
currently I'm not going to bother.
I could only have put the lavc wrapper itself on a separate thread. But
there is some annoying interaction with EDL and backward playback shit,
and also you would have had to loop demuxer packets through the
playloop, so this sounded less annoying.
The food my mother made for us today was delicious.
Because audio uses the same code, also for audio (even if completely
pointless).
Fixes: #6926
This is supposed to enable communication between filter graphs on
separate threads. Having multiple threads makes only sense if they can
run concurrently with each other, which requires such an asynchronous
queue as a building block. (Probably.)
The basic idea is that you have two independent filters, which can be
each part of separate filter graphs, but which communicate into one
direction with an explicit queue. This is rather similar to unix pipes.
Just like unix pipes, the queue is limited in size, so that still some
data flow control enforced, and runaway memory usage is avoided.
This implementation is pretty dumb. In theory, you could avoid avoid
waking up the filter graphs in quite a lot of situations. For example,
you don't need to wake up the consumer filter if there are already
frames queued. Also, you could add "watermarks" that set a threshold at
which producer or consumer should be woken up to produce/consume more
frames (this would generally serve to "batch" multiple frames at once,
instead of performing high-frequency wakeups). But this is hard, so the
code is dumb. (I just deleted all related code when I still got
situations where wakeups were lost.)
This is actually salvaged and modified from a much older branch I had
lying around. It will be used in the next commit.
Instead of vague ideas about making different filter graphs on different
threads interact directly, this have no direct support. Instead, helpers
are required (such as added with the next commit).
Document it. Different root filters (i.e. separate filter graphs) are
now considered to be part of separate threads, so assert() if they're
found to accidentally interact.
mp_filter_mark_async_progress() can asynchronously mark a filter for
processing, without waking up the filter thread. (It's some sort of
idiotic micro-optimization I guess?) But since it sets async_pending
without doing the wakeup, a mp_filter_wakeup() after this will do
nothing, and the wakeup is lost. Fix it by checking for the needed
wakeup separately.
Fortunately, nothing used this function yet, so there is no impact.
I may (optionally) move decoding to a separate thread in a future
change. It's a bit attractive to move the entire decoder wrapper to
there, so if the demuxer has a new packet, it doesn't have to wake up
the main thread, and can directly wake up the decoder. (Although that's
bullshit, since there's a queue in between, and libavcodec's
multi-threaded decoding plays cross-threads ping pong with packets
anyway. On the other hand, the main thread would still have to shuffle
the packets around, so whatever, just seems like better design.)
As preparation, there shouldn't be any mutable state exposed by the
wrapper. But there's still a large number of corner-caseish crap, so
just use setters/getters for them. This recorder thing will inherently
not work, so it'll have to be disabled if threads are used.
This is a bit painful, but probably still the right thing. Like
speculatively pulling teeth.
Filters that fail to create are not supposed to do this. Generally it
should happen in process() only.
This fixes the previous commit. If a filter could not be created, it
"trashed" the wrapper filter with the failure. (Even if the wrapper were
to handle were to handle failures of sub-filter, it couldn't handle init
failure because it cannot call mp_filter_set_error_handler() before the
newly created filter is actually returned.)
Fixes: #7465 (attempt 2)
If hw decoding is used, but no hw deinterlacer is available, even though
we expect that it is present, fall back to using hw-download and yadif
anyway. Until now, it was over if the hw filter was somehow missing; for
example, yadif_cuda apparently requires a full Cuda SDK, so it can be
missing, even if nvdec is available. (Whether this particular case works
was not tested with this commit.)
Fixes: #7465
Libav seems rather dead: no release for 2 years, no new git commits in
master for almost a year (with one exception ~6 months ago). From what I
can tell, some developers resigned themselves to the horrifying idea to
post patches to ffmpeg-devel instead, while the rest of the developers
went on to greener pastures.
Libav was a better project than FFmpeg. Unfortunately, FFmpeg won,
because it managed to keep the name and website. Libav was pushed more
and more into obscurity: while there was initially a big push for Libav,
FFmpeg just remained "in place" and visible for most people. FFmpeg was
slowly draining all manpower and energy from Libav. A big part of this
was that FFmpeg stole code from Libav (regular merges of the entire
Libav git tree), making it some sort of Frankenstein mirror of Libav,
think decaying zombie with additional legs ("features") nailed to it.
"Stealing" surely is the wrong word; I'm just aping the language that
some of the FFmpeg members used to use. All that is in the past now, I'm
probably the only person left who is annoyed by this, and with this
commit I'm putting this decade long problem finally to an end. I just
thought I'd express my annoyance about this fucking shitshow one last
time.
The most intrusive change in this commit is the resample filter, which
originally used libavresample. Since the FFmpeg developer refused to
enable libavresample by default for drama reasons, and the API was
slightly different, so the filter used some big preprocessor mess to
make it compatible to libswresample. All that falls away now. The
simplification to the build system is also significant.
This is for easier use with the "delay_open" feature added in the
previous commit. The "null" codec is reported if the codec is unknown
(because the stream was not opened yet at time the tracks were added).
The rest of the timeline mechanism will set the correct codec at
runtime. But this means every time a delay-loaded track is selected, it
wants to initialize a decoder for the "null" codec.
Accept a "null" decoder. But since FFmpeg has no such codec, and out of
my own laziness, just let it fall back to "common" codecs that need no
other initialization data.
This was used to convert e.g. P010 to NV12 within the filter chain,
which hopefully a thing that is not needed anymore. (And has been dead
code since the ANGLE "RGB" interop code was removed.)
Notably, BGR0, which is the only additional format listed as supported
by the texture mapper, results in broken colors. This is most likely not
a mpv issue, so the whitelist fulfils its purpose.
I think the previous code didn't consider the situation if the input
format was not any of the upload formats. It then could have possibly
tried to upload the wrong format (and not sure what the underlying APIs
do with it).
Take care of this, also improve logging, and change it such that
mp_hwupload_find_upload_format() does not unnecessarily change the state
(although it doesn't really matter).
With the recent change how f_hwtransfer could select formats, it's
possible that the upload_fmts list contains formats that are never
selected, and the filter would have failed.
The way it works now is that f_hwtransfer gets to select the format
(which honestly doesn't make sense, but whatever), and f_autoconvert
gets only a single format.
It would be more ideal if f_hwtransfer provided a list of possible input
formats, but that's absurdly too complex for now. Maybe I'll change it
back at some later point, but I expect this shit to be in a perpetual
state of complexity and brokenness.
(Oh yes, we could have skipped all the complexity, and hardcoded the
cases that work in the first place. This wouldn't be an issue if FFmpeg
or libva exported correct information. Also possible that FFmpeg's
filter chain does not allow to do this correctly in the first place,
since filters expose next to no meta information about what hw formats
etc. they need.)
Note that uploading yuv420p to a nv12 vaapi surface actually works, but
the blacklist excludes it. So this might get a bit slower. I'm not
bothering with this case because it's rarely needed, and the blacklist
specification would become a bit more complex if you had to specify
sw/upload format pairs.
Fixes: #7350
Basically, instead of trusting the upload format, and picking the first
sw format that has a desired upload format, trust the sw format. So now
we pick the sw format first, and then select from the set of upload
formats supported by it.
This is probably more straightforward, and works around a crash with
vaapi:
mpv 8bit.mkv --vf=format=vaapi --gpu-hwdec-interop=all
(Forces vaapi upload if hw decoding is not enabled.)
Unfortunately, this still does not work, because vaapi, FFmpeg, the VO
interop code, or all of them are doing something stupid. In particular,
this picks the yuv420p sw format, which doesn't really exist despiter
advertised (???????????????????????????????????????), and simply breaks.
See: #7350
This was completely broken: it compared the first item of the filter
list only. Apparently I forgot that this is a list. This probably broke
aspects of runtime filter changing probably since commit b16cea750f.
Fix this, and remove some redundant code from obj_settings_equals().
Which is not the same as m_obj_settings_equal(), so rename it to make
confusing them harder. (obj_setting_match() has these very weird label
semantics that should probably just be killed. Or not.)
Better do this here than deal with the moronic project we unfortunately
depend on.
The workaround is generic; unknown whether it works correctly with
multi-input/output filters or filter graphs. It assumes that if all
inputs are EOF, and all outputs are EAGAIN, the bug happened.
This is pretty tricky, because anything could happen. Any time some form
of progress is made, the got_eagain state needs to be reset, because the
filter pad's state could have changed.
With the previous commit, there's no need for 1.5 anymore. And in fact,
it's just too dangerous to rely on 1.5 because of all the EGL craziness.
For example, you might get a 1.5 EGL system library, but a driver might
still give you 1.4 at runtime. If you assume that you can call 1.5
functions, you will probably get random crashes in this case. What a
cursed API. (The same problem exists with EGL 1.3, but fortunately
nothing seems to use that anymore. We can just ignore that problem.)
It's user_filters.c which allows the "lavfi-" prefix to distinguish
libavfilter filters from mpv builtin filters. f_lavfi.c is a layer
below, and strictly passes anything it gets to libavfilter. So the
correct place for this is in user_filters.c, which also has the code for
stripping the prefix in the normal filter instantiation code.
This didn't match what is in wscript_build.py. Also, it should work on
non-X11 platforms... probably. (The condition is convoluted and almost
nonsensical, but the offscreen context creation needs to be cleaned up
anyway as soon as other backends, e.g. for win32, are added.)
Probably pretty useless in this form (see: the wall of warnings), but
someone wanted this.
I think this should be useful to perform some automated tests, maybe.
Fixes: #7194
Just an implementation detail that can be cleaned up now. Internally,
m_config maintains a tree of m_sub_options structs, except for the root
it was not defined explicitly. GLOBAL_CONFIG was a hack to get access to
it anyway. Define it explicitly instead.
This is used to detect whether any filters were changed. This code was
essentially ported to m_option.c.
One possible difference is how the old code did name comparison. It did
not actually compare the name (!?!?), so this might change behavior,
hopefully to the better.
Until now, using a filter not in mpv's builtin filter list would assume
it's a libavfilter filter. If it wasn't, the option value was still
accepted, but creating the filter simply failed. But since this happens
after option parsing, so the result is confusing.
Improve this slightly by checking filter names. This will reject truly
unknown filters at option parsing time. Unfortunately, this still does
not check filter arguments. This would be much more complex, because
you'd have to create a dummy filter graph and allocate the filter. Maybe
another time.
When changing filters at runtime (vf/af commands/properties), this could
crash due to a NULL pointer access. The code for comparing the old and
new option values (to detect changes) was simply buggy.
This wasn't done, probably regression from one of the last dozen of
times this special code path was touched. This meant coverart images
ignored the user-set aspect ratio completely, and some other things.
Shovel the code around to make the data flow slightly simpler (?). At
least there's only one send_packet function now. The old code had the
problem that send_packet() could be called even if there were queued
packets; due to sending the queued packets in the receive_frame
function, this should not happen anymore (the code checking for this
case in send_packet should normally never be called).
Untested with actual full stream hw decoders (none available here); I
created a test case by making hwaccel decoding fail.
This was too hardcoded to libswscale. In particular, IMGFMT_RGB30 output
is only possible with the zimg wrapper, so the context needs to be taken
into account (since this depends on the --sws-allow-zimg option
dynamically). This is still slightly risky, because zimg currently will
still fall back to swscale in some cases, such as when it refuses to
initialize the particular color conversion that is requested.
f_autoconvert.c could actually handle this better, but I'm tool fucking
lazy right now, and nobody cares anyway, so go away, OK?
Purpose uncertain. I guess it's slightly better, maybe.
The move of the sws/zimg options from VO opts (vo_opt_list) to the
top-level option list is tricky. VO opts have some helper code in vo.c,
that sends VOCTRL_SET_PANSCAN to the VO on every VO opts change. That's
because updating certain VO options used to be this way (and not just
the panscan option). This isn't needed anymore for sws/zimg options, so
explicitly move them away.
lavc_process() calls the receive/send callbacks, which mirror
libavcodec's send/receive API. The receive API in particular can return
both a status code and a frame. Normally, libavcodec is pretty explicit
that it can't return both a frame and an error. But the receive callback
does more stuff in addition (vd_lavc does hardware decoding fallbacks
etc.). The previous commit shows an instance where this happened, and
where it leaked a frame in this case.
For robustness, check whether the frame is set first, i.e. trust it over
the status code. Before this, it checked for an EOF status first.
Hopefully is of no consequence otherwise. I made this change after
testing everything (can someone implement a test suite which tests this
exhaustively).
ad_lavc and vd_lavc use the lavc_process() helper to translate the
FFmpeg push/pull API to the internal filter API (which completely
mismatch, even though I'm responsible for both, just fucking kill me).
This interface was "slightly" too tight. It returned only a bool
indicating "progress", which was not enough to handle some cases (see
following commit).
While we're at it, move all state into a struct. This is only a single
bool, but we get the chance to add more if needed.
This fixes mpv falling asleep if decoding returns an error during
draining. If decoding fails when we already sent EOF, the state machine
stopped making progress. This left mpv just sitting around and doing
nothing.
A test case can be created with: echo $RANDOM >> image.png
This makes libavformat read a proper packet plus a packet of garbage.
libavcodec will decode a frame, and then return an error code. The
lavc_process() wrapper could not deal with this, because there was no
way to differentiate between "retry" and "send new packet". Normally, it
would send a new packet, so decoding would make progress anyway. If
there was "progress", we couldn't just retry, because it'd retry
forever.
This is made worse by the fact that it tries to decode at least two
frames before starting display, meaning it will "sit around and do
nothing" before the picture is displayed.
Change it so that on error return, "receiving" a frame is retried. This
will make it return the EOF, so everything works properly.
This is a high-risk change, because all these funny bullshit exceptions
for hardware decoding are in the way, and I didn't retest them. For
example, if hardware decoding is enabled, it keeps a list of packets,
that are fed into the decoder again if hardware decoding fails, and a
software fallback is performed. Another case of horrifying accidental
complexity.
Fixes: #6618
Form some reason (and because of my fault), vf_format converts image
formats, but nothing else. For example, setting the "colormatrix"
sub-parameter would not convert it to the new value, but instead
overwrite the metadata (basically "reinterpreting" the image data
without changing it).
Make the historical mistake worse, and go all the way and extend it such
that it can perform a conversion. For compatibility reasons, this needs
to be requested explicitly. (Maybe this would deserve a separate filter
to begin with, but things are messed up anyway. Feel free to suggest an
elegant and simple solution.)
This demonstrates how zimg can properly perform some conversions which
swscale cannot (see examples added to vf.rst).
Stupidly this requires 2 code paths, one for conversion, and one for
overriding the parameters.
Due to the filter bullshit (what was I thinking), this requires quite
some acrobatics that would not be necessary without these abstractions.
On the other hand, it'd definitely be more of a mess without it. Oh
whatever.
With the previous commit, this is dead code.
This also makes the f_autoconvert.c code for this dead code
(fortunately). Will probably remove this later.
Instead of using custom code.
Now if only f_lavfi knew what formats FFmpeg's vf_yadif accepts, this
could look much nicer, and wouldn't require the additional converter
filter setup.