GNOME Bugzilla – Bug 726325
RFC: Add tunneling support.
Last modified: 2018-11-03 12:59:47 UTC
Still in a quite early stage of development, but the basic idea should be clear now. All omx components that support tunneling should use the omx buffer pool on their sinks in their propose_allocation function. The source components can then detect that the OMX buffer pool is in use and establish direct tunneling. Please review and comment.
Created attachment 271861 [details] [review] #1 omxbufferpool-remove-other_pool-option
Created attachment 271862 [details] [review] #2 omxbufferpool-add-proper-type-definitions
Created attachment 271863 [details] [review] #3 omxvideoenc-use-omx-buffer-pool
Created attachment 271864 [details] [review] #4 omx-improve-tunneling-support
Created attachment 271865 [details] [review] #5 omx-always-enable-disable-both-tunnel-ends-v2
Created attachment 271866 [details] [review] #6 omx-wait-for-message-if-buffer-is-tunneled
Created attachment 271867 [details] [review] #7 omxvideoenc-add-in-port-tunneling
Created attachment 271868 [details] [review] #8 omxvideodec-add-out-port-tunneling
Review of attachment 271867 [details] [review]: ::: omx/gstomxvideoenc.c @@ +367,3 @@ } gst_omx_component_set_state (self->enc, OMX_StateLoaded); + if (!self->enc_in_port->tunneled) Why the inverted condition, just makes it harder to to read. @@ +1400,3 @@ + if (port->tunneled) { + gst_video_codec_frame_unref (frame); + return self->downstream_flow_ret; Why is there any frame flowing through if it's tunnelled ? I assume there should be no buffers flowing in the OMX tunnelled case, which probably means the decoder should drop all the GstVideoCodecFrames too.
Review of attachment 271862 [details] [review]: This one should go in anyway.
I agree that we should try to get rid of these fake data flow somehow. It's hacky and confusing :) Otherwise the general approach with the pools is the way to move forward, I like that. Also what needs to be kept in mind for tunneling is that you need to be careful to keep serialised events in the same order relative to data flow. A tag event between buffer A and B should still be between buffer A and B at the end of the tunnel. This is going to need forwarding events immediately until the end of the tunnel, and there then correlation with the output buffers. And for sink elements we would need to sync those events to the omx clock.
(In reply to comment #9) > Why the inverted condition, just makes it harder to to read. Thx, going to fix this in the next version of the patch. > @@ +1400,3 @@ > + if (port->tunneled) { > + gst_video_codec_frame_unref (frame); > + return self->downstream_flow_ret; > > Why is there any frame flowing through if it's tunnelled ? I assume there > should be no buffers flowing in the OMX tunnelled case, which probably means > the decoder should drop all the GstVideoCodecFrames too. Without data flow gst_video_encoder_chain never gets called, and so we never update the encoder caps. I tried fixing this, but I'm not deep enough into the gstreamer core components to find a fitting solution. It's indeed a hack, but for now I can live with it.
Review of attachment 271861 [details] [review]: It's used with 1- OMX_UseEGLImage or 2- OMX_UseBuffer paths: * 1- omxvideodec ! eglglessink and that's really important on Raspberry Pi OMX.broadcom.egl_render / OMX_UseEGLImage / EGLImage http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n729 http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n754 In this case the GstEGLImageBufferPool proposed by the videosink is decided by GstVideoDecoder. Then omxvideodec set a GstOMXBufferPool on the output port of the omx decoder. This later pool steals buffer from the GstEGLImageBufferPool. We need to do that because we do not choice which id of omx buffer we get filled on new frames. Whereas in default GstBufferPool implementation this a queue so that is necessary circular. * 2- omxvideodec ! ximagesink OMX_UseBuffer / XShmCreateImage http://cgit.collabora.com/git/user/julien/gst-omx.git/tree/omx/gstomxvideodec.c?h=resize#n1721 http://cgit.collabora.com/git/user/julien/gst-omx.git/tree/omx/gstomxvideodec.c?h=resize#n1743 Same as previous except the downstream buffer pool is the GstXImageBufferPool So case 2 shows that we still need that for every gstomx elements.
And thanks for all this effort. Also the patches are quite short but efficient so that's good, I expected much more lines. Could you provide the list of the omx elements you have on your platform ? Something like http://pastebin.com/iLu37Gqm . The tool is here http://cgit.freedesktop.org/gstreamer/gst-omx/tree/tools/listcomponents.c
Review of attachment 271864 [details] [review]: With a new gst_omx_is_tunneled helper :) ::: omx/gstomx.h @@ +200,3 @@ guint32 index; + GstOMXPort *tunneled; Maybe rename it to peer as for GstPad http://cgit.freedesktop.org/gstreamer/gstreamer/tree/gst/gstpad.h#n688
Review of attachment 271868 [details] [review]: ::: omx/gstomxvideodec.c @@ +51,3 @@ #include "gstomxvideo.h" #include "gstomxvideodec.h" +#include "gstomxvideoenc.h" I guess this is a residual of your initial try and then you forgot to remove it ? Otherwise I do not see why it's useful @@ +1146,3 @@ gst_video_codec_state_unref (state); + pool = gst_video_decoder_get_buffer_pool (GST_VIDEO_DECODER (self)); I think you never unref the pool (http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-get-buffer-pool) @@ +1154,3 @@ + if (gst_omx_setup_tunnel (self->dec_out_port, op->port) != OMX_ErrorNone) + GST_WARNING_OBJECT (self, "Failed to setup tunnel"); + } will it fallback to non-tunneling mode if it fails to setup a tunnel ? @@ +2230,3 @@ + if (self->dec_out_port->tunneled) { + frame->output_buffer = gst_buffer_new (); + gst_video_decoder_finish_frame (GST_VIDEO_DECODER (self), frame); Try to just use gst_video_decoder_release_frame http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-release-frame Because gst_video_decoder_finish_frame actually push the data. Note that on the gstomxencoder side you do need to have gst_omx_video_enc_handle_frame being called. In your other patch you just unref the frame. Better to no have the chain to be called at all in case of GST_PAD_PROBE_TYPE_BUFFER + tunnel @@ +2367,3 @@ GST_ERROR_OBJECT (self, "Failed to drain component: %s (0x%08x)", gst_omx_error_to_string (err), err); + g_mutex_unlock (&self->drain_lock); please put this in another patch @@ +2381,3 @@ + g_cond_wait_until (&self->drain_cond, &self->drain_lock, wait_until); + + if (!result) please put this in another patch @@ +2395,3 @@ GST_VIDEO_DECODER_STREAM_LOCK (self); self->started = FALSE; Is it possible to avoid starting the GstTask when tunneling http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1937 ? I mean is gst_omx_video_dec_loop necessary http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1161 ?
(In reply to comment #16) > Note that on the gstomxencoder side you do need to have > gst_omx_video_enc_handle_frame being called. Sorry, miss-typing: -you_do_need +you_do_not_need :)
(In reply to comment #12) > (In reply to comment #9) > Without data flow gst_video_encoder_chain never gets called, and so we never > update the encoder caps. > > I tried fixing this, but I'm not deep enough into the gstreamer core components > to find a fitting solution. It's indeed a hack, but for now I can live with it. Try to redefine sink_event http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst-libs/gst/video/gstvideoencoder.h#n264. Then on a GST_EVENT_CAPS just call gst_video_encoder_setcaps if tunneled.
Review of attachment 271865 [details] [review]: ::: omx/gstomx.c @@ +2001,3 @@ + g_mutex_unlock (&tunneled->comp->lock); + } + When tunneling, does it mean gst_omx_port_set_enabled_unlocked will be called twice on each port when tunneled ? I mean in the case of tunneling element1 ! element2 then element1 will enable its output port and the peer. And the element2 will enable its input port and the peer. So total would be 4 call to gst_omx_port_set_enabled_unlocked, 2 for each omx port. @@ +2194,3 @@ + g_mutex_unlock (&tunneled->comp->lock); + } + When tunneling, could it lead to a dead lock ? each port waiting for the other to be enabled ? (omxelem1 ! omxelem2)
Review of attachment 271866 [details] [review]: ::: omx/gstomx.c @@ +1315,3 @@ + goto retry; + } + when tunneling we do not have access to the buffers so it should be possible to avoid calling gst_omx_port_acquire_buffer. Maybe we can just avoid calling gst_pad_start_task (gst_omx_video_{dec/enc}_loop)
Review of attachment 271863 [details] [review]: ::: omx/gstomxvideoenc.c @@ +1665,2 @@ gst_query_add_allocation_meta (query, GST_VIDEO_META_API_TYPE, NULL); If tunneled, I think do not add GST_VIDEO_META_API_TYPE @@ +1683,3 @@ + + gst_query_add_allocation_pool (query, pool, GST_VIDEO_INFO_SIZE (&info), 0, + return FALSE; should unref the pool @@ +1688,3 @@ return GST_VIDEO_ENCODER_CLASS (gst_omx_video_enc_parent_class)->propose_allocation (encoder, query); I think in the best gst-omx tunneling support it would not need to have a buffer pool at all between tunneled gstomx elements. (note that here the pool has min=max=0 size I guess it is only here to share the peer port) If you manage to get ride of those task/loop (see other reviews), then gst_video_decoder_allocate_output_buffer (underlying call acquire on the buffer pool) and gst_omx_port_acquire_buffer would not be called on corresponding tunneled ports. I think the proper way to share the peer omx port would be to define a new allocation meta: GST_OMX_TUNNELING_META_API_TYPE and do the following in the propose_allocation side: s = gst_structure_new ("GstOMXTunnelingMeta", "peer", GST_OMX_PORT_TYPE, self->in_port, NULL); gst_query_add_allocation_meta (query, GST_OMX_TUNNELING_META_API_TYPE, s); Then in decide_allocation checks the presence of GST_OMX_TUNNELING_META_API_TYPE, if yes then retrieve the peer port.
(In reply to comment #13) > Review of attachment 271861 [details] [review]: > > It's used with 1- OMX_UseEGLImage or 2- OMX_UseBuffer paths: > > * 1- omxvideodec ! eglglessink and that's really important on Raspberry Pi > OMX.broadcom.egl_render / OMX_UseEGLImage / EGLImage > http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n729 > http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n754 > > In this case the GstEGLImageBufferPool proposed by the videosink is decided by > GstVideoDecoder. Then omxvideodec set a GstOMXBufferPool on the output port of > the omx decoder. This later pool steals buffer from the GstEGLImageBufferPool. > We need to do that because we do not choice which id of omx buffer we get > filled on new frames. Whereas in default GstBufferPool implementation this a > queue so that is necessary circular. Ah! It's only used in an #if/#endif block, thats why I couldn't find any user or it in my compile. > * 2- omxvideodec ! ximagesink > OMX_UseBuffer / XShmCreateImage > http://cgit.collabora.com/git/user/julien/gst-omx.git/tree/omx/gstomxvideodec.c?h=resize#n1721 > http://cgit.collabora.com/git/user/julien/gst-omx.git/tree/omx/gstomxvideodec.c?h=resize#n1743 > > Same as previous except the downstream buffer pool is the GstXImageBufferPool > > > So case 2 shows that we still need that for every gstomx elements. Then let's just drop the patch, it just looked like unused code to me.
(In reply to comment #14) > And thanks for all this effort. Also the patches are quite short but efficient > so that's good, I expected much more lines. > > Could you provide the list of the omx elements you have on your platform ? > Something like http://pastebin.com/iLu37Gqm . The tool is here > http://cgit.freedesktop.org/gstreamer/gst-omx/tree/tools/listcomponents.c That only shows one decoder and one encoder component so far. We are developing the components as well, so I will create scaling and conversion components in the future as well.
One really nice test case http://www.mdragon.org/test_ar_problem2.ts DVB ts with change of aspect ratio codec_data are changed in gst_omx_video_dec_set_format pipeline with decoder ! encoder and it crash on this assert gst_omx_port_allocate_buffers_unlocked: assertion `!port->tunneled' failed http://pastebin.com/NTugfGs9
(In reply to comment #16) > Review of attachment 271868 [details] [review]: > > ::: omx/gstomxvideodec.c > @@ +51,3 @@ > #include "gstomxvideo.h" > #include "gstomxvideodec.h" > +#include "gstomxvideoenc.h" > > I guess this is a residual of your initial try and then you forgot to remove it > ? Otherwise I do not see why it's useful Yeah, just a leftover. Fixed by now. > > @@ +1146,3 @@ > gst_video_codec_state_unref (state); > > + pool = gst_video_decoder_get_buffer_pool (GST_VIDEO_DECODER (self)); > > I think you never unref the pool > (http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-get-buffer-pool) Thx, fixed this as well. > @@ +1154,3 @@ > + if (gst_omx_setup_tunnel (self->dec_out_port, op->port) != OMX_ErrorNone) > + GST_WARNING_OBJECT (self, "Failed to setup tunnel"); > + } > > will it fallback to non-tunneling mode if it fails to setup a tunnel ? Yes, that's the plan. > > @@ +2230,3 @@ > + if (self->dec_out_port->tunneled) { > + frame->output_buffer = gst_buffer_new (); > + gst_video_decoder_finish_frame (GST_VIDEO_DECODER (self), frame); > > Try to just use gst_video_decoder_release_frame > http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-release-frame > > Because gst_video_decoder_finish_frame actually push the data. > > Note that on the gstomxencoder side you do need to have > gst_omx_video_enc_handle_frame being called. In your other patch you just unref > the frame. Better to no have the chain to be called at all in case of > GST_PAD_PROBE_TYPE_BUFFER + tunnel So far that didn't worked for various reasons, but I'm already working on cleaning this up as well. > @@ +2367,3 @@ > GST_ERROR_OBJECT (self, "Failed to drain component: %s (0x%08x)", > gst_omx_error_to_string (err), err); > + g_mutex_unlock (&self->drain_lock); > > please put this in another patch Done. > @@ +2381,3 @@ > + g_cond_wait_until (&self->drain_cond, &self->drain_lock, wait_until); > + > + if (!result) > > please put this in another patch That's actually an unnecessary change, going to remove it from the patch. > @@ +2395,3 @@ > GST_VIDEO_DECODER_STREAM_LOCK (self); > > self->started = FALSE; > > Is it possible to avoid starting the GstTask when tunneling > http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1937 ? > I mean is gst_omx_video_dec_loop necessary > http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1161 ? So far my current thinking is to still start the task so that it can react to errors send by the component.
(In reply to comment #19) > Review of attachment 271865 [details] [review]: > > ::: omx/gstomx.c > @@ +2001,3 @@ > + g_mutex_unlock (&tunneled->comp->lock); > + } > + > > When tunneling, does it mean gst_omx_port_set_enabled_unlocked will be called > twice on each port when tunneled ? > I mean in the case of tunneling element1 ! element2 then element1 will enable > its output port and the peer. And the element2 will enable its input port and > the peer. So total would be 4 call to gst_omx_port_set_enabled_unlocked, 2 for > each omx port. Yes that's correct, but the second calls are pretty much nops. > > @@ +2194,3 @@ > + g_mutex_unlock (&tunneled->comp->lock); > + } > + > > When tunneling, could it lead to a dead lock ? each port waiting for the other > to be enabled ? (omxelem1 ! omxelem2) Not as far as I can see. The problem I'm trying to solve is that for reaching idle/loaded state of a component you usually needs to enable/disable both ends of the tunnel.
(In reply to comment #20) > Review of attachment 271866 [details] [review]: > > ::: omx/gstomx.c > @@ +1315,3 @@ > + goto retry; > + } > + > > when tunneling we do not have access to the buffers so it should be possible to > avoid calling gst_omx_port_acquire_buffer. Maybe we can just avoid calling > gst_pad_start_task (gst_omx_video_{dec/enc}_loop) So far I've still used gst_omx_port_acquire_buffer for error handling, it just never really acquires a buffer in tunneled mode, but instead blocks for message signals to arive and only returns EOS/ERROR/FLUSHING. Without this nobody would process any such messages.
(In reply to comment #21) > Review of attachment 271863 [details] [review]: > > ::: omx/gstomxvideoenc.c > @@ +1665,2 @@ > gst_query_add_allocation_meta (query, GST_VIDEO_META_API_TYPE, NULL); > > > If tunneled, I think do not add GST_VIDEO_META_API_TYPE Actually I'm not sure why we need to add that in the first place. > > @@ +1683,3 @@ > + > + gst_query_add_allocation_pool (query, pool, GST_VIDEO_INFO_SIZE (&info), > 0, > + return FALSE; > > should unref the pool fixed. > > @@ +1688,3 @@ > return > GST_VIDEO_ENCODER_CLASS > (gst_omx_video_enc_parent_class)->propose_allocation (encoder, query); > > I think in the best gst-omx tunneling support it would not need to have a > buffer pool at all between tunneled gstomx elements. (note that here the pool > has min=max=0 size I guess it is only here to share the peer port) Disagree. Actually I think that it would be best to move most of the buffer allocation / deallocation handling into the buffer pool instead and in the case of tunneling setup and teardown of that as well. > If you manage to get ride of those task/loop (see other reviews), then > gst_video_decoder_allocate_output_buffer (underlying call acquire on the buffer > pool) and gst_omx_port_acquire_buffer would not be called on corresponding > tunneled ports. > > I think the proper way to share the peer omx port would be to define a new > allocation meta: GST_OMX_TUNNELING_META_API_TYPE and do the following in the > propose_allocation side: > > s = gst_structure_new ("GstOMXTunnelingMeta", "peer", GST_OMX_PORT_TYPE, > self->in_port, NULL); > gst_query_add_allocation_meta (query, GST_OMX_TUNNELING_META_API_TYPE, s); > > Then in decide_allocation checks the presence of > GST_OMX_TUNNELING_META_API_TYPE, if yes then retrieve the peer port. That actually sounds extremely odd to me, cause openmax tunneling parameters aren't metadata that could apply to any type of buffer.
(In reply to comment #27) > So far I've still used gst_omx_port_acquire_buffer for error handling, it just > never really acquires a buffer in tunneled mode, but instead blocks for message > signals to arive and only returns EOS/ERROR/FLUSHING. > > Without this nobody would process any such messages. Ah you are right. I agree :) (In reply to comment #28) > That actually sounds extremely odd to me, cause openmax tunneling parameters > aren't metadata that could apply to any type of buffer. Right indeed. Also now I remember max=0 actually means infinite :) So that's fine. Ok then last thing to check is comment #18, any thought ? I think the support for serialized events exposed in #11 can be added separately.
(In reply to comment #28) > > If tunneled, I think do not add GST_VIDEO_META_API_TYPE > > Actually I'm not sure why we need to add that in the first place. > the VIDEO meta is need if your element is producing data with special stride or with special padding (hence offset). Otherwise it would not be possible to map the video buffer. If you are not producing videobuffers, you don't care much. Though if the last element in a chain of tunneled omx component is not a sink, this element will have to care. > Disagree. Actually I think that it would be best to move most of the buffer > allocation / deallocation handling into the buffer pool instead and in the case > of tunneling setup and teardown of that as well. Long story short, to do things correctly, we also need an allocator and possibly a memory implementation. But this is not your fault. > > I think the proper way to share the peer omx port would be to define a new > > allocation meta: GST_OMX_TUNNELING_META_API_TYPE and do the following in the > > propose_allocation side: > > > > s = gst_structure_new ("GstOMXTunnelingMeta", "peer", GST_OMX_PORT_TYPE, > > self->in_port, NULL); > > gst_query_add_allocation_meta (query, GST_OMX_TUNNELING_META_API_TYPE, s); > > > > Then in decide_allocation checks the presence of > > GST_OMX_TUNNELING_META_API_TYPE, if yes then retrieve the peer port. > > That actually sounds extremely odd to me, cause openmax tunneling parameters > aren't metadata that could apply to any type of buffer. We are not talking about buffer here, but about allocation, like in allocation query. These are meta informat that you can set to help upstream make a decision about how allocation will be handled, or to figure-out if you need allocation at all. Obviously this is aiming other OMX component, not any kind of element. Also, it would remain private to the gst-omx. I think it's a promising approach, that is worth looking into.
Created attachment 272755 [details] [review] #1 omxbufferpool-stop-the-pool-before-destroying-it
Created attachment 272756 [details] [review] #2 omxvideoenc-use-omx-buffer-pool
Created attachment 272757 [details] [review] #3 omx-improve-tunneling-support
Created attachment 272758 [details] [review] #4 omx-always-enable-disable-both-tunnel-ends-v2
Created attachment 272759 [details] [review] #5 omx-wait-for-message-if-buffer-is-tunneled
Created attachment 272760 [details] [review] #6 omxvideoenc-add-in-port-tunneling
Created attachment 272761 [details] [review] #7 omxvideodec-add-out-port-tunneling
I've added a new set of patches, addressing most of the comments found in this bug report. Unfortunately I won't have time finishing this in the next month or so, hopeful I can come back to this task after that.
Created attachment 272839 [details] [review] omxvideodec: keep compatibilty with eglimage support
I was thinking that in the actual omxvideodec when using egl_render tunnelled with the decoder, I mean in the case where it is still not in its own gst element, then we have only one thread to catch all the omx messages. Whereas here with the tunnelling support for we have one thread created for each gstomxelement even when tunnelled together. I that case maybe I think only the last tunneled element should create a thread. In that case all the tunnelled element would need to know where to push those messages. So we maybe should share the message queue through the allocation query too. So that each tunnelled element can push in it.
(In reply to comment #40) > I was thinking that in the actual omxvideodec when using egl_render tunnelled > with the decoder, I mean in the case where it is still not in its own gst > element, then we have only one thread to catch all the omx messages. > > Whereas here with the tunnelling support for we have one thread created for > each gstomxelement even when tunnelled together. I that case maybe I think only > the last tunneled element should create a thread. > > In that case all the tunnelled element would need to know where to push those > messages. So we maybe should share the message queue through the allocation > query too. So that each tunnelled element can push in it. To be more clear, with the current patches we have N threads withomxelement1 ! ... ! omxelementN tunnelled together. So what I suggest is to share the message queue through the allocation query in order to create only one thread. And created by the most downstream tunnelled element, i.e. omxelementN.
Created attachment 273352 [details] [review] omxvideodec/enc: partially remove data flow if tunnelled Several problems from trying to remove the data flow: * need at least one buffer because the encoder initialize stuffs like caps. * Most of the synchronized events are attached to the frames, so that not calling gst_video_decoder_finish_frame will not send them. (I also tried to export and just call gst_video_decoder_prepare_finish_frame (TRUE) but that's a bit a hack) * If no data flow then gst_video_encoder_chain is not called, then gst_video_encoder_new_frame is not called and then http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideoenc.c#n544 is called with input param set to NULL. (and same pb about events) So in the end I wonder if this data flow is really annoying or if you have some ideas to improve my patch as I get sometimes some errors like: qtmux gstqtmux.c:2237:gst_qt_mux_add_buffer: decreasing DTS value 0:00:02.666666666 < 0:00:02.708333296 when doing gst-launch-1.0 filesrc location=movie.mp4 ! qtdemux ! h264parse ! omxh264dec ! omxh264enc ! h264parse ! qtmux ! filesink location=res.mp4
Also, synchronized events rely on the timestamp of the following buffer, not to mention that metas like CropMeta are only in the buffers themselves, but some are needed for tunnelled operation anyway. I am converging to the idea that some level of fake buffer flow will be required in any case in GStreamer 1.x. I guess in 2.x we could add timestamps to the events themselves and move Metas that are not really specific to each buffer (like CropMeta!) to events, but that's far off in the future. Slomo, Tim, Wim? Opinions?
For the metas you could do custom "meta tunneling" between tunneled elements, that forwards the meta the same way serialized events are forwarded... it really requires the same logic as you need to keep track of buffers for both cases, for the events you'll need to put them in the correct place between the correct buffers, for the metas you need to put them on the correct buffers (if the tunnel did not apply/drop the metas already). Main problem with the tunneling I see is that you can't use any of the existing base classes without fake dataflow, but that should be no blocker. Just additional work.
So basically you need a mechanism between tunneled elements, that at the beginning of the tunnel collects events and metas and sends them to the end of the tunnel somehow... and there then waits until the corresponding buffers arrive there (or for sinks until the OMX clock gives the corresponding time). Doesn't sound like a unsolveable problem ;) You will need the same to properly do fake dataflow too and especially to convert the fake dataflow to real dataflow at the end of the tunnel.
I think it would be great to at least support tunneling for transcode use cases. Hi Guillaume, is it something that you consider or intend to work on ? (It is not on my plans so I'm just curious of yours)
(In reply to Julien Isorce from comment #46) > I think it would be great to at least support tunneling for transcode use > cases. > Hi Guillaume, is it something that you consider or intend to work on ? > (It is not on my plans so I'm just curious of yours) Tunneling is not supported on the Zynq (same on Android). With DMABuf, and eventually DMABuf Fence, this won't be of any use for us.
This is a very good point! And it looks like "omxh264dec ! omxh264enc" is already working with dmabuf on Zynq right ? ( omxvideodec -> OMX_AllocBuffer -> pBuffer is a dmabuf -> OMXUseBuffer -> omxvideoenc), Guillaume do you confirm it works with current upstream gst-omx ? I knew about the decoder since I reviewed some Guillaume's patches but I missed those for the encoder or I thought it was specific to use with v4l2. And the requirement for the OMX headers is small, just 1 struct, 1 enum and 1 index: OMX_ALG_PORT_PARAM_BUFFER_MODE OMX_ALG_BUF_DMA OMX_ALG_IndexPortParamBufferMode Just do double check before I make a request to Tizonia to add something similar to its Ext headers too, there will be no issue with licence, correct ? (https://github.com/Xilinx/vcu-omx-il/tree/master/omx_header and https://github.com/Xilinx/vcu-omx-il/blob/master/LICENSE.md)
(In reply to Julien Isorce from comment #48) > This is a very good point! And it looks like "omxh264dec ! omxh264enc" is > already working with dmabuf on Zynq right ? ( omxvideodec -> OMX_AllocBuffer > -> pBuffer is a dmabuf -> OMXUseBuffer -> omxvideoenc), Guillaume do you > confirm it works with current upstream gst-omx ? Yes, the decoder exports dmabuf which are imported by the encoder. > Just do double check before I make a request to Tizonia to add something > similar to its Ext headers too, there will be no issue with licence, correct > ? (https://github.com/Xilinx/vcu-omx-il/tree/master/omx_header and > https://github.com/Xilinx/vcu-omx-il/blob/master/LICENSE.md) I'm not a legal expert so I'll ask around and will let you know.
This license is for the omx stack and have a weird Xilinx HW restriction. The OMX headers (including OMX_Allegro.h) are under Khronos license, which matches the notion of platform headers. And that, even if I'm not a lawyer, you can use use/duplicate.
Also see https://bugzilla.gnome.org/show_bug.cgi?id=796918#c49
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-omx/issues/2.