Re: Review Request 129325: Fix build with mediastreamer >= 2.14
Vijay Krishnavanshi <[email protected]> Thu, 09 Mar 2017 16:53:41 -0000
| Newsgroups | gmane.comp.kde.devel.kopete |
|---|---|
| Message-ID | <[email protected]> |
> On Nov. 3, 2016, 7:32 p.m., Pali Rohár wrote: > > Can you point to documentation or description of that changed function API? Or e.g. to sources? > > Fabian Vogt wrote: > Unfortunately the official online source is broken (as is the online git browser), but I found a mirror on github. > This is the commit that introduced the API break: https://github.com/BelledonneCommunications/mediastreamer2/commit/483fcd85d98ffdf220e37195575c6dd8209e7d6b#diff-3f018531208181605f23c3477ebcda86 > > I guess it has to be a MSFactory*, acquired by calling ms_factory_new_with_voip() during initialization. > > Pali Rohár wrote: > Hm... I have not found any information what would happen if we pass NULL as first argument. This really needs to be tested and ideally also find some documentation what it say about NULL. I just cannot accept such patch which change compile error to runtime (random) crash because of changed API. Hi I looked up the code base and change in mediastreamer APIs and this patch will causing core dump as this argument MSFactory* factory is being used in if in other functions. So I think more work is needed in this patch and this cannot be merged in its present state. - Vijay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129325/#review100543 ----------------------------------------------------------- On Nov. 3, 2016, 6:01 p.m., Fabian Vogt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129325/ > ----------------------------------------------------------- > > (Updated Nov. 3, 2016, 6:01 p.m.) > > > Review request for Kopete. > > > Repository: kopete > > > Description > ------- > > The API changed with mediastreamer 2.14, so introduce new global defines > to support both the new and the old API. > > > Diffs > ----- > > cmake/modules/FindMediastreamer.cmake 47fe5c3 > protocols/jabber/libjingle/CMakeLists.txt 7ea8c3e > protocols/jabber/libjingle/talk/session/phone/linphonemediaengine.cc bd3de2b > > Diff: https://git.reviewboard.kde.org/r/129325/diff/ > > > Testing > ------- > > It builds. I'm not sure whether passing NULL as first parameter works, it might just crash. > > > Thanks, > > Fabian Vogt > >