build: add --disable-ffmpeg flag. #912

Closed
wants to merge 1 commit into
from

7 participants

@theuni
Team Kodi member

I can already hear the screams... @elupus: I'm prepared for my lashings.

Some who package XBMC may be using hardware decoders and don't want to take on the legal responsibility of shipping ffmpeg. This provides that option. It is meant to go along with PR 911, but split off as this one is just an RFC.

Clearly this is just to show what needs to happen to remove ffmpeg from build.
It is not clean or pretty, and is only meant to start the discussion.
In order to do this cleanly, a few things should happen:

  • We need an internal image scaling/resampling class with swscale as one of
    the implementations. That will avoid the ifdefs for swscale all over.

  • We need our own representation of codec id's. These could map to ffmpeg's
    if it's available, but stand alone if not.

theuni build: add --disable-ffmpeg flag.
Clearly this is just to show what needs to happen to remove ffmpeg from build.
It is not clean or pretty, and is only meant to start the discussion.
In order to do this cleanly, a few things should happen:

- We need an internal image scaling/resampling class with swscale as one of
  the implementations. That will avoid the ifdefs for swscale all over.

- We need our own representation of codec id's. These could map to ffmpeg's
  if it's available, but stand alone if not.
d3799e6
@theuni
Team Kodi member

I can already see this is going to be a long discussion. But I hope we can keep this about the removal implementation itself. If in the end it's nixed, I can live with that.

The issue is that these libs (ffmpeg in this case) are poisonous to attorneys. So details like how they're built, what's included, what features are enabled, etc. are irrelevant. Obviously there are many ways to disable offending codecs/implementations, but the core issue here is that they need to be disabled wholesale. Please review with that in mind.

Think of hardware like the raspberry pi, which may be perfect for a tiny, cheap, streaming device. As all playback is handled in hardware, and it is not powerful enough to do in software anyway, the sw decoders are only a liability.

@theuni
Team Kodi member

Also, in case it's not obvious, this won't be the default, and nothing about the way we build/release will change.

@elupus elupus commented on the diff May 1, 2012
xbmc/cores/VideoRenderers/LinuxRendererGLES.cpp
RenderSoftware(index, m_currentField);
VerifyGLState();
+#endif
@elupus
Team Kodi member
elupus added a line comment May 1, 2012

Seem pointless..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus elupus commented on the diff May 1, 2012
xbmc/cores/dvdplayer/DVDCodecs/Audio/DVDAudioCodec.h
#include "DllAvCodec.h"
+#else
+#include "FFMpegInternals.h"
+#endif
@elupus
Team Kodi member
elupus added a line comment May 1, 2012

Just put the Dll include in the ffmpeginternals. It's only for codec id's right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus elupus commented on the diff May 1, 2012
xbmc/cores/dvdplayer/DVDCodecs/DVDCodecs.h
@@ -51,6 +50,7 @@
#else
#include "libavcodec/avcodec.h"
#endif
+#endif
@elupus
Team Kodi member
elupus added a line comment May 1, 2012

This file should include what is needed for the codec id list. Then we should include this file in any place that need the codec id enum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus elupus commented on the diff May 1, 2012
xbmc/cores/dvdplayer/DVDPlayerTeletext.cpp
@@ -116,10 +117,12 @@ signed int CDVDTeletextTools::deh24(unsigned char *p)
bool CDVDTeletextData::CheckStream(CDVDStreamInfo &hints)
{
+#if defined(HAS_FFMPEG)
#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(52,38,1)
if (hints.codec == CODEC_ID_DVB_TELETEXT)
return true;
#endif
@elupus
Team Kodi member
elupus added a line comment May 1, 2012

This seem like a remnant that should be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus elupus commented on the diff May 1, 2012
xbmc/cores/dvdplayer/DVDPlayerVideo.cpp
@@ -636,7 +644,7 @@ void CDVDPlayerVideo::Process()
if (mPostProcess.Process(&picture))
mPostProcess.GetPicture(&picture);
}
-
+#endif
@elupus
Team Kodi member
elupus added a line comment May 1, 2012

This stuff is ugly, create a dummy postprocess class instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus
Team Kodi member

I'm not against this at all. But.. do we really need to drop swscale? That seem utterly silly to replace with anything else.

@theuni
Team Kodi member

Swscale is guilty by association. It's part of the ffmpeg source, so tainted in that respect.

Also consider that if ffmpeg isn't needed, there's likely some way to scale in hardware. That's been the case everywhere I've played, anyway.

@arfoll

This is great, swscale needs to go too, as @theuni said no need for it when using HW decoders.

@elupus elupus was assigned Sep 4, 2012
@justdan0227

So being new to XBMC development, does the --disable-ffmpeg actually remove all FFmpeg from the code such that we do not have to worry about legality of this library being distributed with XBMC and allow hardware to do its job?

@MartijnKaijser
Team Kodi member

@justdan0227
Please ask these question on the forum as we use github for development and not support
http://forum.xbmc.org/showthread.php?tid=162157

@justdan0227

Sorry I meant this to be a development question. With the discussion above, I'm wanting to know if the flag disables the ffmpeg dependency of if I need to add more code.

@davilla

A simple look through the PR code would answer that question and in case you have not noticed, this PR will no longer apply to mainline.

@justdan0227

Thanks, sorry I'm new to GitHub although I'm a programmer of some 30 years. What is PR code? Again my apologies for being new here.

@davilla

PR == Pull Request -> theuni@d3799e6

@FernetMenta
Team Kodi member

obsolete, closing.

@FernetMenta FernetMenta closed this May 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment