[Mesa-dev] Refactored st/omx/tizonia commits
These are the refactored commits related to the GSoC project involving adding a st/omx state tracker using tizonia. There are still some parts of code that i didn't refactor yet as explained below: 1) I wasn't sure if it's okay to use #if-#else declaratives for function declarations. For eg: One function accepts omx_base_PortType and the other one vid_dec_PrivateType 2) Because of the argument type differences there is excessive amounts of #if-#else pairs will be needed So I decided to wait for review before making those changes. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Refactored st/omx/tizonia commits
Quoting Eric Engestrom (2017-11-29 07:19:02) > On Wednesday, 2017-11-29 09:32:09 +0530, Gurkirpal Singh wrote: > > These are the refactored commits related to the GSoC project involving > > adding a st/omx state tracker using tizonia. > > There are still some parts of code that i didn't refactor yet as > > explained below: > > 1) I wasn't sure if it's okay to use #if-#else declaratives for function > > declarations. For eg: One function accepts omx_base_PortType and the other > > one vid_dec_PrivateType > > 2) Because of the argument type differences there is excessive amounts of > > #if-#else pairs will be needed > > So I decided to wait for review before making those changes. > > I notice you left the meson build system out; could you give it a stab? > Feel free to ask me or Dylan for help if you get stuck :) Do note that the meson omx code hasn't landed yet (hopefully that will happen in the next day or two though). One thing I'm not sure about there is how to handle the command line option to enable the build. Currently it's `gallium-omx`, and accepts, 'true', 'false', and 'auto'. It might make sense to make it more like the glx option, and accept 'auto', 'bellagio', 'disabled', and when this lands 'tizonia'. Dylan signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Refactored st/omx/tizonia commits
Hi Gurkirpal, I am glad to see you continue working on this out of the GSoC project. I have reviewed it already during that period so just giving my official "the series is:" Reviewed-by: Julien Isorce On 30 November 2017 at 00:23, Dylan Baker wrote: > Quoting Eric Engestrom (2017-11-29 07:19:02) > > On Wednesday, 2017-11-29 09:32:09 +0530, Gurkirpal Singh wrote: > > > These are the refactored commits related to the GSoC project involving > > > adding a st/omx state tracker using tizonia. > > > There are still some parts of code that i didn't refactor yet as > > > explained below: > > > 1) I wasn't sure if it's okay to use #if-#else declaratives for > function > > > declarations. For eg: One function accepts omx_base_PortType and the > other > > > one vid_dec_PrivateType > > > 2) Because of the argument type differences there is excessive amounts > of > > > #if-#else pairs will be needed > > > So I decided to wait for review before making those changes. > > > > I notice you left the meson build system out; could you give it a stab? > > Feel free to ask me or Dylan for help if you get stuck :) > > Do note that the meson omx code hasn't landed yet (hopefully that will > happen > in the next day or two though). > > One thing I'm not sure about there is how to handle the command line > option to > enable the build. Currently it's `gallium-omx`, and accepts, 'true', > 'false', > and 'auto'. It might make sense to make it more like the glx option, and > accept > 'auto', 'bellagio', 'disabled', and when this lands 'tizonia'. > > Dylan > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Refactored st/omx/tizonia commits
On Wednesday, 2017-11-29 16:23:26 -0800, Dylan Baker wrote: > Quoting Eric Engestrom (2017-11-29 07:19:02) > > On Wednesday, 2017-11-29 09:32:09 +0530, Gurkirpal Singh wrote: > > > These are the refactored commits related to the GSoC project involving > > > adding a st/omx state tracker using tizonia. > > > There are still some parts of code that i didn't refactor yet as > > > explained below: > > > 1) I wasn't sure if it's okay to use #if-#else declaratives for function > > > declarations. For eg: One function accepts omx_base_PortType and the other > > > one vid_dec_PrivateType > > > 2) Because of the argument type differences there is excessive amounts of > > > #if-#else pairs will be needed > > > So I decided to wait for review before making those changes. > > > > I notice you left the meson build system out; could you give it a stab? > > Feel free to ask me or Dylan for help if you get stuck :) > > Do note that the meson omx code hasn't landed yet (hopefully that will happen > in the next day or two though). > > One thing I'm not sure about there is how to handle the command line option to > enable the build. Currently it's `gallium-omx`, and accepts, 'true', 'false', > and 'auto'. It might make sense to make it more like the glx option, and > accept > 'auto', 'bellagio', 'disabled', and when this lands 'tizonia'. Agreed, sounds to me like the best approach :) > > Dylan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Refactored st/omx/tizonia commits
Am 29.11.2017 um 05:02 schrieb Gurkirpal Singh: These are the refactored commits related to the GSoC project involving adding a st/omx state tracker using tizonia. There are still some parts of code that i didn't refactor yet as explained below: 1) I wasn't sure if it's okay to use #if-#else declaratives for function declarations. For eg: One function accepts omx_base_PortType and the other one vid_dec_PrivateType 2) Because of the argument type differences there is excessive amounts of #if-#else pairs will be needed So I decided to wait for review before making those changes. Looks really good to me and I think as well that we should avoid excessive #if-#else pairs even if that means we have a bit of code duplication. One question I have is why do you move the vl screen helpers into the auxiliary code in the first patch and not just keep it as common code under the st/omx directory? I mean could make sense to use that somewhere else, but we currently don't do this and your solution for the rest of the code looks, e.g. the H264 decoder, looks really nice to me. And the EGL image stuff really works? Well that is extremely cool. Regards, Christian. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Refactored st/omx/tizonia commits
On Wednesday, 2017-11-29 09:32:09 +0530, Gurkirpal Singh wrote: > These are the refactored commits related to the GSoC project involving > adding a st/omx state tracker using tizonia. > There are still some parts of code that i didn't refactor yet as > explained below: > 1) I wasn't sure if it's okay to use #if-#else declaratives for function > declarations. For eg: One function accepts omx_base_PortType and the other > one vid_dec_PrivateType > 2) Because of the argument type differences there is excessive amounts of > #if-#else pairs will be needed > So I decided to wait for review before making those changes. I notice you left the meson build system out; could you give it a stab? Feel free to ask me or Dylan for help if you get stuck :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Refactored st/omx/tizonia commits
If you are referring to earlier discussion about having tizonia build problems then it was about disabling some extra CPU intensive features like the tizonia player which isn't needed here. This was only required if you wanted to build from source instead of downloading it. Later Juan added an option to do the same https://github.com/tizonia/tizonia-openmax-il/commit/9ab5ecea12ee4dfe6ee058274c8e1a96d4d051e6 Other than that any changes that were needed in the tizonia omxil code were pushed to the main branch before the project officially ended in august. On Wed, Nov 29, 2017 at 10:59 PM, Leo Liu wrote: > > > On 11/29/2017 12:23 PM, Christian König wrote: > > Am 29.11.2017 um 18:08 schrieb Gurkirpal Singh: > > > > On Wed, Nov 29, 2017 at 3:20 PM, Christian König < > ckoenig.leichtzumer...@gmail.com> wrote: > >> Am 29.11.2017 um 05:02 schrieb Gurkirpal Singh: >> >>> These are the refactored commits related to the GSoC project involving >>> adding a st/omx state tracker using tizonia. >>> There are still some parts of code that i didn't refactor yet as >>> explained below: >>> 1) I wasn't sure if it's okay to use #if-#else declaratives for function >>> declarations. For eg: One function accepts omx_base_PortType and the >>> other >>> one vid_dec_PrivateType >>> 2) Because of the argument type differences there is excessive amounts of >>> #if-#else pairs will be needed >>> So I decided to wait for review before making those changes. >>> >> >> Looks really good to me and I think as well that we should avoid >> excessive #if-#else pairs even if that means we have a bit of code >> duplication. >> >> One question I have is why do you move the vl screen helpers into the >> auxiliary code in the first patch and not just keep it as common code under >> the st/omx directory? > > >> I mean could make sense to use that somewhere else, but we currently >> don't do this and your solution for the rest of the code looks, e.g. the >> H264 decoder, looks really nice to me. > > Before refactoring process both the state trackers were in independet > directories. During earlier refactoring effort we decided to keep that > directory structure so it made sense to move > them to auxiliary code. After that I moved them both under st/omx. Since > there could be a chance of it being useful out of st/omx, I left the > decision to keep it or move it back to st/omx > to the mailing list. > > > Fine with me. > > Leo any more comments on this? Otherwise I'm going to give it a few more > days on the list and push it if nobody objects. > > I just had a quick look, it's pretty good to me as well. > > @Gurkirpal, do we still need some changes from Tizonia in order to get it > built/run? > > Leo > > > > Regards, > Christian. > > >> And the EGL image stuff really works? Well that is extremely cool. >> > I double checked the EGL feature with "top" and it shows significantly > less CPU usage compared to when not using this feature. > Thanks to Julien for helping out a lot with this one when he was mentoring > me. > >> >> Regards, >> Christian. >> >> ___ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >>> >> >> > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
