Closed
Bug 1425219
Opened 7 years ago
Closed 7 years ago
Stop using GetNativePath in ffvpx
Categories
(Core :: Audio/Video: Playback, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: emk, Assigned: emk)
References
Details
Attachments
(1 file)
No description provided.
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•7 years ago
|
||
Attached a patch in bug 685236.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 2•7 years ago
|
||
I'll move ffvpx part from bug 685236 to this bug.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from bug 685236 comment #50)
> there's no commit message.
>
> Additionally, the windows replacement for PR_GetLibraryName shouldn't be
> found in this file, it's totally out of scope.
>
> Please, create a generic PR_GetLibraryName that will work on windows as it
> should rather than having multiple implementations wherever it's used
PR_GetLibraryName can't be generic because NSPR is written in C (not C++).
Does it make sense to add NS_GetLibraryName to XPCOM?
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → VYV03354
Status: REOPENED → ASSIGNED
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8950899 [details]
Bug 1425219 - Stop using GetNativePath in ffvpx.
https://reviewboard.mozilla.org/r/220158/#review227056
PathString needs to be better abstracted so that its use is not platform dependent
::: dom/media/platforms/ffmpeg/ffvpx/FFVPXRuntimeLinker.cpp:53
(Diff revision 1)
> #else
> PRLibrary* lib = PR_LoadLibraryWithFlags(lspec, PR_LD_NOW | PR_LD_LOCAL);
> #endif
> if (!lib) {
> - FFMPEG_LOG("unable to load library %s", aName);
> +#ifdef XP_WIN
> + FFMPEG_LOG("unable to load library %s", NS_ConvertUTF16toUTF8(aName).get());
it's rather disappointing that an abstract class like PathString could return a different argument depending on the platform it's running on:
e.g. utf8 or utf16
Why couldn't that be abstracted too?
Attachment #8950899 -
Flags: review?(jyavenard) → review-
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8950899 [details]
Bug 1425219 - Stop using GetNativePath in ffvpx.
https://reviewboard.mozilla.org/r/220158/#review227056
> it's rather disappointing that an abstract class like PathString could return a different argument depending on the platform it's running on:
> e.g. utf8 or utf16
>
> Why couldn't that be abstracted too?
PathString is based on the C++17 std::filesystem::path::string_type that depends on platforms. The purpose of PathString is not such abstraction. Even if we use the same character type, PathString will depend on platforms in some other aspects. For example, the path separator will be different.
We should treat PathString as an opaque type as much as possible. The latest patch uses nsLocalFile more for better abstraction. The last `#ifdef XP_WIN` is inevitable because PR_LibSpec_PathnameU is supported only on Windows and PR_LibSpec_Pathname is lossy on Windows.
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8950899 [details]
Bug 1425219 - Stop using GetNativePath in ffvpx.
https://reviewboard.mozilla.org/r/220158/#review227062
Attachment #8950899 -
Flags: review?(jyavenard) → review+
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/f60cfdc81e94
Stop using GetNativePath in ffvpx. r=jya
Comment 11•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in
before you can comment on or make changes to this bug.
Description
•