GNOME Bugzilla – Bug 727825
omxvideodec: Enhance colorformat support
Last modified: 2014-07-23 08:15:08 UTC
Created attachment 273797 [details] [review] omxvideodec: Enhance colorformat support It add support of following color format: ABGR, ARGB, RGB16, BGR16, YUY2, UYVY, YVYU, GRAY8 and NV16. It also add an helper to convert OMX color format to GStreamer color format and use it to factorize some code.
Hi, I do not see anything wrong but it's hard to check everything :) I can at least test it on RPI by the end of the week if no body can do it before. I also wonder if you could split the commit this way: 1 commit that add the helper and 1 commit per place where you use it. Also it seems you did some refactoring, would be nice to put them in separate commits. Because it could help for git bisect later. Your decoder can output all those formats ? On RPI it can only output I420 or NV12. But there is an other component "resize" that can do colorspace conversion and scaling. About OMX_COLOR_Format32bitARGB8888, on RPI I had to do this http://cgit.collabora.com/git/user/julien/gst-omx.git/commit/?h=resize&id=68ce07b32b285bccca82b19fde31b3646ac225a5 but we do not use this path anyway :) Out of curiosity, do you have support for OMX_UseEGLImage on your platform ?
(In reply to comment #1) > About OMX_COLOR_Format32bitARGB8888, on RPI I had to do this > http://cgit.collabora.com/git/user/julien/gst-omx.git/commit/?h=resize&id=68ce07b32b285bccca82b19fde31b3646ac225a5 > but we do not use this path anyway :) This clearly needs to go into a RPI specific hack and not unconditionally be used. And should be reported to the RPI people.
Comment on attachment 273797 [details] [review] omxvideodec: Enhance colorformat support Looks good but I'll wait until Julien has tested it on RPi before merging this.
Julien, feel free to merge once you tested
(In reply to comment #1) > Hi, I do not see anything wrong but it's hard to check everything :) I can at > least test it on RPI by the end of the week if no body can do it before. > > I also wonder if you could split the commit this way: 1 commit that add the > helper and 1 commit per place where you use it. > Also it seems you did some refactoring, would be nice to put them in separate > commits. > > Because it could help for git bisect later. I can if you prefer, I just did a single patch to ease attachment. > Your decoder can output all those formats ? On RPI it can only output I420 or > NV12. But there is an other component "resize" that can do colorspace > conversion and scaling. The video decoder, no, but it have a post-processor which can do color conversion. > About OMX_COLOR_Format32bitARGB8888, on RPI I had to do this > http://cgit.collabora.com/git/user/julien/gst-omx.git/commit/?h=resize&id=68ce07b32b285bccca82b19fde31b3646ac225a5 > but we do not use this path anyway :) I agree with Sebastian, OMX_COLOR_Format32bitARGB8888 to GST_VIDEO_FORMAT_BGRx should be RPi specific. Also, as I mentionned in code comments, there is a little pitfall about OMX_COLOR_Format32bitARGB8888 and OMX_COLOR_Format32bitBGRA8888 : name don't match description. I prefer to follow description > Out of curiosity, do you have support for OMX_UseEGLImage on your platform ? No, I haven't.
Created attachment 273917 [details] [review] omx: add an helper to convert OMX color format to GStreamer color format
Created attachment 273918 [details] [review] omxbufferpool: make video stride and offset calculation easier
Created attachment 273919 [details] [review] omxvideodec: simplify color format conversion in fill_buffer function
Created attachment 273920 [details] [review] omxvideodec: add support of more color format
I split my first patch in four commits. Tomorrow i will try to make some tests on RPi.
Sorry, my RPi environment was broken, i have to recompile the whole thing. So, I tested HW decoding using RPi with each patch, I try: - omxh264dec ! filesink - omxh264dec ! eglglessink On my side, decoded output seems good.
I'll certainly have a try tomorrow and push if fine. Thx for those patches :)
Created attachment 274333 [details] [review] omx: add an helper to convert OMX color format to GStreamer color format Oops my code causes warnings, just fixed it
Tested on RPI and I do not see anything wrong. So I'm going to push it if no objection comes.
commit 1b6879921cdddf261c47391a47150c13158ac0cb Author: Aurélien Zanelli <aurelien.zanelli@parrot.com> Date: Wed Apr 9 18:52:16 2014 +0200 omxvideodec: add support of more color format Add support for ABGR, ARGB, RGB16, BGR16, YUY2, UYVY, YVYU, GRAY8 and NV16 color format. commit f5f876f6817b4dc98cc88fc5e6ee1e62a78c119b Author: Aurélien Zanelli <aurelien.zanelli@parrot.com> Date: Wed Apr 9 18:51:57 2014 +0200 omxvideodec: simplify color format conversion in fill_buffer function commit 6834d2e0b353a0807257ba2177ebe39ae93a5fa4 Author: Aurélien Zanelli <aurelien.zanelli@parrot.com> Date: Wed Apr 9 18:51:41 2014 +0200 omxbufferpool: make video stride and offset calculation easier It will be easier to support more color format. commit c115da9fd508272cf3ffec5ae66892c52e7682f1 Author: Aurélien Zanelli <aurelien.zanelli@parrot.com> Date: Wed Apr 9 18:51:12 2014 +0200 omx: add an helper to convert OMX color format to GStreamer color format
> So I'm going to push it if no objection comes. If you're going to wait for possible objections maybe wait longer than 14 minutes, or just don't ;)
I also realized I forgot to add the bug id in the commit messages.