Repository navigation
fix(mediaplayer): build file URIs that work on Windows - #959
Merged
Merged
Conversation
normalize_uri and two media player tests pasted path.display() after file://, which on Windows gives file://C:\media\clip.mp4: no source element accepts it, so the Media Player could not play a local file on Windows, and three of its tests failed there once #945 landed (Windows only builds on main or with the ci:windows label). file_uri now uses glib::filename_to_uri, which handles the drive, backslashes and escaping, and keeps the old form for a path it refuses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The two test-file writers put the path into a gst::parse::launch string. There a Windows path's backslashes are read as escapes, so the file was written somewhere else and the URI the tests then played pointed at nothing: the three tests still failed on Windows with StateChangeError after the file URI fix. file_writer sets the location as a property. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
Description
normalize_uriand two media player tests built file URIs asformat!("file://{}", path.display()). On Windows that givesfile://C:\media\clip.mp4, which no source element accepts. So the Media Player could not play a local file on Windows. Since #945 landed, three of its tests have also failed there withStateChangeError(seen in #879's Windows build):decodebin3_decodes_only_the_tracks_it_has_outputs_for,each_audio_track_gets_its_own_output_when_there_are_enoughandplayback_carries_on_in_time_after_a_file_switch. Windows builds only onmainor with theci:windowslabel, so #945's own CI did not catch it.file_urinow usesglib::filename_to_uri, which handles the drive letter, backslashes and escaping. A path it refuses (one that is not absolute) keeps the old form. The tests use the same function.The test writers needed a fix too. With the URI fix alone, the three tests still failed on Windows. The tests write their media files with
gst::parse::launch("... filesink location=<path>"), and in a launch string a Windows path's backslashes are read as escapes. The file was written somewhere else, so the URI pointed at nothing.file_writernow setslocationas a property.Type of Change
Testing
cargo test --lib mediaplayeron macOS: 28 passed, including the newa_file_uri_turns_back_into_its_path. That test builds a URI for a temp path with spaces and requiresglib::filename_from_urito give the same path back. On Windows the old form fails that.ci:windowslabel. It runs the three failing tests and the new one.cargo clippy --all-targets -- -D warningsandcargo fmtare clean.🤖 Generated with Claude Code