Track selection cleanup by filip-hejsek · Pull Request #17280 · mpv-player/mpv

filip-hejsek · 2026-01-17T20:41:44Z

This is a cleanup of track selection code with the aim to:

remove inconsistencies and broken behavior in some rare edge cases
make it easier to modify the track selection logic (I plan to follow this up with an implementation of Proposal: a more reasonable order in compare_track #6071)

The changes are separated into multiple commits so that each commit can be reviewed individually. I ran the test suite (meson test) after each commit.

filip-hejsek · 2026-01-17T20:57:56Z

I'm currently compiling a list of some edge cases where the behavior changes. I will post it here when I finish.

kasper93 · 2026-01-18T05:47:12Z

Those changes break subs-match-os-language=yes selection, see the test output. Also please keep commit message short (<72 chars)

I'm currently compiling a list of some edge cases where the behavior changes. I will post it here when I finish.

You can update the tests with all corner cases you wish to be fixed. The tests is somehow our documentation how exactly track selection should work.

Dudemanguy · 2026-01-18T05:57:23Z

Does it? That might just be that weird race condition that sometimes happens in the CI. Doesn't make sense that the other msys2 all pass fine.

The first two commits are most likely just fine. The next two not so sure.

filip-hejsek · 2026-01-18T06:37:57Z

I have finished creating test cases for the behavior changes.

Likely bugs

I believe these to be bugs in the current implementation:

BUG 1a: Selection between two forced subtitle tracks that match audio language is sometimes incorrect (last one wins; ignores default flag and OS lang)
BUG 1b: Selection between two forced subtitle tracks that don't match audio language is sometimes incorrect (first one wins; ignores OS lang)
BUG 2: A forced track that matches audio language is not selected in some cases when it should be (a different track ordered after it replaces it because of OS lang)
BUG 3: A forced subtitle track is sometimes selected when a non-forced one should be chosen instead (depends on track order; chosen because of OS lang or tid)
BUG 4: The presence of an eligible subtitle track can cause an ineligible fallback track to be selected (e.g. the presence of a track that matches --slang can cause mpv to select a different non-matching track based on program id, when without the first track, no subtitles would be selected)

Please correct me if some of this behavior is intended.

Test cases

The media files used by the test cases can be generated with this script: prepare_test_cases.sh (requires ffmpeg and mkvmerge)

The test cases assume default config and OS language set to English. You can use run_test_cases.sh to run all of them at once. I will figure out how to add them to the test suite later if this is desired.

A list of test cases, with comparison of behavior before/after changes

`mpv forced_default_vs_just_forced_with_audio.mka`

Before: sid=2 is selected because it is last (due to BUG 1a).
After: sid=1 is selected because it has default flag.

`mpv forced_cs_vs_forced_en.mks`

Before: sid=1 is selected because it is first (due to BUG 1b).
After: sid=2 is selected because it matches OS language.

`mpv forced_cs_vs_en_with_cs_audio.mka`

Before: sid=2 is selected because it matches OS language. (Due to BUG 2, a forced track that should have been preferred is not selected.)
After: sid=1 is selected because it is forced and matches audio language.

`mpv cs_vs_forced_en.mks`

Before: sid=2 is selected because it matches OS language, even though it is forced and doesn't match audio language. (Due to BUG 3, a non-forced track is not selected.)
After: sid=1 is selected because it is not forced.

`mpv forced_en_vs_cs.mks`

Before: sid=2 is selected because it is not forced.
After: sid=2 is selected because it is not forced (unchanged).

(Compare this with the previous case)

`mpv --vid=2 --slang=en multiprogram.ts`

Before: sid=1 is first considered as a candidate because it matches --slang, but then sid=2 is selected because its program ID matches the video. However, if the language of sid=1 didn't match, sid=2 wouldn't be considered and no track would be chosen. (Due to BUG 4, the presence of a candidate track can cause a different and normally ineligible track to be selected.)
After: sid=1 is selected because it matches --slang.

`mpv --subs-fallback=no forced_cs_vs_en_with_cs_audio.mka`

Before: This is a combination of BUG 2 & BUG 4: sid=2 is selected, even though sid=1 should have been preferred (BUG 2) and no track at all would have been selected without sid=1 present (BUG 4).
After: sid=1 is selected because no other track is eligible.

kasper93 · 2026-01-18T06:43:47Z

Does it? That might just be that weird race condition that sometimes happens in the CI. Doesn't make sense that the other msys2 all pass fine.

Only clang64 is built with ASAN, which triggers the issue. There is uninitialized variable, you can repro this also with -ftrivial-auto-var-init=pattern too.

kasper93 · 2026-01-18T06:51:13Z

The media files used by the test cases can be generated with this script: prepare_test_cases.sh (requires ffmpeg and mkvmerge)

For test cases, please use directly our meson testing, it will be easier to track changes.

Files are generated here:

mpv/test/samples/meson.build

Lines 20 to 22 in 468d34c

    
           samples = { 
        
               'eng_default.mkv': 
        
                   [ffmpeg, common_args, '-disposition:s:0', 'default', '@OUTPUT@'],

And tested in libmpv_test_track_selection.c

Use track->forced_select instead. The value would be assigned there anyway after the track is selected and the field is not used for any other purpose.

This brings compare_track closer to implementing a consistent linear order and fixes many broken edge cases.

This fixes the problem that a subtitle track that normally wouldn't be selected if it were the only track available could end up being selected after a different candidate track was selected first.

When autoload-files=no, external files are never selected. Simplify the logic by excluding them up front instead of removing them afterwards. This doesn't change the outcome of track selection, apart from edge cases that are already broken because compare_track is not transitive.

The new test case exercises --subs-fallback-forced=always and verifies that default+forced is preferred over forced.

This is a copy of the existing locale test, but with forced flag added to all tracks. The forced flag should not affect language selection in this case.

filip-hejsek · 2026-01-18T11:54:33Z

I rebased the PR, updated commit descriptions (all commits now have an informative description) and added some tests.

Please note that the tests I added to the test suite do not yet cover all test cases in the comment above.

github-actions · 2026-01-18T12:12:35Z

Download the artifacts for this pull request:

Windows

macOS

kasper93 · 2026-01-19T23:52:36Z

Please note that the tests I added to the test suite do not yet cover all test cases in the comment above.

Please, let us know when you consider this ready for review, we can merge all the improvements in one go once they are ready.

filip-hejsek · 2026-01-20T00:15:14Z

Please note that the tests I added to the test suite do not yet cover all test cases in the comment above.

Please, let us know when you consider this ready for review, we can merge all the improvements in one go once they are ready.

I won't change the existing commits, so they are technically ready for review already, although I can understand if you want to review everything at once.

For one of the test cases, I need a file with multiple programs and subtitles. Unfortunately, the only container supporting multiple programs that I know of is MPEG-TS, and it only supports image subtitles. I couldn't find an easy way of generating image subtitles, so I instead used a file from ffmpeg test suite: https://fate-suite.ffmpeg.org/sub/dvbsubtest_filter.ts

The script in my previous comment fetches the file from that URL, but I don't think doing that is acceptable in the test suite - there are no guarantees the file won't be taken down.

For reference, this is the command used to generate the test file with multiple programs:

ffmpeg \
    -f lavfi -i color=white:2x2:d=0.1 \
    -i https://fate-suite.ffmpeg.org/sub/dvbsubtest_filter.ts \
    -map 0:v -map 1:s \
    -map 0:v -map 1:s \
    -c:v libx264 -c:s dvbsub \
    -map_metadata -1 \
    -program program_num=1:st=0:st=1 \
    -program program_num=2:st=2:st=3 \
    -metadata:s:1 language=eng \
    -metadata:s:3 language=cze \
    -f mpegts multiprogram.ts

kasper93 · 2026-01-20T01:17:23Z

You can add a sample bitmap subtitle file to https://github.com/mpv-player/mpv/tree/master/test/samples as we already have srt files there. I would avoid adding full .ts sample there, as generating them gives us flexibility to generate various mpegts with this single input file. Subtitle Edit can export vobsub, which you can later use to mux into mpegts as dvbsub.

The purpose of this test is mostly to verify that matching the OS language is not wrongly prioritized over other things (like the forced flag, matching the audio language, etc).

filip-hejsek · 2026-01-20T06:26:04Z

I have added all the tests now.

Unfortunately, CI is failing because it has a ffmpeg build without a dvbsub encoder.

filip-hejsek · 2026-01-20T06:39:29Z

Also, the pre-commit hooks are complaining about the subtitle index...

Maybe it would be easier to include a matroska file with just dvb subtitles, and then just remux that to mpegts.

kasper93 · 2026-01-20T06:41:09Z

Also, the pre-commit hooks are complaining about the subtitle index...

Maybe it would be easier to include a matroska file with just dvb subtitles, and then just remux that to mpegts.

You can include dvbsub itself.

filip-hejsek · 2026-01-20T06:44:26Z

My ffmpeg doesn't have raw dvbsub muxer (only demuxer) and I think SubtitleEdit doesn't support dvb subtitles either (or maybe it does and I just didn't find it)

kasper93 · 2026-01-20T06:54:43Z

My ffmpeg doesn't have raw dvbsub muxer (only demuxer) and I think SubtitleEdit doesn't support dvb subtitles either (or maybe it does and I just didn't find it)

let's fix CI image then.

kasper93 · 2026-01-20T07:00:20Z

Also, the pre-commit hooks are complaining about the subtitle index...

It's typo in comment, I suggest fixing it manually, or even removing all comments from this file, they are not needed and will save on filesize.

llyyr · 2026-01-20T09:59:51Z

dvbsub isn't available on the linux ci for now, I'll add it but it'll take a few days to make it into the image

Dudemanguy · 2026-01-21T00:46:24Z

Thanks for all the hard work on this and building all the tests. I'll try to double check on my end later this week but I highly suspect you are correct in everything. And since the tests are all passing (old and new ones), that's a good sign.

kasper93 · 2026-01-21T01:31:45Z

test/libmpv_test_track_selection.c

+        check_string("track-list/3/selected", "no");
+
+        // because subs-fallback=default, the jpn track cannot be chosen
+        // so we choose the eng track despite it having the wrong program ID


despite it having the wrong program ID

Is this really what we expect to happen?

I don't know, I have just attempted to mostly preserve the existing behavior while removing inconsistencies, but I agree this behavior doesn't make much sense.

To better understand the issue, I think it is helpful to consider a simpler case where subs-fallback doesn't play a role. Consider a file with the following tracks:

video, program ID = 1

video, program ID = 2

subs, default, program ID = 2

When the first video is selected, should the subtitles with wrong program ID be used, or is it better to have no subtitles at all? Currently, mpv prefers wrong program to no track at all, but maybe that isn't right. And what if instead of subtitles it was an audio track?

I think selecting track (audio or subtitle) from wrong PID is wrong and should instead select none in this case. (regardless of any fallback options)

Also, on that note, selecting video track from different PID, imho, should reselect other tracks. So the multi program files are usable in mpv. I know it's outside the scope of this PR, but I wanted to point this out. Generally we don't have menu for program selection, it could be plugged into edition selection same as dvd/bluray titles are. Serves the same purpose. And the displayed/selected tracks should consist only of matching PID.

Sorry for offtopic. I just had this though, because I think it's low hanging fruit to greatly improve multiprogram support...

I've changed the selection logic to never choose a track with the wrong PID, as this was simple enough to do. Further improvements will be left for future PRs.

(On the topic of improving multiprogram support, I also want to note down one more problem with the current implementation: I think it doesn't work on files with no video. Edition selection really seems like a much better fit for this than autoselect.)

Dudemanguy

The program id stuff can for another day if we want to iron out that should work (niche within a niche really). But that's unrelated and this PR is indeed a nice cleanup. I'll merge once dvbsub makes its way into our CI image.

kasper93 · 2026-01-23T18:12:05Z

Could you change multiprogram test file to mpeg2 (or anything else)? Apparently our CI image doesn't have a h264 decoder, so the test fails.

kasper93 · 2026-01-23T18:15:31Z

test/samples/meson.build

         '-disposition:s:0', 'default', '-disposition:s:1', 'forced', '-disposition:s:3', 'default',
         '@OUTPUT@'],
+    'multiprogram.ts':
+        [ffmpeg, '-v', 'error', '-y', '-i', video, '-f', 'vobsub', '-i', img_sub,


Why not with audio? It can influence the selection too.

Mostly because I didn't care enough about multiprogram files to put effort into making comprehensive tests, but also, currently only the video determines preferred program ID:

int preferred_program = (type != STREAM_VIDEO && mpctx->current_track[0][STREAM_VIDEO]) ? mpctx->current_track[0][STREAM_VIDEO]->program_id : -1;

filip-hejsek · 2026-01-23T20:37:45Z

Could you change multiprogram test file to mpeg2 (or anything else)? Apparently our CI image doesn't have a h264 decoder, so the test fails.

I'm pretty sure the multiprogram file uses the same video codec as the others, because it is created with -c:v copy like the others. I don't understand why this is a problem only for mpegts but not for the mkv files. It seems that lavf somehow fails to even recognize what codec is used? It seems to get detected as bin_data: [ffmpeg/demuxer:debug] mpegts: parser not found for codec bin_data, packets or times may be invalid.

filip-hejsek · 2026-01-23T21:12:07Z

Could you change multiprogram test file to mpeg2 (or anything else)? Apparently our CI image doesn't have a h264 decoder, so the test fails.

I'm pretty sure the multiprogram file uses the same video codec as the others, because it is created with -c:v copy like the others. I don't understand why this is a problem only for mpegts but not for the mkv files. It seems that lavf somehow fails to even recognize what codec is used? It seems to get detected as bin_data: [ffmpeg/demuxer:debug] mpegts: parser not found for codec bin_data, packets or times may be invalid.

The problem was that ffmpeg chose vp9 when creating video.mkv, and MPEG-TS doesn't support vp9.

I have changed video.mkv to use mpeg2, so all tests use mpeg2 now. Is that okay?

Dudemanguy · 2026-01-23T22:25:30Z

Should be OK I think.

Currently, we prefer selecting a track that matches the video program ID, but can fall back on a track with a different program ID when there is no such track or when no such track matches subs-fallback criteria. Change this to instead select no track when only tracks with wrong PID are available. Program ID will now act as a filter, not as a preference, and only files with either matching or unset PID will be considered for autoselect. Also, ignore the program ID of external tracks, as it doesn't make much sense to compare PIDs from different files.

In CI, ffmpeg chooses vp9 by default, which cannot be muxed into mpegts.

filip-hejsek · 2026-01-23T23:59:49Z

So my first attempt at limiting track selection to matching PID actually broke --sub-file and --audio-file when the main file has PIDs (because the PID of the external file wouldn't match), and this wasn't caught by any test. But I have fixed this now. The new logic is that a track is considered for autoselect if either:

a video with no PID is selected, or
the track has the same PID as the video, or
the track has no PID, or
the track is external (because it doesn't make much sense to compare PIDs across files).

kasper93

LGTM, thanks!

We can iterate on remaining issues in future changes.

filip-hejsek added 6 commits January 18, 2026 12:27

player/loadfile: remove forced param from compare_track

9ec6054

Use track->forced_select instead. The value would be assigned there anyway after the track is selected and the field is not used for any other purpose.

player/loadfile: clean up track comparison logic

361cbb4

This brings compare_track closer to implementing a consistent linear order and fixes many broken edge cases.

player/loadfile: simplify track selection logic

abd38aa

This fixes the problem that a subtitle track that normally wouldn't be selected if it were the only track available could end up being selected after a different candidate track was selected first.

test/libmpv_test_track_selection: add additional multilang test case

13bf3b9

The new test case exercises --subs-fallback-forced=always and verifies that default+forced is preferred over forced.

test/libmpv_test_track_selection: add a locale test with forced tracks

9d07e0e

This is a copy of the existing locale test, but with forced flag added to all tracks. The forced flag should not affect language selection in this case.

filip-hejsek force-pushed the track-selection-cleanup branch from 7e88dcd to 9d07e0e Compare January 18, 2026 11:51

test/libmpv_test_track_selection: add a more complex locale test

256f6cf

The purpose of this test is mostly to verify that matching the OS language is not wrongly prioritized over other things (like the forced flag, matching the audio language, etc).

filip-hejsek force-pushed the track-selection-cleanup branch from cd8544a to 986d07d Compare January 20, 2026 06:21

test/libmpv_test_track_selection: add a program ID matching test

df08c40

filip-hejsek force-pushed the track-selection-cleanup branch from 986d07d to df08c40 Compare January 20, 2026 07:02

Dudemanguy self-requested a review January 21, 2026 00:46

kasper93 reviewed Jan 21, 2026

View reviewed changes

kasper93 added this to the Release v0.42.0 milestone Jan 23, 2026

Dudemanguy approved these changes Jan 23, 2026

View reviewed changes

kasper93 reviewed Jan 23, 2026

View reviewed changes

filip-hejsek added 2 commits January 24, 2026 00:48

test/samples: use mpeg2 codec for video samples

9f7a3c3

In CI, ffmpeg chooses vp9 by default, which cannot be muxed into mpegts.

filip-hejsek force-pushed the track-selection-cleanup branch from 47ead65 to 9f7a3c3 Compare January 23, 2026 23:48

kasper93 approved these changes Jan 24, 2026

View reviewed changes

kasper93 merged commit 7298ce1 into mpv-player:master Jan 24, 2026
29 checks passed

filip-hejsek deleted the track-selection-cleanup branch January 24, 2026 13:37

Conversation

filip-hejsek commented Jan 17, 2026

Uh oh!

filip-hejsek commented Jan 17, 2026

Uh oh!

kasper93 commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dudemanguy commented Jan 18, 2026

Uh oh!

filip-hejsek commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Likely bugs

Test cases

mpv forced_default_vs_just_forced_with_audio.mka

mpv forced_cs_vs_forced_en.mks

mpv forced_cs_vs_en_with_cs_audio.mka

mpv cs_vs_forced_en.mks

mpv forced_en_vs_cs.mks

mpv --vid=2 --slang=en multiprogram.ts

mpv --subs-fallback=no forced_cs_vs_en_with_cs_audio.mka

Uh oh!

kasper93 commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kasper93 commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

filip-hejsek commented Jan 18, 2026

Uh oh!

github-actions bot commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kasper93 commented Jan 19, 2026

Uh oh!

filip-hejsek commented Jan 20, 2026

Uh oh!

kasper93 commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

filip-hejsek commented Jan 20, 2026

Uh oh!

filip-hejsek commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kasper93 commented Jan 20, 2026

Uh oh!

filip-hejsek commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kasper93 commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kasper93 commented Jan 20, 2026

Uh oh!

llyyr commented Jan 20, 2026

Uh oh!

Dudemanguy commented Jan 21, 2026

Uh oh!

kasper93 Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

filip-hejsek Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

kasper93 Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kasper93 Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

filip-hejsek Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dudemanguy left a comment

Choose a reason for hiding this comment

Uh oh!

kasper93 commented Jan 23, 2026

kasper93 commented Jan 18, 2026 •

edited

Loading

filip-hejsek commented Jan 18, 2026 •

edited

Loading

`mpv forced_default_vs_just_forced_with_audio.mka`

`mpv forced_cs_vs_forced_en.mks`

`mpv forced_cs_vs_en_with_cs_audio.mka`

`mpv cs_vs_forced_en.mks`

`mpv forced_en_vs_cs.mks`

`mpv --vid=2 --slang=en multiprogram.ts`

`mpv --subs-fallback=no forced_cs_vs_en_with_cs_audio.mka`

kasper93 commented Jan 18, 2026 •

edited

Loading

kasper93 commented Jan 18, 2026 •

edited

Loading

github-actions bot commented Jan 18, 2026 •

edited

Loading

kasper93 commented Jan 20, 2026 •

edited

Loading

filip-hejsek commented Jan 20, 2026 •

edited

Loading

filip-hejsek commented Jan 20, 2026 •

edited

Loading

kasper93 commented Jan 20, 2026 •

edited

Loading

kasper93 Jan 21, 2026 •

edited

Loading

filip-hejsek Jan 23, 2026 •

edited

Loading

filip-hejsek commented Jan 23, 2026 •

edited

Loading