Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ffmpeg] Add support for libdcadec #7102

Merged
merged 2 commits into from Sep 1, 2015
Merged

Conversation

popcornmix
Copy link
Member

This enables libdcadec support in ffmpeg which means you get the 8 channel, lossless decode of DTS-HD tracks.

Just pushing this out in case anyone would find it useful to cherry-pick for now, but we might want to include it in Isenguard+1

@fritsch
Copy link
Member

fritsch commented May 10, 2015

@UniversaI says who?

@fritsch
Copy link
Member

fritsch commented May 10, 2015

Besides that - I think that all platforms should have the opportunity to use dcadec when we compile it. It's also worth it on linux, windows and so on.

Let me PR you something.

@MartijnKaijser
Copy link
Member

@fritsch it's DTS-HD as said by the DTS company

@fritsch
Copy link
Member

fritsch commented May 10, 2015

@MartijnKaijser I know - but there are other things to be discussed here first. I will open the BikeShedding Process with a green lamp, when ready.

@fritsch
Copy link
Member

fritsch commented May 10, 2015

via: popcornmix#11

@un1versal
Copy link
Contributor

@fritsch to add what @MartijnKaijser already said.

You have currently this sentence

+msgid "This option supports 8 channel DTS HD decoding, but may use more CPU. It is only available when DTS and DTS-HD audio passthrough is disabled"

You use both DTS HD* and _DTS-HD_ on same sentence.
Also All existing entries refer only to _DTS-HD_ only https://github.com/xbmc/xbmc/search?utf8=%E2%9C%93&q=DTS-hd

So why are we causing more inconsistencies? Why cant it all be consistent and uniform when referring to same thing?

FYI its not bikeshedding, it genuine review.


#: system/settings/rbp.xml system/settings/imx6.xml
msgctxt "#38120"
msgid "Support 8 channel DTS HD audio decoding"

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@wsnipex
Copy link
Member

wsnipex commented May 10, 2015

Please don't add ffmpeg patches in depends, they should go to our ffmpeg repo instead(or come in automatically with the next rebase)


#: system/settings/rbp.xml system/settings/imx6.xml
msgctxt "#38121"
msgid "This option supports 8 channel DTS HD decoding, but may use more CPU. It is only available when DTS and DTS-HD audio passthrough is disabled"

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@jjd-uk
Copy link
Member

jjd-uk commented May 10, 2015

I'm curious why a new setting is being introduced? as we don't have a similar setting for TrueHD, we simply automatically decode to PCM if TrueHD is not enabled. I'm asking as in my view the methods for TrueHD and DTS-HD should be consistent from a UI point of view.

@fritsch
Copy link
Member

fritsch commented May 10, 2015

Reason: IMX (2 core), Raspberry Pi A, B+ might be too slow to get that decoded.

On all other platforms it is default enabled.

@popcornmix popcornmix force-pushed the dcadec branch 2 times, most recently from ba83587 to bdf128d Compare May 13, 2015 13:06
@popcornmix
Copy link
Member Author

Squashed existing commits.

@wsnipex Looks like there are no tags on https://github.com/foo86/dcadec
and it's pretty active. Milhouse builds have been rebuilding with latest package every day for about six weeks with no reports of regressions so it seems quite stable. If you can upload a version (e.g. from now) as a point in the sand, I can make the PR use that, so we don't forget.

Agreed the first commit should be dropped and added to our ffmpeg repo. Depending on when this gets merged one of our ffmpeg bumps might pick it up (the commits are from master of ffmpeg).

@fritsch
Copy link
Member

fritsch commented Jul 26, 2015

@popcornmix we need something additional here? What is needed to get that one in?

@popcornmix
Copy link
Member Author

I think it's virtually there. The ffmpeg patches have been dropped (they are in current master version).
We are currently using this as the source:
https://github.com/foo86/dcadec/archive/master.tar.gz

I suspect we need to pick a version and host it ourselves.
There are no releases/tags for libdcadec, but as it hasn't changed for six weeks and as the current version seems fine (e.g. in Milhouse build) I'd suggest we just grab the current master version.

If someone can upload the tarball to a suitable place I can update the URL in the makefile.

@wsnipex
Copy link
Member

wsnipex commented Jul 26, 2015

what happend to the changes from popcornmix#14

@popcornmix
Copy link
Member Author

@wsnipex they were there... Let me have a look.

@popcornmix
Copy link
Member Author

Okay, I've added back the @wsnipex commits.
Might be worth bumping the version - there are a couple of dozen commits since 396e75652.

@fritsch
Copy link
Member

fritsch commented Jul 26, 2015

You mean bumping ffmpeg? and then we are done?

@popcornmix
Copy link
Member Author

No, ffmpeg is okay. The tarball from https://github.com/foo86/dcadec is a little old.

@@ -56,6 +56,7 @@ endif
ifeq ($(Configuration), Release)
ffmpg_config += --disable-debug
endif
ffmpg_config += --enable-libdcadec

This comment was marked as spam.

@fritsch
Copy link
Member

fritsch commented Jul 26, 2015

@popcornmix thx. @wsnipex could you bump the tar? then we could continue this here to get it merged?

@wsnipex
Copy link
Member

wsnipex commented Jul 26, 2015

current master tarball uploaded to mirrors as libdcadec-git-2a9186e3.tar.gz
might take a bit to propagate.

@popcornmix
Copy link
Member Author

squashed in @stefansaraev's latest patch. jenkins build this please

@Paxxi
Copy link
Member

Paxxi commented Aug 20, 2015

Spoke with @stefansaraev in IRC and agreed to do the win32 work, tonight if time permits or over the weekend

@Paxxi
Copy link
Member

Paxxi commented Aug 20, 2015

@popcornmix pick Paxxi/xbmc@4fad9d8 for win32 work.
someone needs to upload http://delusional.nu/libdcadec-git-396e75652-win32.zip to build-deps/win32/mingw-msys/ and it should be good to go. Not runtime tested as I don't have anything to test with but ffmpeg built fine

@popcornmix
Copy link
Member Author

@Paxxi added your commit to PR.

@MartijnKaijser
Copy link
Member

file uploaded. will take some time to be available

@stefansaraev
Copy link
Contributor

jenkins build this please

@stefansaraev
Copy link
Contributor

mind rebasing ?

@MartijnKaijser MartijnKaijser added this to the Jarvis 16.0-alpha3 milestone Sep 1, 2015
@MartijnKaijser
Copy link
Member

jenkins build this please

@popcornmix
Copy link
Member Author

I updated the string numbers and added a comment about where the settings strings were from during the rebase, so would be useful if someone just sanity checks that looks okay.

@stefansaraev
Copy link
Contributor

strings look good to me @popcornmix thanks.

@popcornmix
Copy link
Member Author

Android-x86 and win32 failed, but doesn't look related to this PR.

@stefansaraev
Copy link
Contributor

jenkins error unrelated. triggered manual builds (just in case) - all green. @MartijnKaijser your button

MartijnKaijser added a commit that referenced this pull request Sep 1, 2015
[ffmpeg] Add support for libdcadec
@MartijnKaijser MartijnKaijser merged commit 7500b5d into xbmc:master Sep 1, 2015
@metaron-uk
Copy link
Contributor

I've a problem:

andrew@blackbox ~/saturn/ksoo-kodi/kodi $ git checkout master && git pull
andrew@blackbox ~/saturn/ksoo-kodi/kodi $ git clean -xfd
andrew@blackbox ~/saturn/ksoo-kodi/kodi $ ./bootstrap && ./configure
...
checking ogg/ogg.h presence... yes
checking for ogg/ogg.h... yes
checking vorbis/vorbisfile.h usability... yes
checking vorbis/vorbisfile.h presence... yes
checking for vorbis/vorbisfile.h... yes
checking for LIBDCADEC... no
configure: error: libdcadec not found
andrew@blackbox ~/saturn/ksoo-kodi/kodi $ make -C tools/depends/target/libdcadec PREFIX=/usr/local
make: Entering directory '/mnt/saturn/home/andrew/ksoo-kodi/kodi/tools/depends/target/libdcadec'
Makefile:1: ../../Makefile.include: No such file or directory
make: *** No rule to make target '../../Makefile.include'.  Stop.
make: Leaving directory '/mnt/saturn/home/andrew/ksoo-kodi/kodi/tools/depends/target/libdcadec'

any clues?

@MilhouseVH
Copy link
Contributor

any clues?

On Ubuntu I added the nightly PPA:

https://launchpad.net/~team-xbmc/+archive/ubuntu/xbmc-nightly

then apt-get install dcadec-dev.

@metaron-uk
Copy link
Contributor

My solution:

cd tools/depends
su
./configure --with-toolchain=/usr --prefix=/usr/local
cd ../..
make -C tools/depends/target/libdcadec PREFIX=/usr/local

(Gentoo's media-sound/dcadec package isn't ready for primetime yet: https://bugs.gentoo.org/buglist.cgi?quicksearch=dcadec )

#. Description of setting #38118 Settings -> System -> Audio output -> Support 8 channel DTS-HD audio decoding
#: system/settings/settings.xml
msgctxt "#38119"
msgid "Enables decoding of high quality DTS-HD audio streams. Note: This increases CPU load and is only available when DTS and DTS-HD audio passthrough are disabled."

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@metaron-uk
Copy link
Contributor

Unfortunately I still can't get configure to find libdecadec (see http://forum.kodi.tv/showthread.php?tid=229552&pid=2098454#pid2098454)

@fritsch
Copy link
Member

fritsch commented Sep 5, 2015

Sorry - this is not place to answer gentoo build system issues. The Ubuntu way, that we support is supported via our ppa.

@@ -1139,6 +1140,7 @@ PKG_CHECK_MODULES([GNUTLS], [gnutls], [have_gnutls=yes];AC_DEFINE([HAVE_GNUTLS],
AC_CHECK_LIB([bz2], [main],, AC_MSG_ERROR($missing_library))
AC_CHECK_LIB([jpeg], [main],, AC_MSG_ERROR($missing_library)) # check for cximage
AC_CHECK_LIB([tiff], [main],, AC_MSG_ERROR($missing_library))
AC_CHECK_LIB([dcadec], [main],, AC_MSG_ERROR($missing_library))

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet