configure: enable --enable-external-libraries switch #38

Merged
merged 1 commit into from Oct 9, 2012

Conversation

Projects
None yet
4 participants
Collaborator

wsnipex commented Sep 21, 2012

allows building with external ffmpeg

Collaborator

wsnipex commented Sep 21, 2012

@EricV for intree building something like this is needed: wsnipex/xbmc@65fb794

note: I did not really test this

EricV commented Sep 21, 2012

Thanks. Hope the first part gets merged soon.

Owner

opdenkamp commented Oct 4, 2012

could you rebase it, then i'll merge it in. thanks

Collaborator

wsnipex commented Oct 5, 2012

rebased. Are you going to take care of the xbmc changes needed for intree building or do you want me to do it?

Owner

opdenkamp commented Oct 5, 2012

would be nice if you could create a PR for that too

Collaborator

wsnipex commented Oct 5, 2012

ok, will do

Owner

opdenkamp commented Oct 9, 2012

will see if i update master myself. merging this one

@opdenkamp opdenkamp added a commit that referenced this pull request Oct 9, 2012

@opdenkamp opdenkamp Merge pull request #38 from wsnipex/external-ffmpeg
configure: enable --enable-external-libraries switch
ed77bf7

@opdenkamp opdenkamp merged commit ed77bf7 into opdenkamp:master Oct 9, 2012

Collaborator

wsnipex commented Oct 10, 2012

On 10/10/2012 01:53 AM, Lars Op den
  Kamp wrote:

  will see if i update master myself. merging this one

    —
    Reply to this email directly or view
      it on GitHub. 

I already prepared the stuff for master, just didn't open a PR until
this one is merged.https://github.com/wsnipex/xbmc/commit/2271b9116f73dd2b7a49c94a1939dd88e80b53e1
will rebase and open the PR in case you didn't already take care of
it yourself.
Owner

opdenkamp commented Oct 10, 2012

no i haven't yet

Collaborator

wsnipex commented Oct 10, 2012

On 10/10/2012 10:50 AM, Lars Op den
  Kamp wrote:

  no i haven't yet

    —
    Reply to this email directly or view
      it on GitHub. 

https://github.com/xbmc/xbmc/pull/1591
Owner

opdenkamp commented Oct 10, 2012

thx

Collaborator

theuni commented Oct 16, 2012

@opdenkamp I didn't get involved until too late, but I'm not understanding the principle here. Allowing and encouraging external libs seems like a terrible idea for a binary addon. What's the rationale?

EricV commented Oct 16, 2012

@theuni --external-libraries exists for XBMC aand you can thus use external ffmpeg. This is exactly what many distribution do. Problem is that doing that you select a libavacoded/libavcodec.h that contains defines that are different than the internal ffmpeg version of the file (I dunno why they did not use enum explicit definition with equal but that's another matter)

Without this option, using external ffmpeg, PVR demuxing was just wrong for audio on HDTV in france because it was taking EAC3 for MPEG1 from memory.

And BTW defining binary addon without clear API like XBMC-dev package leads to duplicating a lot of code with all consistency problem. Libavcodec.h being an example among many others.

Owner

opdenkamp commented Oct 16, 2012

@theuni it's only used for headers really by add-ons, for codec ids, and some people were afraid that we'd miss a vital codec id when ffmpeg devs added it ;-)

@EricV: no, that was not the cause of the wrong codec issue, like i already told you back then. the ids were not different.

and yes, we are aware about that -dev package thing. patches welcome.

EricV commented Oct 16, 2012

@opdenkamp and I already answered you that a manual diff really gave a different values given the number of ifdefs, removal and addition... True is that using external ffmpeg did not fix the problem entirely but corrected one half of the pb... And I would love to have more time to dedicate to coding!

Owner

opdenkamp commented Oct 16, 2012

the contents were different, the values, and especially the ones that are actually being used, never changed. so no, it didn't fix the problem and won't fix anyone's (real) problem, but i just got it in to stop people asking about it.

Collaborator

theuni commented Oct 16, 2012

@opdenkamp So it's header-only and this doesn't have any linking effect? Then we need to hurry along the process of using our own codec id's rather than relying on ffmpeg.

@EricV If you have to build your plugin to match your xbmc config, then we've failed somewhere.. it may-as-well be part of xbmc-core then. Hence, Imo this addresses the wrong problem.

Owner

opdenkamp commented Oct 16, 2012

@theuni correct and agreed :)

EricV commented Oct 16, 2012

@theuni True. The configuration of xbmc, if it may influence addon configuration, should be passed one way or other or you should make sure it cannot happen. As external/internal libraries will never be totally in sync and and thus API via .h files, I dunno how you can solve this for reasonably sized addon that will forcibly also use libraries that XBMC may use. And exporting abstract codec id will not solve the problem or force you to either analyse the stream like the demuxer or deliver the stream intact to demuxer

EricV commented Oct 21, 2012

@theuni BTW, simple things like the installation prefix also depends on the way XBMC itself has been compiled. Already proposed a PR for that so that default installation paths for XBMC and PVR addons are at least consistent.

Owner

opdenkamp commented Oct 22, 2012

what pr?

EricV commented Oct 23, 2012

The one your already integrated to have default prefix in /usr/local instead of /usr like XBMC.

Collaborator

theuni commented Oct 24, 2012

/usr/local is always the default (in XBMC as well).

If something is using prefix at runtime it is very broken. Specifics please.

Collaborator

theuni commented Oct 24, 2012

Ok, I see what you mean about /usr/local, that fix was correct.

But the 2nd part still stands. Need specifics.

@Jalle19 Jalle19 added a commit to Jalle19/xbmc-pvr-addons that referenced this pull request May 11, 2014

@Jalle19 Jalle19 Merge pull request #38 from Jalle19/fix-addon-destroy
Fix addon destroy and remove unused variable
99da01b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment