This exposes whether a video track is detected as an image. This is
useful for profile conditions, property expansion and lavfi-complex, and
is more accurate than any detection even Lua scripts can perform, since
they can't differentiate between images and videos without container-fps
and audio and with duration 1 (which is the duration set by the mf
demuxer with the default --mf-fps=1).
The lavf demuxer image check is moved to where the number of frames is
available for comparison, and is modified to check the number of frames
and duration instead of the video codec. This doesn't misdetect videos
in a codec commonly used for images (e.g. mjpeg) as images, and can
detect images in a codec commonly used for videos (e.g. 1-frame gifs).
pix files are also now detected as images, while before they weren't
since the condition was checking if the AVInputFormat name ends with
_pipe, and alias_pix doesn't.
Both nb_frames and codec_info_nb_frames are checked because nb_frames is
0 for some video codecs (hevc, av1, vc1, mpeg1video, vp9 if forcing
--demuxer=lavf), and codec_info_nb_frames is 1 for others (mpeg, mpeg4,
wmv3).
The duration is checked as well because for some uncommon codecs and
containers found in FFMpeg's FATE suite, libavformat returns nb_frames =
0 and codec_info_nb_frames = 1. For some of them it even returns
duration = 0, so they are blacklisted in order to never be considered
images.
The extra codecs that would have to be blacklisted without checking the
duration are AV_CODEC_ID_4XM, AV_CODEC_ID_BINKVIDEO,
AV_CODEC_ID_DSICINVIDEO, AV_CODEC_ID_ESCAPE130, AV_CODEC_ID_MMVIDEO,
AV_CODEC_ID_NUV, AV_CODEC_ID_RL2, AV_CODEC_ID_SMACKVIDEO and
AV_CODEC_ID_XAN_WC3, while the containers are film-cpk, ivf and ogg.
The lower limit for duration is 10 because that's the duration of
1-frame gifs.
Streams with codec_info_nb_frames 0 are not considered images because
vp9 and av1 have nb_frames = 0 and codec_info_nb_frames = 0, and we
can't rely on just the duration to detect them because they could be
livestreams without an initial duration, and actually even if we could
for these codecs libavformat returns huge negative durations like
-9223372036854775808.
Some more images in the FATE suite that are really frames cut from a
video in an uncommon codec and container, like cine/bayer_gbrg8.cine,
could be detected by allowing codec_info_nb_frames = 0, but then any
present and future video codec with nb_frames = 0 and
codec_info_nb_frames = 0 would need to be added to the blacklist. Some
even have duration > 10, so to detect these images the duration check
would have to be removed, and all the previously mentioned extra codecs
and containers would have to be added added to the blacklists, which
means that images that use them (if they exist anywhere) will never be
detected. These FATE images aren't detected as such by mediainfo either
anyway, nor can a Lua script reliably detect them as images since they
have container-fps and duration > 0 and != 1, and you probably will
never see files like them anywhere else.
For attached pictures the lavf demuxer always set image to true, which
is necessary because they have duration > 10. There is a minor change in
behavior for which audio with attached pictures now has mf-fps as
container-fps instead of unavailable, but this makes it consistent with
external cover art, which was already being assigned mf-fps.
When the lavf demuxer fails, the mf one guesses if the file is an image
by its extension, so sh->image is set to true when the mf demuxer
succeds and there's only one file.
Even if you add a video's file type to --mf-type and open it with the mf
protocol, only the first frame is used, so setting image to true is
still accurate.
When converting an image to the extensions listed in demux/demux_mf.c,
tga and pam files are currently the only ones detected by the mf demuxer
rather than lavf. Actually they are detected with the image2 format, but
it is blacklisted; see d0fee0ac33.
The mkv demuxer just sets image to true for any attached picture.
The timeline demuxer just copies the value of image from source to
destination. This sets image to true for attached pictures, standalone
images and images added with !new_stream in EDL playlists, but it is
imperfect since you could concatenate multiple images in an EDL playlist
(which should be done with the mf demuxer anyway). This is good enough
anyway since the comment of the modified function already says it is
"Imperfect and arbitrary".
Let audio-display determine whether embedded images or external cover
art tracks should be selected when both are present.
Attached pictures are given priority by default as requested in #8539.
Also updates references to attached pictures in the log and manpage to
refer to cover art as well.
Closes#8539.
Though, only when the output format is matroska, to avoid muxing errors.
This is quite useful when the input has ASS subtitles, as they tend to
rely on embedded fonts.
Now loading cover art through mp_add_external_file requires an
additional argument to be set to true. This way not all video-add
commands end up being marked as cover art when they move through
mp_add_external_file, as originally changed in 55d7f9ded1 .
Additionally, this lets us clean up some logic that would otherwise be
duplicated between open_external_files and autoload_external_files, if
the logic had been kept split from mp_add_external_file.
Fixes#8358
Picks up files like "cover.jpg". It's made part of normal external file
loading, so I'm adding 3 new options that are direct equivalents for the
options that control loading of external subtitle and audio files. Even
though I bet nobody wants them and they just increase confusion... I
guess the world is actually hell, so this outcome should be fine.
It prefers non-specific external files like "cover.jpg" over embedded
cover art. Not sure if that's wanted or unwanted.
There's some pain over explicitly marking such files as external
pictures. This is basically an optimization: in most cases, a heuristic
would treat an image file loaded with --external-file the same (it's a
heuristic because ffmpeg can't tell us whether something is an image or
a video). However, even with this heuristic, it would decode the cover
art picture again on each seek, which would essentially slow down
seeking in audio files. This bothered me greatly, which is why I'm
adding these additional options at all, and bothered with the previous
commit.
Fixes: #3056
If you encode to e.g. an audio-only format, then video is disabled
automatically. This also takes care of the very cryptic error message.
It says "[vo/lavc] codec for video not found". Sort of true, but
obscures the real problem if it's e.g. an audio-only format.
This allows users to control whether full dialogue subtitles are displayed
with an audio track already in their preferred subtitle language.
Additionally, this improves handling for the forced flag, automatically
selecting between forced and unforced subtitle streams based on the user's
settings and the selected audio.
Apparently, this was a bit of a mess, which caused the bug fixed by
commit ec7f2388af. Try to improve this, and only use track selection
entries that exist.
e1e714ccc introduced a regression here when unloading a track (e.g. on
VO/AO initilization error), due to no corresponding option existing for
video/audio tracks with orders above 0, but the loop in
`mp_deselect_track` being hard-coded to clear tracks up to NUM_PTRACKS.
Introduce the simplest possible hackaround.
Consider e.g. --aid=2 with a file that has only 1 track. Then it would
fall back to selecting track 1. Stop doing this. If no matching track is
found, this will not select any track now.
Note that the fingerprint stuff (track_layout_hash in the source)
prevents softens the impact of this change. Without the fingerprint,
playing a dual-audio file with the second track selected, and then a
single-audio file, would play the second file without audio. But the
fingerprint resets it due to differences in the track list.
Try to exhaustively document this and tricky interactions between the
other features. What a damn mess, I think it's simply cursed. Of course
it's still my fault.
See: #7608
Some time ago, properties and options were mostly unified. However, the
track selection properties/options semantics are incompatible to this
change. I'm still trying to handle the fallout.
There are two things that are in the way:
1. Track properties somehow return the runtime selection, not the option
value (all while properties are supposed to be aliases to options
with the same name).
2. The user's track options are not supposed to be changed without
interaction. If a track is auto-selected, the property should return
its ID, but the option value should remain at "auto". Only if the
user actually writes to the property the option should change. E.g.
playing e.g. an audio-only file and then a normal video file not play
the video file with --vid=no just because the audio file had no video
track.
In addition to each of them being in conflict with the property/option
unification, attempt to fix one of them breaks the other one.
Today, we're trying to fix parts of this and avoiding an unfortunate
case where you can get a conflicting option/property value, and where
trying to select a track does nothing if the track to select has the
same ID as the option value.
This breaks 2. from above in certain situations. See manpage additions.
See: #7608
This should make it behave roughly like when switching from a file to
the next (clearing audio buffers, keeping AO, but closing AO if the
audio format seems to have changed and gapless mode is "weak").
Not necessarily useful, but harmless and may help with #7579 (untested).
Add an infrastructure for collecting performance-related data, use it in
some places. Add rendering of them to stats.lua.
There were two main goals: minimal impact on the normal code and normal
playback. So all these stats_* function calls either happen only during
initialization, or return immediately if no stats collection is going
on. That's why it does this lazily adding of stats entries etc. (a first
iteration made each stats entry an API thing, instead of just a single
stats_ctx, but I thought that was getting too intrusive in the "normal"
code, even if everything gets worse inside of stats.c).
You could get most of this information from various profilers (including
the extremely primitive --dump-stats thing in mpv), but this makes it
easier to see the most important information at once (at least in
theory), partially because we know best about the context of various
things.
Not very happy with this. It's all pretty primitive and dumb. At this
point I just wanted to get over with it, without necessarily having to
revisit it later, but with having my stupid statistics.
Somehow the code feels terrible. There are a lot of meh decisions in
there that could be better or worse (but mostly could be better), and it
just sucks but it's also trivial and uninteresting and does the job. I
guess I hate programming. It's so tedious and the result is always shit.
Anyway, enjoy.
May or may not help when dealing with playlist loading in scripts. It's
supposed to help with the mean fact that loading a recursive playlist
will essentially edit the playlist behind the API user's back.
Unfortunately, merely changing the playlist current position affects the
flags returned by the "playlist" property, so the entirely thing needs
to be marked as changed. Seems to be a design mistake.
Should give a good deal more explicit control and insight over the
player state.
Some feel a bit pointless, and/or expose internal weirdness. However,
it's not like the existing weirdness didn't exist before, or can be made
go away. (In part, the weirdness is because certain in-between states
are visible. Hiding them would make things simpler, but less flexible.)
Maybe this actually gives users a better idea how the API _should_ look
like, too.
On a side note, this tries to really guarantee that mpctx->playing is
set between playback start/end. For that, the loadfile.c changes assume
that mpctx->playing is set (guaranteed by code above the change), and
that playing->filename is set (probably could never be false; was broken
before and actually would have crashed if that could ever happen; in any
case, also add an assert to playlist.c for this).
playlist_entry_to_index() now tolerates playlist_entrys that are not
part of the playlist. This is also needed for mpctx->playing.
It's odd that this state is observable, but is made implicit by making
the property unavailable. It's also odd that an API user cannot directly
put the player into such a state.
Just allow reading/writing -1 (or in fact, any out of bounds index) for
this case.
I'm also refraining from using OPT_CHOICE for the "no selection" case,
because although that would be cleaner in theory, it would cause only
problems to API users due to the more complex property type (worse is
better).
One reason for not restricting the integer range on the input property
anymore is that if there are no playlist elements, the range would
contain only 1 integer, which cannot be represented anymore since the
recent m_option change. This was actually broken with 1 element
playlists before (and still is, with the constricted type for OSD and
the add/cycle commands). Doesn't matter too much.
If the user manages to run a "loadfile x append" command before the loop
in mp_play_files() is entered, then the player could start playing
these. This isn't expected, because appending files to the playlist in
idle mode does not normally start playback. It could happen because
there is a short time window where commands are processed before the
loop is entered (such as running the command when a script is loaded).
The idle mode semantics are pretty weird: if files were provided in
advance (on the command line), then these should be played immediately.
But if idle mode was already entered, and something is appended to the
playlist using "append", i.e. without explicitly triggering playback,
then it should remain in idle mode.
Try to follow this by redefining PT_STOP to strictly mean idle mode.
Remove the playlist->current check from idle_loop(), since only the
stop_play field counts now (cf. what mp_set_playlist_entry() does).
This actually introduces the possibility that playlist->current, and
with it playlist-pos, are set to something, even though playback is not
active or being started. Previously, this was only possible during state
transitions, such as when changing playlist entries.
Very annoyingly, this means the current way MPV_EVENT_IDLE was sent
doesn't work anymore. Logically, idle mode can be "active" even if
idle_loop() was not entered yet (between the time after mp_initialize()
and before the loop in mp_play_files()). Instead of worrying about this,
redo the "idle-active" property, and deprecate the event.
See: #7543
The code that determines the process exit code ignores all stop_play
values other than PT_QUIT. Generally, PT_ERROR is meaningless outside of
play_current_file(), and is mostly equivalent to PT_NEXT_ENTRY.
Do something that makes it report a non-0 exit code, and indicates in
the terminal exit message that something went wrong.
Untested.
Really minor detail that doesn't really matter. If frame stepping pauses
playback on end (why does this special case even exist), it should
probably be done after on_unload, because all works is supposed to be
finished at that point.
When the current file changes (or rather, when starting/finishing
playback of a playlist entry), clients tend to have the problem that
it's hard to tell whether a property change notification (via
mpv_observe_property() and mechanisms layered on top of it) is from the
previous or new playlist entry. The previous commit probably helps, but
all the asynchronity is still a bit unhelpful.
Try to make this better by adding new hooks, that are run before/after
playback init/deinit. This is similar to the existing hooks, except
they're outside of "initialized" playback, which excludes that you might
accidentally get an overlap between the current and the previous/next
playlist entry.
That still doesn't seem quite enough, since normally, property change
notifications come after the hook event. So basically a client would
have to explicitly "drain" the event queue within the hook, and make the
hook continue only after that is done. Knowing when property
notifications are done is another asynchronous nightmare (how exactly it
works keeps changing within client.c, and an API user probably can't
tell anymore when all pending properties are truly done). So introduce
another guarantee: properties that were changed before the hook happens
will be returned before the hook event is returned. That means the
client will have received all pending property notifications from the
previous playlist entry (or whatever) before the hook is entered.
As another minor complication, we shouldn't just keep the hook pending
until _all_ property notifications are done, since the client's hook
could produce new ones. (Or just consider things like the demuxer thread
hammering the client with cache update events, while the "on_preloaded"
hook is run.) So there is some extra untested, fragile logic in client.c
to handle this (the waiting_for_hook flag).
This probably works, but was barely tested. Not sure if this helps
anyone, but I think it's fine for my own purposes. (I really hated this
aspect of the API whenever I used it myself.)
Some filters may block the playloop for a longer time. For example, if a
decoder fails to decode anything and somehow just discards packets, the
filter graph would run (in a blocking manner) until all packets are
read, which could take a longer time if the demuxer thread is fast
enough.
Make it exit every 100ms. That should at least give the user a chance to
stop playback.
Filtering could run on a different thread, but I don't see much value in
doing that in the general case. It would just waste a thread. Although
being able to use mp_filter_graph_interrupt() would be slightly nicer
than such a timeout mechanism. Decoding in particular can actually use a
separate thread (--vd-queue-enable), but again, this is not enabled by
default, because it just wastes a thread.
Like the previous f_decoder_wrapper commit, this is probably a sin.
This was a hack that attempted to line up external audio tracks with
video. The problem is that if you do a keyframe seek backwards, video
will usually seek much farther back than audio (due to much higher
keyframe aka seek point distances). The hack somehow made seeking a 2
step process.
This existed in 4 different forms in the history of this code base, and
it was always very cumbersome. We mostly needed this for ytdl_hook (I
think?), which uses the 4th form, which is nicely confined to
demux_timeline and is unrelated to the "external" audio tracks in the
high level player.
Since this is (probably) not really widely needed anymore, get rid of
it. Better do this now, than when somehow rewriting all the seeking code
(which might happen in this decade or the next or so) and when it
wouldn't be easily revertable anymore in case we find we "really" need
it unlike expected.
There is no issue if hr-seeks are used. Also, you can still use edl
files to "bundle" multiple streams as if it was a single stream (this is
what ytdl_hook does now).
Hr-seek past the last frame instantly enters EOF, which means
handle_playback_time() will not set playback_pts to the video PTS (as
all video frames are skipped), which leads to the playback time being
taken from the last seek target. This results in confusing behavior,
especially since the seek time will be clipped to the file duration for
display, but not for further relative seeks.
Obviously, the time should be set to the last video frame, so use the
last video frame as fallback if both audio and video have ended. Also,
since the same problem exists with audio-only playback, add a fallback
for audio PTS too. We don't know which was the "last" fragment of media
played (to decide whether to use the audio or video PTS as the
fallback), but it doesn't matter since the maximum works.
This could lead to some undesired effects. In particular the audio PTS
is basically a bad guess, and is for example not clipped against --end.
(But the ridiculous way audio syncing and clamping currently works, I'm
not going to touch that shit unless I rewrite it completely.) The cover
art case is slightly broken: using --keep-open with keyframe seeks will
result in 0 as playback PTS (the video PTS). OK, who cares, it got late.
Also casually get rid of last_vo_pts, since that barely made any sense
at all.
Fixes: #7487
Although a linked list was ideal at first, there are cases where it
sucks, and became increasingly awkward (with the mpv command API
preferring integer indexes to access the list). In future, we probably
want to add more playlist-related functionality, so better change it to
an array now.
An array isn't always ideal either. Since playlist entries are still
separate objects (because in some cases you need a stable "iterator" to
it), but you still need to efficiently get the next/previous playlist
entry, there's a pl_index field, that needs to be maintained. E.g.
adding an entry at the start of the playlist => update the pl_index
field for all other entries. Well, it's not really worth to do something
more complicated to avoid these things.
This commit is probably buggy as shit. It's not like I bothered to test
everything. That's _your_ role.
mpv has a very weak and very annoying policy that determines whether a
playlist should be used or not. For example, if you play a remote
playlist, you usually don't want it to be able to read local filesystem
entries. (Although for a media player the impact is small I guess.)
It's weak and annoying as in that it does not prevent certain cases
which could be interpreted as bad in some cases, such as allowing
playlists on the local filesystem to reference remote URLs. It probably
barely makes sense, but we just want to exclude some other "definitely
not a good idea" things, all while playlists generally just work, so
whatever.
The policy is:
- from the command line anything is played
- local playlists can reference anything except "unsafe" streams
("unsafe" means special stream inputs like libavfilter graphs)
- remote playlists can reference only remote URLs
- things like "memory://" and archives are "transparent" to this
This commit does... something. It replaces the weird stream flags with a
slightly clearer "origin" value, which is now consequently passed down
and used everywhere. It fixes some deviations from the described policy.
I wanted to force archives to reference only content within them, but
this would probably have been more complicated (or required different
abstractions), and I'm too lazy to figure it out, so archives are now
"transparent" (playlists within archives behave the same outside).
There may be a lot of bugs in this.
This is unfortunately a very noisy commit because:
- every stream open call now needs to pass the origin
- so does every demuxer open call (=> params param. gets mandatory)
- most stream were changed to provide the "origin" value
- the origin value needed to be passed along in a lot of places
- I was too lazy to split the commit
Fixes: #7274
Both the "stop" and "loadfile" commands are asynchronous. "stop" starts
terminating playback, which used to be done in the same playloop
iteration, but now it may take longer, so a "loadfile" command can be
received while this is going on. (Possible that it used to work if the
second command was issued at least in the next playloop iteration.)
Make the "loadfile" override the "stop" mode, so the next file will be
played, instead of quitting or going into idle mode.
Unlike the referenced bug report claims, this has nothing to do with
IPC.
Fixes: #7225
Instead of making m_config a special-case, it more or less uses the
underlying m_config_cache/m_config_shadow APIs properly. This makes the
player core a (relatively) equivalent user of the core option API. In
particular, this means that other threads can change core options with
m_config_cache_write_opt() calls (before this commit, this merely led to
diverging option values).
An important change is that before this commit, mpctx->opts contained
the "master copy" of all option data. Now it's just another copy of the
option data, and the shadow copy is considered the master. This is why
whenever mpctx->opts is written, the change needs to be copied to the
master (thus why this commits add a bunch of m_config_notify... calls).
If another thread (e.g. a VO) changes an option, async_change_cb is now
invoked, which funnels the change notification through the player's
layers.
The new self_notification parameter on mp_option_change_callback is so
that m_config_notify... doesn't trigger recursion, and it's used in
cases where the change was already "processed". It's still needed to
trigger libmpv property updates. (I considered using an extra
m_config_cache for that, but it'd only cause problems with no
advantages.)
I think the recent changes actually forgot to send libmpv property
updates in some cases. This should fix this anyway. In some cases,
property updates are reworked, and the potential for bugs should be
lower (probably).
The primary point of this change is to allow external updates, for
example by a VO writing the fullscreen option if the window state is
changed by the window manager (rather than mpv changing it). This is not
used yet, but the following commits will.
As preparation for making repl.lua part of the core (maybe), add some
mechanisms which are supposed to improve its behavior.
Add a silent mode. Calling mpv_request_log_messages() with the log level
name prefixed with "silent:" will disable logging from the API user's
perspective. But it will keep the log buffer, and record new messages,
without returning them to the user. If logging is enabled again by
requesting the same log level without "silent:" prefix, the buffered log
messages are returned to the user at once. This is not documented,
because it's far too messy and special as that I'd want anyone to rely
on this behavior, but it will be perfectly fine for an internal script.
Another thing is that we record early startup messages. The goal is to
make the repl.lua script show option and config parsing file errors.
This works only with the special "terminal-default" log level.
In addition, reduce the "terminal-default" capacity to only 100 log
messages. If this is going to be enabled by default, it shouldn't use
too much resources.
Used to contain flags for "save" setting of options at runtime. Now
there is nothing special needed anymore and it's 0. So drop it
completely, and remove anything that distinguishes between runtime and
initialization time.
Since a track may not be selected twice, it makes sense e.g. for
secondary subtitles to select the next best match and avoid the
duplicate selection.
This allows for example `--slang=en,ja --secondary-sid=auto` to select
'en' as primary and 'ja' as secondary without needing to know the actual
sid for 'ja'.
Looks like this didn't actually work. Prefetching will do nothing if
there isn't a thread to "drive" it, and the demuxer thread needs to be
explicitly enabled. (I guess I did the worst possible job in verifying
whether this actually worked when I implemented it. On the other hand,
the user didn't confirm back whether it worked, so who cares.)
Like in the previous commit, bad factoring makes everything worse. It
duplicates logic and implementation of enable_demux_thread(), since the
opener thread cannot access the mpctx->opts field freely. But it's deep
night, so fuck it.
Fixes: c1f1a0845eFixes: #6753
demux_start_prefetch() was called unconditionally in two cases. This is
completely wrong. I'm not sure what part of my brain died off that
something this obviously wrong went in.
The prefetch case is a bit more complicated. It's a different thread, so
you can't access just access mpctx->opts there. So add an explicit field
for this, which is the simplest way to get this done. (Even if it's bad
factoring.)
Fixes: c1f1a0845e
Fixes: 556e204a11
Although this was sort of elegant, it just seems to complicate things
slightly. Originally, the API meant that you cache mp_recorder_sink
yourself (which would avoid the mess of passing an index around), but
that too seems slightly roundabout.
In a later change, I want to change the set of streams passed to
mp_recorder_create(), and then I'd have to keep track of the index for
each stream, which would suck. With this commit, I can just pass the
unambiguous sh_stream to it, and it will be guaranteed to match the
correct stream.
The disadvantages are barely worth discussing. It's a new linear search
per packet, but usually only 2 to 4 streams are active at a time. Also,
in theory a user could want to write 2 streams using the same sh_stream
(same metadata, just writing different packets or so), but in practice
this is never done.
With the stream cache gone, this function had almost no use anymore
(other than opening the stream). Improve this by triggering demuxer
cache readahead.
This enables all streams. At this point we can't know yet what streams
the user's options would select (at least not without great additional
effort). Generally this is what you want, and the stream cache would
have read the same amount of data.
In addition, this will work much better for files that e.g. need to seek
to the end when opening (typically mp4, and mkv files produced by newer
mkvmerge versions).
Remove the deselection call from add_stream_track(). This should be
fine, as streams normally start out as deselected anyway. In the
prefetch case, some code in play_current_file() actually deselects it.
Streams that appear during demuxing are disabled by default, so this
doesn't break this logic either.
Fixes: #6753
That's right, and it's probably not the end of it. I'll just claim that
I have no idea how to create a proper user interface for this, so I'm
creating multiple partially-orthogonal, of which some may work better in
each of its special use cases.
Until now, there was --record-file. You get relatively good control
about what is muxed, and it can use the cache. But it sucks that it's
bound to playback. If you pause while it's set, muxing stops. If you
seek while it's set, the output will be sort-of trashed, and that's by
design.
Then --stream-record was added. This is a bit better (especially for
live streams), but you can't really control well when muxing stops or
ends. In particular, it can't use the cache (it just dumps whatever the
underlying demuxer returns).
Today, the idea is that the user should just be able to select a time
range to dump to a file, and it should not affected by the user seeking
around in the cache. In addition, the stream may still be running, so
there's some need to continue dumping, even if it's redundant to
--stream-record.
One notable thing is that it uses the async command shit. Not sure
whether this is a good idea. Maybe not, but whatever. Also, a user can
always use the "async" prefix to pretend it doesn't.
Much of this was barely tested (especially the reinterleaving crap),
let's just hope it mostly works. I'm sure you can tolerate the one or
other crash?
The old implementation didn't work for the OGG case. Discard the old
shit code (instead of fixing it), and write new shit code. The old code
was already over a year old, so it's about time to rewrite it for no
reason anyway.
While it's true that the old code appears to be broken, the main reason
to rewrite this is to make it simpler. While the amount of code seems to
be about the same, both the concept and the actual tag handling are
simpler. The result is probably a bit more correct.
The packet struct shrinks by 8 byte. That fact that it wasted 8 bytes
per packet for a rather obscure use case was the reason I started this
at all (and when I found that OGG updates didn't work). While these 8
bytes aren't going to hurt, the packet struct was getting too bloated.
If you buffer a lot of data, these extra fields will add up. Still quite
some effort for 8 bytes. Fortunately, it's not like there are any
managers that need to be convinced whether it's worth doing. The freedom
to waste time on dumb shit.
The old implementation attached the current metadata to each packet.
When the decoder read the packet, the packet's metadata was made
current. The new implementation stores metadata as separate list, and
requires that the player frontend tells it the current playback time,
which will be used to find the currently valid metadata. In both cases,
the objective was to correctly update metadata even if a lot of data is
buffered ahead (and to update them correctly when seeking within the
demuxer cache).
The new implementation is actually slightly more correct, because it
uses the playback time for the metadata lookup. Consider if you have an
audio filter which buffers 15 seconds (unfortunately such a filter
exists), then the old code would update the current title 15 seconds too
early, while the new one does it correctly.
The new code also simplifies mixing the 3 metadata sources (global, per
stream, ICY). We assume these aren't mixed in a meaningful way. The old
code tried to be a bit more "exact". I didn't bother to look how the old
code did this, but the new code simply always "merges" with the previous
metadata, so if a newer tag removes a field, it's going to stick around
anyway.
I tried to keep it simple. Other approaches include making metadata a
special sh_stream with metadata packets. This would have been
conceptually clean, but the implementation would probably have been
unnatural (and doesn't match well with libavformat's API anyway). It
would have been nice to make the metadata updates chapter points (makes
a lot of sense for the intended use case, web radio current song
information), but I don't think it would have been a good idea to make
chapters suddenly so dynamic. (Still an idea to keep in mind; the new
code actually makes it easier to work towards this.)
You could mention how subtitles are timed metadata, and actually are
implemented as sparse packet streams in some formats. mp4 implements
chapters as special subtitle stream, AFAIK. (Ironically, this is very
not-ideal for files. It would be useful for streaming like web radio,
but mp4 is extremely bad for streaming by design for other reasons.)
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
It seems the so called demuxer cache wasn't really disabled for
sub-demuxers (timeline stuff). This was relatively harmless, since the
actual packet data was shared anyway via refcounting. But with the
addition of a mmap cache backend, this may change a lot.
So strictly disable any caching for sub-demuxers. This assumes that
users of sub-demuxers (only demux_timeline.c by now?) strictly use
demux_read_any_packet(), since demux_read_packet_async() will require
some minor read-ahead if a low level packet read returned a packet for a
different stream.
This requires some awkward messing with this fucking heap of trash. The
thing that is really wrong here is that the demuxer API mixes different
concepts, and sub-demuxers get the same API as decoders, and use the
cache code.
Obviously should seek back to the end of the file when it loops.
Also remove some minor code duplication around start times. This isn't
the correct solution by the way. Rather than hoping we know a reasonable
start/end time, this stuff should instruct the demuxer to seek to the
exact location. It'll work with 99% of all normal files, but add an
appropriate comment (that basically says the function is bullshit) to
get_start_time() anyway.
