aisouard / libwebrtc

:package: Google's WebRTC implementation in a single static library.
https://axel.isouard.fr/libwebrtc
Apache License 2.0
638 stars 190 forks source link

Support older WebRTC releases #30

Open aisouard opened 7 years ago

aisouard commented 7 years ago

As discussed in #29, older releases using dependencies from the Chromium repository must be supported.

It should be done in two parts:

agouaillard commented 7 years ago

beware of the drift in depot_tools .... you have to get the right revision of depot_tools, and block the auto-update.

aisouard commented 7 years ago

@agouaillard I might have to remove the depot_tools Git submodule, then use CMake to manage it as an ExternalProject.

Right now, I'm thinking about focusing with the releases branch heads, maintaining a sort of database (as a file), telling which version of depot_tools and Chromium CMake should retrieve, depending on the specified branch head specified by the user.

For instance:

{
  "refs/branch-heads/54": {
    "date": "2016-09-07T08:15:32Z",
    "chromium": "e3860bd297e465778059f3d845280634b4074e19", // set in DEPS
    "depot_tools": "fbfa601efda3f683e305ff9c82873dd017e11b82" // commit having the closest date
  }
  "refs/branch-heads/55": {
    "date": "2016-11-28T11:03:32Z",
    "chromium": "a77953fe670968fe6728796b77cedf48f0954d78",
    "depot_tools": "b8c535f696faf93835aa1fe7b99e00cbdc6d5a79"
  }
}

If the user specifies a commit hash instead of a release branch head, the script would have to figure out which branch head was commited before, using the commit date.

Do you know if there would be a simpler solution for this situation ? If I remember correctly, you've told me that forking the Chromium repository and remove the unused dependencies would be a problem due to their license, is there a way to deal with this ?

Auto-update won't be a problem, the environment variable will be set with the prefix.sh file.

agouaillard commented 7 years ago

There is no license problem, unless you compile H.264.

I said that making a copy of an already sync libwebrtc and pushing it to GitHub was eventually not maintainable. It makes sense if you only support one OS, but otherwise, bypassing the DEPS mechanism is a bad idea IMHO. As soon as you support more than one OS, you need to keep in your GitHub all the dependencies, while the DEPS mechanism will only download some parts depending on the host and target OSes, to avoid keeping all in one place.

Google does not commit to maintain old versions of the dependencies ..... In chrome the DEPS file itself is separately hosted in git, and there is also a synchronization between the depot_tools version and the chrome version. Not in libwebrtc .........

One way to deal with it, to avoid keeping a manually curated DB, is to do by date:

This is all automated, now the remaining problem is the generation and build command itself. You will have to handle GYP and GN and all possible flags there. This is a relatively small subset of things to handle compared to the original problem.

Hope This helps.

aisouard commented 7 years ago

Thanks for your advice @agouaillard, I'll focus on managing depot_tools first (#33), then do some kind of abstraction between GYP and GN later.

agouaillard commented 7 years ago

you re welcome. It sounds like the right way to go. There will be more problems going forward, beyond depot_tool versioning and GN/GYP, but I can help when you bump into them. Let's take it step by step ;-)

aisouard commented 7 years ago

Waiting for CI to finish the builds, then I merge the first part into dev.

juliao commented 7 years ago

The branch-heads/58 (now in beta) is failing with this error below, I tested on Ubuntu 16.04 x64 and Windows 8.1 x64, both failed with the same main error:

ninja: error: unknown target 'libjingle_peerconnection' CMakeFiles/webrtc-build.dir/build.make:60: recipe for target 'CMakeFiles/webrtc-build-complete' failed make[5]: *** [CMakeFiles/webrtc-build-complete] Error 1 CMakeFiles/Makefile2:67: recipe for target 'CMakeFiles/webrtc-build.dir/all' failed

FYI, I tested the branch-heads/59 (unstable version) on Ubuntu and the same error happens.

aisouard commented 7 years ago

Thanks for the report.

I'll check which commit removed or renamed this target, then I'll add a git date comparison when I get the time.

juliao commented 7 years ago

Thank you, I just removed the reference to libjingle_peerconnection from webrtc/CMakeLists.txt.in and the compilation was successful, but I did not link some sample application yet, will test the compilation on Windows later.

aisouard commented 7 years ago

In the meantime, you can just remove all references after the output directory, it should look like ninja -C out/Release only.

aisouard commented 7 years ago

Testing under branch release-58