Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test/libmpv_test: add default track selection testing #14393

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

test/libmpv_test: add default track selection testing #14393

wants to merge 3 commits into from

Conversation

Dudemanguy
Copy link
Member

The amount of options we have that are related track selection is insane and touching any of this stuff is scary. Add some track selection testing to the CI to hopefully make it slightly less scary to touch any of that. Since trying to work in media files from some outside source would be a pain and need to live in some repo somewhere, it's nicer to just generate dummy files on the fly with ffmpeg during the ci runs and use that. Obviously, the actual tests themselves could be even more thorough (external tracks were not even considered), but this is more than a good enough start for now.

Read this before you submit this pull request:
https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md

Reading this link and following the rules will get your pull request reviewed
and merged faster. Nobody wants lazy pull requests.

@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 4 times, most recently from d9d5e5c to cad0996 Compare June 20, 2024 04:35
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 20, 2024 •
edited
Loading

Download the artifacts for this pull request:

Windows
macOS

@Dudemanguy
Copy link
Member Author

Dudemanguy commented Jun 20, 2024 •
edited
Loading

Just for the record, I excluded the mingw, win32 (laziness) , openbsd (the ffmpeg commands got stuck), and ffmpeg 4.4 (the ffmpeg commands are bugged and give wrong output) from running the track selection tests.

Edit: dated.

kasper93
kasper93 reviewed Jun 20, 2024
View reviewed changes
ci/generate-test-media.sh Outdated

# Create core video, audio, and subtitle streams to construct files out of.
ffmpeg -y -f lavfi -i testsrc=duration=2:size=1280x720 video.mkv
ffmpeg -y -f lavfi -i sine=frequency=1000:duration=2 audio.flac
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't be better to do it with mpv?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would personally prefer avoiding mpv's encoding mode.

@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 2 times, most recently from 435b1c8 to 0a8dda2 Compare June 20, 2024 14:40
kasper93
kasper93 reviewed Jun 20, 2024
View reviewed changes
test/meson.build Outdated Show resolved Hide resolved
kasper93
kasper93 reviewed Jun 20, 2024
View reviewed changes
test/meson.build Outdated Show resolved Hide resolved
@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 2 times, most recently from 9feae32 to 7e70efe Compare June 22, 2024 14:20
sfan5
sfan5 reviewed Jun 22, 2024
View reviewed changes
test/libmpv_test.c Outdated Show resolved Hide resolved
@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 3 times, most recently from a9abe5d to 9a560fd Compare January 24, 2025 04:04
@Dudemanguy
Copy link
Member Author

Dudemanguy commented Jan 24, 2025 •
edited
Loading

I reworked this so the libmpv_test.c file is split into separate files for every single test it does. Relevant functions are shared inline in libmpv_shared.h now. libmpv_test_track_selection.c uses meson custom targets as well.

edit: openbsd fails the test for some reason. Will take a look later.

@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 4 times, most recently from d8d8768 to 53536d8 Compare January 24, 2025 16:57
@Dudemanguy
Copy link
Member Author

Dudemanguy commented Jan 24, 2025 •
edited
Loading

openbsd fails the test for some reason. Will take a look later.

Fixed now. It's because openbsd had the old ffmpeg version (4.4) in its path and that version has a bug when setting forced tracks causing the test to fail. I don't believe there is a way for find_program to find something at compile time instead of configure time (at least it didn't seem to work for me), so it doesn't seem possible to use the ffmpeg binary that would be generated during compilation. So instead, I just added some version checks so this particular test is skipped.

@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch from 53536d8 to 652763b Compare January 24, 2025 21:37
kasper93
kasper93 reviewed Jan 25, 2025
View reviewed changes
test/samples/meson.build Outdated Show resolved Hide resolved
test/samples/meson.build Outdated Show resolved Hide resolved
test/samples/meson.build Outdated Show resolved Hide resolved
test/samples/meson.build Outdated Show resolved Hide resolved
test/libmpv_shared.h Outdated Show resolved Hide resolved
test/libmpv_shared.h Outdated Show resolved Hide resolved
test/libmpv_shared.h Outdated Show resolved Hide resolved
test/libmpv_shared.h Outdated Show resolved Hide resolved
test/libmpv_test_file_loading.c Outdated Show resolved Hide resolved
test/libmpv_test_track_selection.c Outdated Show resolved Hide resolved
@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 2 times, most recently from 0d912e4 to e9a99c3 Compare January 25, 2025 01:57
@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 2 times, most recently from 07a1ed0 to c95dc47 Compare January 25, 2025 04:21
@sfan5 sfan5 self-requested a review January 25, 2025 10:34
kasper93
kasper93 reviewed Jan 25, 2025
View reviewed changes
test/samples/meson.build Show resolved Hide resolved
test/samples/meson.build Outdated Show resolved Hide resolved
@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch 2 times, most recently from 9da5616 to 3416af5 Compare January 25, 2025 18:24
@kasper93
Copy link
Contributor

Program ffmpeg found: NO

I think we should ensure that at least on linux ci the ffmpeg is there.

@Dudemanguy
Copy link
Member Author

I think we should ensure that at least on linux ci the ffmpeg is there.

It just needs to thrown on the tumbleweed container. cc @llyyr.

kasper93
kasper93 reviewed Jan 28, 2025
View reviewed changes
test/samples/meson.build Outdated Show resolved Hide resolved
kasper93
kasper93 reviewed Jan 28, 2025
View reviewed changes
test/samples/meson.build Outdated Show resolved Hide resolved
@Dudemanguy
test: group together all the libmpv tests under the libmpv suite
5053368
@Dudemanguy
test: split libmpv_test into separate files
33413dc 
The big libmpv test file is really actually three different tests shoved
together into one. They do all use fairly similar underlying structures,
so put the shared stuff into an inline header. There's a little bit of
duplication with regards to the main function, but it's cleaner to
separate out the tests.
@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch from 3416af5 to 10c6870 Compare January 28, 2025 21:33
kasper93
kasper93 reviewed Jan 28, 2025
View reviewed changes
test/samples/meson.build Outdated Show resolved Hide resolved
@Dudemanguy
test: add default track selection testing
ad51b84 
The amount of options we have that are related track selection is insane
and touching any of this stuff is scary. Add some track selection
testing to the CI to hopefully make it slightly less scary to touch any
of that. Since trying to work in media files from some outside source
would be a pain and need to live in some repo somewhere, it's nicer to
just generate dummy files on the fly with ffmpeg during the ci runs and
use that. Obviously, the actual tests themselves could be even more
thorough (external tracks were not even considered), but this is more
than a good enough start for now.
@Dudemanguy Dudemanguy force-pushed the test-default-track-selection branch from 10c6870 to ad51b84 Compare January 28, 2025 23:26
kasper93
kasper93 approved these changes Jan 28, 2025
View reviewed changes
Copy link
Contributor

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kasper93 kasper93 kasper93 approved these changes

@sfan5 sfan5 Awaiting requested review from sfan5

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Dudemanguy @kasper93 @sfan5