qgroundcontrol
qgroundcontrol copied to clipboard
fix android udp h264 video record
This PR refers to issue #9792. The fix it's only a copy and paste from this commit. Thanks to the original author @andrewvoznytsa. I think this is needed because the muxers (mp4mux, matroskamux and qtmux) only accepts stream-format=avc and not byte-stream. I've tested and got the video record of udp h264 video stream working again.
@kressinluiz @andrewvoznytsa Any ideas as to why this was in Stable 4.0 but then somehow disappeared? Wondering if it disappeared for some important reason.
No idea - I'm not working on QGC video for more than a year. I think someone initiated decodebin upgrade from decodebin (stable) to decodebin3 (experimental, IIRC) and dropped that code. It seems they did not pass full testing cycle and we have what we have.
I was able to track that this code got dropped on this commit by author @patrickelectric but I don't know if there is a reason. @patrickelectric could you give some input on this?
@patrickelectric Comments?
Any updates on this? Would really like to use the latest versions compatible with Ardupilot 4.1+ but no video recording is a deal breaker.
I just tested this commit on master with RTSP video on an Android device. Now video records as it should. Thanks.
I fixed the MacOS video by reverting the "decodebin" to "decodebin3" switch. "decodebin3" does not work on MacOS and results in a crash when trying to videostream. I changed that here: https://github.com/mavlink/qgroundcontrol/pull/10380
I rebased this fix on current master https://github.com/fredowski/qgroundcontrol/pull/6 and tested this on an Android phone: It works. Without the change, the red recording button remains red and the phone does not go into recording mode. With the change, the recording works again.
This also fixes the issue for me in Arch Linux
I just tested this and it fixes for me Android recording. I think it is an important one, I would like to merge.
I know a lot of work has been done by @fredowski lately on this front. Do you think it is a good idea to merge this one now? Would it implicate any downside from your part regarding the work you are doing on the rest of gstreamer commits? Thank you!
Would it implicate any downside from your part regarding the work you are doing on the rest of gstreamer commits? Thank you!
Hi @Davidsastresas, I think this one is good to be merged.
Thank you very much @fredowski. I will give it a couple days in case anybody has any additional thought about it. If everything all right I will get it merged.
Thanks!
After a couple weeks, I tested myself on android 11, 12 and 13 ( xiaomi poco x3 and samsung galaxy s7 ) and it works on my testing with rtsp video stream ( streaming through orqa fpv goggles ).
Andrew tridgel tested this on a Skydroid h16 and it wasn't working there, the saved files are 0 bytes, so I think we should figure out how to make this work on all devices, or at least verify on more devices, before merging. Any thoughts?
After having digged in this dirt for quite a while my impression is that one problem might be the use of the more sophisticated "decodebin", "decodebin3" and "parsebin" technology. This promises to automagically figure out the right codecs (including hardware accelerators) on all platforms - but it sometimes does not work and requires "hints" or it does not work at all. Maybe it is safer to step back to the architecture in 3.5.6 where the pipeline was set more explicit: See https://github.com/mavlink/qgroundcontrol/blob/Stable_V3.5.6/src/VideoStreaming/VideoReceiver.cc#L351
That uses the software codec on all platforms and the parsing is explicit.
The problem with that approach is that it end up using software decoders when others with lower priority are available, like: directx, nvidia, intel, amd. This can result in video fragment, higher latency and more, moving to decodebin3 and allowing the user to choose the video decoder priority fixes almost entirely all these issues.
Em qua., 11 de jan. de 2023 às 05:23, Friedrich Beckmann < @.***> escreveu:
After having digged in this dirt for quite a while my impression is that one problem might be the use of the more sophisticated "decodebin", "decodebin3" and "parsebin" technology. This promises to automagically figure out the right codecs (including hardware accelerators) on all platforms - but it sometimes does not work and requires "hints" or it does not work at all. Maybe it is safer to step back to the architecture in 3.5.6 where the pipeline was set more explicit: See https://github.com/mavlink/qgroundcontrol/blob/Stable_V3.5.6/src/VideoStreaming/VideoReceiver.cc#L351
That uses the software codec on all platforms and the parsing is explicit.
— Reply to this email directly, view it on GitHub https://github.com/mavlink/qgroundcontrol/pull/9838#issuecomment-1378388934, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJIYCPEQSS7ZLLWHPR3SHTWRZUXRANCNFSM5CVWMW2A . You are receiving this because you were mentioned.Message ID: @.***>
-- Patrick José Pereira Electronics Engineer
Hi @patrickelectric, I agree the current pipeline with parsebin + decodebin(3) promises to automagically select the correct parser and the possible hardware accelerated decoder. However see:
https://github.com/mavlink/qgroundcontrol/pull/10420#issuecomment-1353152604 => parsebin + decodebin3 does not show mpeg-ts already with gst-launch on MacOS. So something is not working when parsebin and decodebin3 are both trying to autonegotiate.
or
https://github.com/mavlink/qgroundcontrol/pull/10420#issuecomment-1288175173 => decodebin3 cannot fallback after vtdec_hw fails to decode on MacOS, decodebin does not negotiate the correct output format on Galaxy S5 mini.
At the moment it seems very difficult to conceive a stable system working on all platforms. You just added the feature that users are supposed to influence the gstreamer choice of the decoder. As @andrewvoznytsa already said:
The trick is to make it working on all platforms, keep control on everything and at the same time don't spend a lot of time maintaining manual pipeline building.
@kressinluiz can you just update the code based on the review ?
Closing in favor of https://github.com/mavlink/qgroundcontrol/pull/10610