Repository navigation
Report the codec name, not the decoder name, in probe/1 - #91
Merged
Merged
Conversation
resolve_codec used the name of the first registered decoder, so probe/1 reported "mp3float" for MP3 and "libdav1d" for AV1, and the value changed with the decoders of the FFmpeg build. Read name and long name from avcodec_descriptor_get, as ffprobe does.
# Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before
Exmpeg.probe/1onmain, compared with ffprobe on the same files:Problem
probe/1reports the name of the first registered decoder, not the codec name. The value changes with the decoders in the FFmpeg build, so a source build and the precompiled NIF can report a differentcodecfor the same file.After
codecbeforecodecaftercodec_namea.mp3mp3floatmp3mp3av1.mkvlibdav1dav1av1h264,aac,vp9,opus, andpcm_s16lestay the same. This is a breaking change for Hex users, andCHANGELOG.mdsays so under## Unreleased.Cause, design, tests
Cause.
native/exmpeg_native/src/probe.rs:resolve_codectookname()andlong_name()fromAVCodec::find_decoder(codec_id).Design. ffprobe reads both strings from the codec descriptor. rsmpeg has no safe wrapper for
avcodec_descriptor_get, soffi_helpers::codec_descriptoradds one.ffi_helpersis the only module that holdsunsafe. The function returnsNonefor an id without a descriptor, andresolve_codeckeeps("unknown", None)for that case, so the contract ofExmpeg.Stream.codecstays. The existingffi_helpers::codec_namestays as it is, because concat uses it for its error output.Tests.
probe.rs:AV_CODEC_ID_MP3gives("mp3", Some("MP3 (MPEG audio layer 3)")),AV1givesav1,H264givesh264,AACgivesaac, andAV_CODEC_ID_NONEgives("unknown", None).task test:rust: 19 passed.task check: exit 0 (fmt, compile, lint, tests, zizmor).task test:integration: 104 passed. These tests assert the names that must not change.