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.

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

Already on GitHub? Sign in to your account

stats.lua: add codec profile #13841

Merged
merged 3 commits into from
Apr 15, 2024
Merged

stats.lua: add codec profile #13841

merged 3 commits into from
Apr 15, 2024

Conversation

kasper93
Copy link
Contributor

@kasper93 kasper93 commented Apr 8, 2024

No description provided.

Copy link

github-actions bot commented Apr 8, 2024

Download the artifacts for this pull request:

Windows
macOS

@kasper93 kasper93 added this to the Release v0.38.0 milestone Apr 8, 2024
@kasper93 kasper93 force-pushed the dec_desc branch 3 times, most recently from 84a8dc8 to 7bd5462 Compare April 10, 2024 17:09
@kasper93 kasper93 marked this pull request as draft April 10, 2024 18:49
@Andarwinux
Copy link
Contributor

Why did this become a draft? It worked fine.

@kasper93 kasper93 force-pushed the dec_desc branch 2 times, most recently from cd5e7a5 to 62a0c03 Compare April 11, 2024 18:49
@kasper93 kasper93 changed the title f_decoder_wrapper: add codec profile to decoder description stats.lua: add codec profile Apr 11, 2024
@kasper93 kasper93 marked this pull request as ready for review April 11, 2024 18:49
@kasper93
Copy link
Contributor Author

Why did this become a draft? It worked fine.

Previous version has been rejected on IRC. New version is available now.

@kasper93
Copy link
Contributor Author

kasper93 commented Apr 13, 2024

I would like to have it in this release, but if it don't get reviewed in time, we can drop it. So don't consider it blocking.

@kasper93 kasper93 force-pushed the dec_desc branch 3 times, most recently from f89bb97 to 9f7c638 Compare April 15, 2024 16:53
Adds support for extracting codec profile. Old properties are redirected
to new one and removed from docs. Likely will stay like that forever as
there is no reason to remove them.

As a effect of unification of properties between audio and video,
video-codec will now print codec (format) descriptive name, not decoder
long name as it were before. In practice this change fixes what docs
says. If you really need decoder name, use the `track-list/N/decoder-desc`.
May be interesting information for users.

Fixes: mpv-player#13839
Copy link
Member

@jeeb jeeb left a comment

Choose a reason for hiding this comment

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

Code-wise for the new properties LGTM.

Not sure how much of an API promise we have regarding these existing properties, but at least the commit message now mentions that there are changes regarding {video,audio}-codec's contents.

@kasper93
Copy link
Contributor Author

Not sure how much of an API promise we have regarding these existing properties, but at least the commit message now mentions that there are changes regarding {video,audio}-codec's contents.

Coded description is unlikely to be used differently than just printing its value. For format matching the short name should be used. Also the new behavior arguably matches current documentation. (and the value of this prop has been changed not so long ago and there was no complaints)

@kasper93 kasper93 merged commit ab50451 into mpv-player:master Apr 15, 2024
15 checks passed
@kasper93 kasper93 deleted the dec_desc branch April 15, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants