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

[rfc] ppaplayer: handle .opus extension #4994

Merged
merged 1 commit into from Aug 3, 2014
Merged

Conversation

stefansaraev
Copy link
Contributor

this was requested in irc.

so far I have build-tested for linux (ubuntu and openelec) only. the linux part is quite trivial, but since I am not much familiar with xbmc's own buildsystem - this needs some love from osx/android/rbpi platform devs (opus in tools/depends/target), and (maybe) changes to jenkins

/cc @wsnipex @FernetMenta to check for stupids.

@wsnipex
Copy link
Member

wsnipex commented Jul 9, 2014

Thanks, will take a look at adding it to unified deps.
Win32 needs special treatment.

@wsnipex
Copy link
Member

wsnipex commented Jul 9, 2014

see wsnipex@308222b
seems to compile on linux and android. OSX claims opus is not found when configuring ffmpeg, although opus builds and installs there as well....

There is another rather annoying issue: ARM cpu optimizations need perl, which we don't have in our build env. I'm not going to mess with adding perl just for opus. Maybe someone can do a configure run and figure out some defines to stick into config.h

@stefansaraev
Copy link
Contributor Author

thanks @wsnipex
I missed the perl part. compiling for x86_64 here. does --disable-asm help on arm ?

EDIT: http://sprunge.us/VJOU

@wsnipex
Copy link
Member

wsnipex commented Jul 10, 2014

it will probably be dead slow without optims on low power devices like rpi and atv2

@t-nelson
Copy link
Contributor

We're distributing the sources anyway. Run the perl before packaging and
patch it away from the build system.

@stefansaraev
Copy link
Contributor Author

ok. perl is required for celt/arm/arm2gnu.pl to convert arm asm to gnu as format. I believe using host perl should be OK (just like java for codegenerators)

however, according to configure.ac, for using asm optimizations it is required to build WITHOUT floating point support (./configure --enable-fixed-point), this will need special handling for arm, anyway.

@t-nelson unfortunately we can't ship celt_pitch_xcorr_arm-gnu.S / armopts-gnu.S as it's up to configure to decide whether to set OPUS_ARM_MAY_HAVE_EDSP/MEDIA/NEON. hint ./celt/arm/armopts.s.in

so here are our options:

  • dont bother with asm, force --disable-asm (it does not have any effect on x86 anyway)
  • use host perl
  • rewrite arm2gnu.pl in c and build a native tool. I dont have skills to do that..
  • drop this PR and wait for better times if none of the above is acceptable :)

@wsnipex
Copy link
Member

wsnipex commented Jul 10, 2014

host perl should do. I'm already working on it.

@wsnipex
Copy link
Member

wsnipex commented Jul 10, 2014

@wsoltys or @Montellese could you take a look at the win32 part? there is already a VS project included in opus.
@Memphiz darwin ping

jenkins build this please

@wsnipex
Copy link
Member

wsnipex commented Jul 10, 2014

meh, jenkins issues again..
jenkins build this please

@Memphiz
Copy link
Member

Memphiz commented Jul 10, 2014

On vacation until end of the week - but i saw Opus here in the town - it was a store for crappy Clothes :D

@wsoltys
Copy link

wsoltys commented Jul 11, 2014

Can't help atm as I'm deep buried in other projects currently.

@akva2
Copy link
Contributor

akva2 commented Jul 15, 2014

note that the upcoming ffmpeg 2.3 has a native opus decoder. waiting for/backporting that might be a good idea.

@Memphiz
Copy link
Member

Memphiz commented Jul 15, 2014

Ha - if we could agree on that one i could skip looking into libopus for now?

@stefansaraev
Copy link
Contributor Author

thanks @akva2 I was not aware.
yes @Memphiz

@fritsch
Copy link
Member

fritsch commented Jul 15, 2014

We need to decide if we want to bump ffmpeg 2.2.x to 2.3 for helix release. I did not look into API breaks yet. Last time vdpau needed a minor rework.

If we decide to bump, I can add 2.3-preX release to our ffmpeg fork done from master with our patches rebased on top.

@FernetMenta
Copy link
Contributor

ffmpeg 2.3 has not been forked yet and we plan August merge window to be the last for Helix.

@akva2
Copy link
Contributor

akva2 commented Jul 15, 2014

seems to be around the corner though, ref http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2014-July/159611.html

@Memphiz
Copy link
Member

Memphiz commented Jul 15, 2014

already on it - this pr already works for osx - need to check for ios and do a pr against the op ...

@stefansaraev
Copy link
Contributor Author

rebased after 7318eaa

@Memphiz
Copy link
Member

Memphiz commented Jul 15, 2014

args why? grrr please hold on until i come up with a pr for osx and ios - so i don't need to rebase my changes on top of yours again and again...

@stefansaraev
Copy link
Contributor Author

oh. because it does not compile on current master (anymore) ;(

@Memphiz
Copy link
Member

Memphiz commented Jul 15, 2014

@stefansaraev cherry-pick top 4 commits of https://github.com/Memphiz/xbmc/tree/opus - can't do a pull request to your repo (maybe you are private on github or something).

@popcornmix @koying - might wanna check opus compilation out - it might be a bitch with this handcoded asm in there ...

tested with an opus sample file on osx and ios - needs testing on atv2 (maybe tomorrow when i am back in town).

needs #5036 to go in first ... (won't compile for osx/ios without it).

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Jul 16, 2014
@wsnipex
Copy link
Member

wsnipex commented Jul 16, 2014

jenkins build this please

@fritsch
Copy link
Member

fritsch commented Jul 16, 2014

Concerning ffmpeg 2.3, here we go: https://github.com/FFmpeg/FFmpeg/commits/release/2.3

@wsnipex
Copy link
Member

wsnipex commented Jul 16, 2014

nice, so lets bump ffmpeg instead?

@popcornmix
Copy link
Member

I'd vote for bumping ffmpeg.
ffmpeg 2.3 includes a number of armv6 optimisations that are currently being added as Pi specific patches in OpenELEC/raspbmc. Would be nice to drop the patches.

@FernetMenta
Copy link
Contributor

now with correct account: yes, lets bump it.

@fritsch
Copy link
Member

fritsch commented Jul 16, 2014

There were some code changes, our custom patches don't apply cleanly.
Currently don't have the time to look in detail - so someone with ffmpeg
inside knowledge should create release/2.3-xbmc branch and pick / merge /
drop our custom patches.

I will be offline most of the time until July 28th (business trip)

2014-07-16 17:51 GMT+02:00 Rainer Hochecker notifications@github.com:

now with correct account: yes, lets bump it.

Reply to this email directly or view it on GitHub
#4994 (comment).

               Key-ID:     0x1A995A9B
               keyserver: pgp.mit.edu

Fingerprint: 4606 DA19 EC2E 9A0B 0157 C81B DA07 CF63 1A99 5A9B

@Memphiz
Copy link
Member

Memphiz commented Jul 16, 2014

Even if i am a bit late - and somehow the timing for this decision was a bit murphy (cause i could have saved some time instead of "playing" with fscking libopus) - just fyi - this PR also works on the atv2 :p

@stefansaraev
Copy link
Contributor Author

apologies for wasting your time @Memphiz (and @wsnipex) :)
unfortunately I dont follow ffmpeg development closely. if I knew, I'd wait a bit and PR only fc50ab8 which should be enough after ffmpeg 2.3

@FernetMenta
Copy link
Contributor

our ffmpeg 2.3 sha for testing until I create a tag "d028c907004e8a3c0f5161ce595331e4cc57c86c"

to be used in https://github.com/xbmc/xbmc/blob/master/tools/depends/target/ffmpeg/FFMPEG-VERSION

@Memphiz
Copy link
Member

Memphiz commented Jul 16, 2014

that ffmpeg hash + fc50ab8 works on osx and ios (needs gas-preprocessor bumped for ios else ffmpeg won't compile ...)

@stefansaraev stefansaraev changed the title [rfc] enable opus codec in ffmpeg [rfc] ppaplayer: handle .opus extension Aug 1, 2014
@fritsch
Copy link
Member

fritsch commented Aug 3, 2014

jenkins build this please

fritsch added a commit that referenced this pull request Aug 3, 2014
ppaplayer: handle .opus extension
@fritsch fritsch merged commit 7909911 into xbmc:master Aug 3, 2014
@MartijnKaijser MartijnKaijser modified the milestones: Pending for inclusion, Helix 14.0-alpha3 Aug 3, 2014
@stefansaraev stefansaraev deleted the opus branch August 4, 2014 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants