GNOME Bugzilla – Bug 782800
gst-omx: add initial support for Tizonia
Last modified: 2017-12-11 17:04:08 UTC
Hi, this adds initial support to use tizonia as OMX IL target. The commits are on my github tizonia branch https://github.com/gpalsingh/gst-omx/commits/tizonia Validated with pipeline: gst-launch-1.0 filesrc location=~/Downloads/file.mp3 ! id3demux ! mpegaudioparse ! omxmp3dec ! audioconvert ! audioresample ! pulsesink I'm a GSoC 17 student and needed to port gst-omx to test the new component i'll be working on https://summerofcode.withgoogle.com/projects/#4737166321123328
Thanks for working on this! Please attach patches in "git format-patch" here for review
Created attachment 352191 [details] [review] configure target
Created attachment 352192 [details] [review] add target
Created attachment 352193 [details] [review] guards enums
Created attachment 352194 [details] [review] ignore OMX_ErrorPortUnpopulated
Created attachment 352195 [details] [review] add config for omxmp3dec
Review of attachment 352191 [details] [review]: This one can go.
Review of attachment 352192 [details] [review]: That's good. Could you add a header check to detect if the Bellagio HEADERS are installed ? Also, are the internal headers out of sync with latest OMX ?
Review of attachment 352193 [details] [review]: Fair.
Review of attachment 352194 [details] [review]: Good.
Review of attachment 352195 [details] [review]: ::: config/tizonia/gstomx.conf @@ +1,3 @@ +[omxmp3dec] +type-name=GstOMXMP3Dec +core-name=/usr/lib/x86_64-linux-gnu/libtizcore.so We should make the file a template, and use the path reported by pkg-config here. Hardcoding debian style path isn't nice here.
Comment on attachment 352191 [details] [review] configure target commit c69232852120d064c689caef07b3c68ad8fe6288 Author: Julien Isorce <jisorce@oblong.com> Date: Thu May 18 13:50:56 2017 +0100 configure.ac: move omx features check after target selection Does not change anything, except this will be useful for future commits. Indeed some targets provide a .pc file where to look for the omx headers. https://bugzilla.gnome.org/show_bug.cgi?id=782800
Don't forget to update meson.build file too please.
Created attachment 354859 [details] [review] add config for omxmp3dec Changed to not use hard-coded path
Review of attachment 354859 [details] [review]: One minor issue, and then we are good, thanks. ::: .gitignore @@ +9,3 @@ config.rpath configure +gstomx.conf Use: /config/tizonia/gstomx.conf Otherwise it will hide changes in the other gstomx.conf, which are not template.
Created attachment 354860 [details] [review] add config for omxmp3dec Changed to only ignore config/tizonia/gstomx.conf
Review of attachment 354860 [details] [review]: ::: .gitignore @@ +9,3 @@ config.rpath configure +config/tizonia/gstomx.conf The initial / was snot a typo. In gitignore, you should add the / for any single occurrence, otherwise you could endup with unwanted side effect. Notice that this .gitignore isn't great, since configure, libtool and pretty much everything surrounding your change is a bit too loose.
(In reply to Nicolas Dufresne (stormer) from comment #17) > Review of attachment 354860 [details] [review] [review]: > > ::: .gitignore > @@ +9,3 @@ > config.rpath > configure > +config/tizonia/gstomx.conf > > The initial / was snot a typo. In gitignore, you should add the / for any > single occurrence, otherwise you could endup with unwanted side effect. > Notice that this .gitignore isn't great, since configure, libtool and pretty > much everything surrounding your change is a bit too loose. Ah I'm on it. Sry about the back and forth.
Created attachment 354861 [details] [review] add config for omxmp3dec
Created attachment 354957 [details] [review] Add tizonia option to meson
I've had to resolve some merge conflict with a Xilinx target, please double check if nothing bad happened please.
remote: sending e-mail for ae14bb362a26bb1b48f7f99c7fc2350e0fa50ffa remote: sending e-mail for fc1a8229b0595456e3294df11b89560697d76ee5 remote: sending e-mail for 065878a5d4a21db4cfb46cbf5381d41017edd0d1 remote: sending e-mail for eed49b4231a063639f90279c8044404c2149902a remote: sending e-mail for 043b4177172f69807d02b6bceff41d700157c4e7
commit ff74c66a9a5e0b78a22e8de470180f32ae01aa08 Author: Julien Isorce <jisorce@oblong.com> Date: Mon Dec 11 15:55:44 2017 +0000 meson: move omx features check after target selection And uses gst_omx_args instead of add_global_arguments. Similar to c69232852120d064c689caef07b3c68ad8fe6288 which was only for configure.ac Useful to get omxvp8dec with meson too: meson . buildtmp -D with_omx_target=tizonia https://bugzilla.gnome.org/show_bug.cgi?id=782800