GNOME Bugzilla – Bug 735436
Puzzle: Porting GstPuzzle to 1.0
Last modified: 2018-05-04 09:40:27 UTC
Created attachment 284480 [details] [review] Porting GstPuzzle element to 1.0 GstPuzzle was not ported to the latest version. Hence porting the same. The same can be tested with the following command gst-launch-1.0 videotestsrc pattern=snow ! videoconvert ! puzzle ! 'video/x-raw,width=320,height=240,format=(string)I420,framerate=30/1' ! videoconvert ! ximagesink This is my first time porting an element, hence expecting a few review comments. Please review the code. PS: Right now the actual puzzle game is not implemented. This patch takes care of just porting the element. Will submit one more patch by changing the existing logic.
Don't have time at this moment to review the code, but this is really cool! Looking forward to playing with it soon.
:).. please review the code whenever you get time.. will implement the actual game as soon as first version is submitted
Try it with the following pipeline: gst-launch-1.0 uridecodebin uri="file:///home/user/samplevideo" ! videoconvert ! puzzle columns="3" rows="3" ! 'video/x-raw,width=320,height=240,format=(string)I420,fram erate=30/1' ! videoconvert ! ximagesink Using a concrete video you will notice the image isn't being shuffled in 9 blocks, but each channel. So it looks almost random instead of the classic puzzle like the image. http://www.iossdktutorials.com/wp-content/uploads/2012/05/iconBig.png
Also, try avoiding having changes like the one in lines 212-218 of gst/games/gstpuzzle.c. You are just removing an empty line, which is an unnecessary style change. https://bugzilla.gnome.org/attachment.cgi?id=284480&action=diff#a/gst/games/gstpuzzle.c_sec10
Hi Luis, Thanks for the review. As i have already mentioned in the first comment, the actual logic is not correct. This patch takes care of just porting the existing element. Since i was not sure when this patch will get accepted, i did not work on the logic part yet. But now i will check the logic part as to why it is not working and update the same. Can you please review the porting part in the meantime. Regards, Vineeth
Hi Vineeth, The patch looks good then, but we should wait until the logic is fixed before merging. This way we avoid having a element that doesn't work well making it to the next release in case the logic isn't fixed by then. I know you are busy, so I will work on fixing the element as soon as I can. Sounds like fun :)
This element has been removed from the repo and hasn't been fixed here. If you care, please submit a patch to re-add a fixed version.