GNOME Bugzilla – Bug 737786
android: Upstream one of the android video source implementations
Last modified: 2016-01-21 19:08:16 UTC
As of today, two Android video source for GStreamer are available outside the main tree. I'd like to see one of these two (or a mix of both) merge into -bad. From Ericsson Research OpenWebRTC (1.0): https://github.com/EricssonResearch/openwebrtc-gst-plugins/tree/master/sys/androidvideo From Collabora (not yet ported, 0.10): http://cgit.collabora.com/git/user/tester/gst-plugins-bad.git/tree/sys/androidcamera?h=android-camerasrc-0.10 The first is much simplier, and support 1.0, while the second is a bit more complete, more error handling etc. Both injects a Java class to do callback handling. The second abstract the JNI logic to inject this with the goal to make it easier if we want more element being called back by the Java side.
We also have a test app to go with the 0.10 source we built: http://cgit.collabora.com/git/user/tester/gst-camera-test.git
The build system of the Ericsson one (and how the Java foo is integrated) looks simpler. Also it's probably less bitrotten :) Maybe we should move that one, and then add anything on top of it that the 0.10 one has additionally? And then at a later point looking at abstracting the Java handling into its own library, like the 0.10 one does. The library as is in the 0.10 one is not really useful anymore as the original code from androidmedia (where libdvm comes from) has changed quite a bit since then.
(In reply to Sebastian Dröge (slomo) from comment #2) > Maybe we should move that one, and then add anything on top of it that the > 0.10 one has additionally? And then at a later point looking at abstracting > the Java handling into its own library, like the 0.10 one does. The library > as is in the 0.10 one is not really useful anymore as the original code from > androidmedia (where libdvm comes from) has changed quite a bit since then. Looks like a good approach. What interest me the most from the 0.10 work, is the configurability and the error handling. Re-implementing that on top or the "simpler" 1.0 version seems the most productive (and does not have to happen right away).
We should probably use the gresource compiler instead of xxd, to avoid the extra dependency. It probably makes sense to package the generated files, so people can build the plugin without having the SDK installed.
(In reply to Olivier Crête from comment #4) > We should probably use the gresource compiler instead of xxd, to avoid the > extra dependency. Only problem here is that we then unconditionally get GIO as a dependency, which is huge. But otherwise makes sense to me, just needs someone to do that ;) > It probably makes sense to package the generated files, so > people can build the plugin without having the SDK installed. What do you mean? In cerbero we would ship the static plugin, which would then also include the Java resources and everything.
We also unconditionally link with gio for anything that uses sockets anyway. I meant in cerbero, we'll need to add the sdk (not just the ndk) unless the pre-compiled class/dex/jar file are included, but it's more simple to include the .c/.h files like we do for orc.
Before taking advantage from both implementations, I am making collabora's code work with gst 1.x. https://git.collabora.com/cgit/user/joykim/gst-plugins-bad.git/log/?h=ahc-wip I just reuse jniutils in androidmedia to remove dependency on dvm, and the ported codes are placed in andromedia directory naturally. If andromedia is not a good place for the codes, jniutils should be a shared library like dvm did. Can anybody advise me?
I was also just going to port the Collabora one to 1.x and use the JNI bits from androidmedia (and if necessary extend them). But if you already almost finished that... :) Make sure that each commit on its own is compilable, so you probably have to squash all those per-file commits together. And the current solution for the Java files is to use the plugin-java-file magic from cerbero. Basically you place the .java files in some directory in share/gst-android (check the cerbero/data/ndk-build code for details).
Also try to keep history, the code at http://cgit.collabora.com/git/user/tester/gst-plugins-bad.git/tree/sys/androidcamera?h=android-camerasrc-0.10 has a clean history that we can just take over :) That will make it easier later to understand some decisions that were taken as the commit that added the code is in the history with its commit message.
Are you planning to update your branch?
If you are asking me, yes and I hope. but currently I stucked in crash with my wip branch. (and even I am not sure if it is only one critical issue ;) anyway, I need help to find what I did wrong if my work is worth it. I ported android application also https://git.collabora.com/cgit/user/joykim/gst-camera-test.git/ With my wip branch, a crash on gl stuff happens. but I think it's not gl issue. It might be wrong somewhere my ported code, maybe GstBuffer handling in ahcsrc/on_preview_frame function. Here's back trace. Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 19963] 0x400de810 in __memcpy_base () from /Users/justin/Documents/git/gst-camera-test/obj/local/armeabi/libc.so (gdb) bt
+ Trace 235392
This crash doesn't come from your code it seems, but from the GL thread. What is the test application doing? Just outputting the camera to glimagesink? Does outputting something else (videotestsrc) to glimagesink work for you? Does the camera work if you use fakesink instead of glimagesink? Does the camera work if you use something like ... ! "video/x-raw,format=AYUV" ! fakesink as the sink (to check if videoconvert likes your buffers or also crashes)?
> What is the test application doing? Just outputting the camera to glimagesink? Yes, it's very simple These two cases just worked well. videotestsrc ! video/x-raw,format=(string)NV21 ! glimagesink name=vsink sync=false ahcsrc ! video/x-raw,format=(string)NV21 ! fakesink name=vsink sync=false but, only this causes crash ahcsrc ! video/x-raw,format=(string)NV21 ! glimagesink name=vsink sync=false > (to check if videoconvert likes your buffers or also crashes)? I didn't check with videoconvert yet. I will.
With this pipeline, the point moves to videoconvert ahcsrc ! video/x-raw,format=(string)YV12 ! queue ! videoconvert ! video/x-raw,format=(string)NV21 ! glimagesink name=vsink sync=false Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 5924] _backup_video_orc_unpack_I420 (ex=<optimized out>) at tmp-orc.c:942 942 var37 = ptr4[i]; (gdb) bt
+ Trace 235393
Ok, so whatever buffers/memory you provide from your source is broken then in one way or another. It could be something simple as stride/plane offset mismatches, or also that the memory became freed/invalid before downstream used it (which should be prevented with the various GstMemory mechanisms on your side).
I fixed a crash problem on my branch so this works now. However, handling java file is still vague a bit. Gstreamer.java is managed in cerbero and this file is placed in project directory when ndk-build run. but for this android camera, Callback java class is located in gst-plugins-bad and even it should be built before linking with android application. so I am a bit confused about what solution is more proper for initial upstream android camera. Could you advise me?
You can install your .java file in share/gst-android/ndk-build/$plugin/ and then the GStreamer ndk-build integration will build it into the application. This is from memory though, check the code in gstreamer-1.0.mk. There should be something like GSTREAMER_PLUGINS_CLASSES in there, which is supposed to automatically do this.
Also please preserve the GIT history of the 0.10 camera code, it contains many hints at why things were done like this. And ideally squash some of your own commits together so that we have something compilable with each commit.
The plugin-java-file magic from cerbero works very well and it seems to solve troublesome java class issue. :) I finished to cherry-pick the history of 0.10 works. But as you know, it makes huge conflicts so there's no other option except for using ours strategy. $ git cherry-pick -X ours ... You can see that the entire commits are on the top of master branch in here; https://git.collabora.com/cgit/user/joykim/gst-plugins-bad.git/log/?h=ahc-history-preserved you might want to get the commits from '51d5b4f androidmedia: Add androidmedia plugin' to HEAD in the url above. the HEAD commit which I made is to restore build.
Created attachment 311570 [details] [review] porting from 0.10 to 1.0 This patch is created on the top of ahc-history-preserved branch, 'https://git.collabora.com/cgit/user/joykim/gst-plugins-bad.git/log/?h=ahc-history-preserved' Unfortunately, current cerbero recipe doesn't deploy java file. I'll attach cerbero patch also in different issue.
Just for the record, the Collabora one will be merged some time soon and not the Ericsson one. It has more features, is more well tested and integrates with the existing JNI helpers we have.
That the commits touch the audio/video encoders/decoders and you revert most of the changes later is not good. When preserving the history, you should try to filter changes only to the relevant files (that is, the new files) and keep everything else untouched. Or we could just have the old 0.10 stuff in a branch, that is then merged into master. That was master always stays buildable at least. Nonetheless don't merge any changes to existing files just to revert these changes again later. Or we really just ignore the history, mention it in the commit message (where to find the history, where the stuff comes from) and just do a single commit with the final result. All messy :) Opinions?
(In reply to Sebastian Dröge (slomo) from comment #22) > That the commits touch the audio/video encoders/decoders and you revert most > of the changes later is not good. When preserving the history, you should > try to filter changes only to the relevant files (that is, the new files) > and keep everything else untouched. > I agree, but it's not easy to filter changes because some of changes have dependency with major history a lot. To do like that, imo, more comments should be required between history to explain what or how the changes are modified. > Opinions? How about mixing your 2nd and 3rd opinion together? Having another 0.10 branch for android camera, then in the master branch, we can mention the branch. :)
I think we should have history in a branch that's then merged into master, that's also how I do plugin moves these days. You can filter out things with git filter branch, so that e.g. the branch only has those files you want to keep and nothing else (and commits that touch other files simply have the bits that don't touch files we're interested in removed).
Hi, I want to use this branch and for ease of integration with cerbero, I picked and squashed the entire ahc-history-preserved branch into a single commit, on top of which I also squashed https://git.collabora.com/cgit/user/joykim/gst-plugins-bad.git/commit/?h=ahc&id=9ff3c982c13ef51d0fc5393ec8bd73438f25802c (which seems to be the same patch as the one attached here) Squashing and patching works fine, and the result is quite clean (no references to anything outside androidmedia), but it looks like the examples are broken. First of all, there are 2 of them, very similar to each other; not sure what is the difference. Second, they contain references to eglglessink as well as GST_X_OVERLAY...
I am not sure if I understand how/what I should do for this. @Tim, your plan is to create a branch in for history preserving, then will merge a squashed commit on top of master. do I correctly understand? @George, thank you for testing. > but it looks like the examples are broken. Which example was broken? Do you mean https://git.collabora.com/cgit/user/joykim/gst-camera-test.git/ ?
(In reply to Justin J. Kim from comment #26) > I am not sure if I understand how/what I should do for this. > > @Tim, your plan is to create a branch in for history preserving, then will > merge a squashed commit on top of master. do I correctly understand? As I understand it, the plan is to create a branch starting from somewhere in the 0.10 series that contains all the history of the android plugin, without any touches in other files (using git filter-branch to filter what the commits contain) and then merge this in master (no squashing, just git merge). This avoids rebasing the entire history on top of master, which I think makes sense for this case, because otherwise master would contain non-compilable commits. > @George, thank you for testing. > > but it looks like the examples are broken. > Which example was broken? Do you mean > https://git.collabora.com/cgit/user/joykim/gst-camera-test.git/ ? No, the example from this repository is good, but the *same* example also exists in the ahc-history-preserved branch (together with one more very similar example) and is broken there.
Hello, Happy new year! https://git.collabora.com/cgit/user/joykim/gst-plugins-bad.git/log/?h=filterout-ahc I made a branch again to filter out files which are not interested in preserving history. When I tested, it works well except one thing. glimagesink has longer delays (about 1~2 min after building pipeline). This delay issue is reproduced when I used 'videotestsrc'. Please check again the url. Thank you!
Is the problem with glimagesink maybe fixed by http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=22a0914cce9f6352e3042b9f1f7b9cacac2db0b7 ? Or were you using that version already?
(In reply to Sebastian Dröge (slomo) from comment #29) > Is the problem with glimagesink maybe fixed by > http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/ > ?id=22a0914cce9f6352e3042b9f1f7b9cacac2db0b7 ? Or were you using that > version already? Thank you for information. Although I didn't check only one commit unit which you point out, it works fine after rebasing to yesterday HEAD (http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=1e5eb72532613efb15bb687df68b6fb5048896c0)
I was about to merge this, but found that latency reporting is wrong. It reports min/max base on min/max frame duration. Max latency is how long can the stream be delayed before the information get lost by the source. Also, the largest frame duration makes a lot of more sense for min latency. Try it out, remove sync=0 to the test program and try it in a darker room. You endup with a slideshow.
I was looking the at the git foo the other day, I had something slightly different in mind, but guess this would work too.
Ok, the timestamping code is also completely random (it timestamp buffer with the previous buffer timestamp and duration, and drop the first one). I would complete drop that code, and let the base class pick a timestamp, leave the duration to unknown.
Ok, sorry for the noise, but the timestamp code do one thing right, it timestamp as soon as possible (in the Android callback). Doing timestamp at ->create() time does not look nice here.
Ok, I think it's good enough to merge now. To preserve history and not mess with bisect, we need a non fast-foward merge to happen. Though, this is disabled on our repository. Tim and/or Sebastian will take care as soon as they find the time. Here's the final branch link with a fix for the latency: https://git.collabora.com/cgit/user/nicolas/gst-plugins-bad.git/log/?h=android-hardware-camera And my master, with the merge commit: https://git.collabora.com/cgit/user/nicolas/gst-plugins-bad.git/log/
I'm sorry, but as I've said before, the example in this branch is outdated. It still references GstXOverlay from 0.10... https://git.collabora.com/cgit/user/nicolas/gst-plugins-bad.git/tree/tests/examples/androidcamera/gst-camera-test/jni/camera_test.c?h=android-hardware-camera#n7
I think the examples need more work anyway, there are also some namespacing issues that need to be fixed. I think we should merge the camerasrc itself first, and the examples can always be added back later as a single commit, I don't think history is so interesting for the example code.
Agreed, do you have time to filter this out or shall I ?
I can, but not before tomorrow evening at the earliest, so feel free to do it yourself :)
What's the status here?
I think it's waiting for me to fix up the git stuff and push it (because of the merge commit the server-side hooks will have to be disabled for that).
And we agreed to remove the patches that adds the example (they are broken apparently, I only tested with the example repo provided). That should not be hard though.
So, that should make things simpler from now on. For any issues with this code, please open specific bugs. I'll open one to put back an example in -bad. commit 2193fa8ffeca7830cdce195c6cbff4cb16717178 Merge: 5b04e77 09dbc5b Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Thu Jan 21 13:50:44 2016 -0500 Merge branch 'android-hardware-camera' This branch adds support for Android Hardware Camera API through a new element called ahcsrc. This is the "old" Android Camera API, then only API available on Android 4.X. https://bugzilla.gnome.org/show_buf.cgi?id=737786