GNOME Bugzilla – Bug 728438
v4l2: Implement a v4l2 video encoder
Last modified: 2017-05-24 18:08:42 UTC
I will begin my work on exynos 4412 using MFC v5.1. The kernel is 3.5. I am not familiar with the working mechanism of encoder in gstreamer.
I can't make the v4l2videodec work on my platform And I can't register my element using the way used in v4l2-allocator and the old way could be registered but it doesn't work.
I didn't change the way the element is registered yet. Would it be possible to provide more information ?
Take note that I doubt 3.5 kernel can work well. There is many bugs there, and unless you address them there is no magic.
(In reply to comment #2) > I didn't change the way the element is registered yet. Would it be possible to > provide more information ? I am sorry about that, it is a old problem I have reported about sink_caps = gst_v4l2_video_enc_probe_caps (device, video_fd,V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE , gst_v4l2_object_get_raw_caps ()); in encoder, it is V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, but for decoder, it is V4L2_BUF_TYPE_VIDEO_OUTPUT.
gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro r: Failed to allocate required memory. 0:00:02.883168960 336 0xde350 WARN v4l2videoenc gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro r: Buffer pool activation failed about that problem, it is causing by the try to reqbuf while the count = 0, in s5p-mfc, the decoder will release the buffer but encoder won't.
(In reply to comment #5) > gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro > r: Failed to allocate required memory. > 0:00:02.883168960 336 0xde350 WARN v4l2videoenc > gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro > r: Buffer pool activation failed > > about that problem, it is causing by the try to reqbuf while the count = 0, > in s5p-mfc, the decoder will release the buffer but encoder won't. This looks like a bug in the driver, calling reqbuf(0) should never fail according to the v4l2 spec. This issue is most likely fixed in recent kernel, I would suggest to search for a backport. If you'd like to share your work, for a little more transparency, you may just post an URI to it, in a comment or through the "Add Bug URLs" form.
no, it doesn't fix in the and I have modified my kernel to support count == 0, but vb2_reqbufs() will failed. If I don't call vb2_reqbufs() while count == 0, it will also change ctx->state != MFCINST_GOT_INST which will failed s5p_mfc_queue_setup. I think the properly way to do that is to don't do that in gst_v4l2_buffer_pool_set_config， I am sorry I have no idea where have called REQBUF I set the bufferpool. I would like to share my work, I just didn't know that I shall do that if I haven't done it.
https://github.com/hizukiayaka/gst-plugins-good sorry, I can't add it in "see also“
(In reply to comment #6) > (In reply to comment #5) > > gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro > > r: Failed to allocate required memory. > > 0:00:02.883168960 336 0xde350 WARN v4l2videoenc > > gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro > > r: Buffer pool activation failed > > > > about that problem, it is causing by the try to reqbuf while the count = 0, > > in s5p-mfc, the decoder will release the buffer but encoder won't. > > This looks like a bug in the driver, calling reqbuf(0) should never fail > according to the v4l2 spec. This issue is most likely fixed in recent kernel, I > would suggest to search for a backport. If you'd like to share your work, for a > little more transparency, you may just post an URI to it, in a comment or > through the "Add Bug URLs" form. And please review https://github.com/hizukiayaka/gst-plugins-good/commit/0c94a99f34967f9cc14ee94b2f880896e0cf27fe Although it doesn't have any effect here, but it will avoid the future problem.
(In reply to comment #9) > And please review > https://github.com/hizukiayaka/gst-plugins-good/commit/0c94a99f34967f9cc14ee94b2f880896e0cf27fe > Although it doesn't have any effect here, but it will avoid the future problem. Fixed, slightly differently but CAN_REQUEST should work now. This flag check was with automatic io-mode in mind.
(In reply to comment #7) > no, it doesn't fix in the and I have modified my kernel to support count == 0, > but > vb2_reqbufs() will failed. If I don't call vb2_reqbufs() while count == 0, it > will also change ctx->state != MFCINST_GOT_INST which will failed > s5p_mfc_queue_setup. > I think the properly way to do that is to don't do that in > gst_v4l2_buffer_pool_set_config， I am sorry I have no idea where have called > REQBUF I set the bufferpool. > I would like to share my work, I just didn't know that I shall do that if I > haven't done it. We don't do that in set_config() any-more in git master, since set_config() can be called multiple time. As you stated setting REQBUFS(0) REQBUFS(N) multiple time is not efficient and may starve the migrating allocators.
Note in good practice, you should never merge code on top of unreviewed/un-accepted commits. It makes it really haed to review from you branch directly, and will make you life really hard to fix easy thing like style and stuff.
I have modified the gst_v4l2_allocator_new() to set flags |= GST_V4L2_ALLOCATOR_FLAG_MMAP_REQBUFS; directly but it doesn't work. ============================================================================ gstv4l2videoenc.c:446:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> Send ing header 0:00:03.621489960 319 0xde490 DEBUG v4l2 gstv4l2bufferpool.c:505:gst_v4l2_buffer_pool_set_config:<v4l2video9enc0:poo l:sink> config GstBufferPoolConfig, caps=(GstCaps)"video/x-raw\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ format\=\(string\)NV12\,\ interlace-mode\=\(string\)mixed\,\ pixel-aspect-ratio\=\(fraction\)1/1", size=(uint)116736, min-buffers=(uint)8, max-buffers=(uint)16, allocator=(GstAllocator)"NULL", params=(GstAllocationParams)NULL, options=(string)< GstBufferPoolOptionVideoMeta >; 0:00:03.621593002 319 0xde490 INFO v4l2 gstv4l2bufferpool.c:557:gst_v4l2_buffer_pool_set_config:<v4l2video9enc0:poo l:sink> can't allocate, setting maximum to minimum 0:00:03.621623293 319 0xde490 DEBUG v4l2videoenc gstv4l2videoenc.c:466:gst_v4l2_video_enc_handle_frame: set config 0:00:03.621664668 319 0xde490 WARN v4l2videoenc gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro r: Failed to allocate required memory. 0:00:03.621686210 319 0xde490 WARN v4l2videoenc gstv4l2videoenc.c:558:gst_v4l2_video_enc_handle_frame:<v4l2video9enc0> erro r: Buffer pool activation failed ============================================================== I back to the decoder, I can't find when does it call REQBUFS, I have make a debug message in echo ioctl but I can't find it. ============================================================== gstv4l2bufferpool.c:505:gst_v4l2_buffer_pool_set_config:<v4l2video8dec0:poo l:sink> config GstBufferPoolConfig, caps=(GstCaps)"video/x-h264\,\ level\=\(string\)2\,\ profile\=\(string\)high\,\ stream-format\=\(string \)byte-stream\,\ alignment\=\(string\)au\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ parsed\=\(boolean\)tr ue\,\ pixel-aspect-ratio\=\(fraction\)1/1", size=(uint)1048576, min-buffers=(uint)0, max-buffers=(uint)0, allocator=(GstAllocator)"NULL", p arams=(GstAllocationParams)NULL; 0:00:02.198014208 293 0xe8150 INFO v4l2 gstv4l2bufferpool.c:540:gst_v4l2_buffer_pool_set_config:<v4l2video8dec0:poo l:sink> increasing minimum buffers to 2 0:00:02.198035833 293 0xe8150 INFO v4l2 gstv4l2bufferpool.c:546:gst_v4l2_buffer_pool_set_config:<v4l2video8dec0:poo l:sink> reducing maximum buffers to 32 0:00:02.198056125 293 0xe8150 INFO v4l2 gstv4l2bufferpool.c:557:gst_v4l2_buffer_pool_set_config:<v4l2video8dec0:poo l:sink> can't allocate, setting maximum to minimum 0:00:02.198158666 293 0xe8150 DEBUG v4l2videodec gstv4l2videodec.c:437:gst_v4l2_video_dec_handle_frame:<v4l2video8dec0> Hand ling frame 0 0:00:02.233165666 293 0xe8150 DEBUG v4l2videodec gstv4l2videodec.c:455:gst_v4l2_video_dec_handle_frame:<v4l2video8dec0> Send ing header 0:00:02.233272041 293 0xe8150 DEBUG v4l2 gstv4l2bufferpool.c:505:gst_v4l2_buffer_pool_set_config:<v4l2video8dec0:poo l:sink> config GstBufferPoolConfig, caps=(GstCaps)"video/x-h264\,\ level\=\(string\)2\,\ profile\=\(string\)high\,\ stream-format\=\(string \)byte-stream\,\ alignment\=\(string\)au\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ parsed\=\(boolean\)tr ue\,\ pixel-aspect-ratio\=\(fraction\)1/1", size=(uint)1048576, min-buffers=(uint)2, max-buffers=(uint)2, allocator=(GstAllocator)"NULL", p arams=(GstAllocationParams)NULL; 0:00:02.233488209 293 0xe8150 DEBUG v4l2 gstv4l2bufferpool.c:706:gst_v4l2_buffer_pool_start:<v4l2video8dec0:pool:sin k> requesting 2 MMAP buffers 0:00:02.244613583 293 0xe8150 DEBUG v4l2 gstv4l2bufferpool.c:1260:gst_v4l2_buffer_pool_release_buffer:<v4l2video8dec 0:pool:sink> release buffer 0xb561ca00 0:00:02.244809583 293 0xe8150 LOG v4l2 gstv4l2bufferpool.c:1329:gst_v4l2_buffer_pool_release_buffer:<v4l2video8dec 0:pool:sink> buffer 0 not queued, putting on free list 0:00:02.246805458 293 0xe8150 DEBUG v4l2 gstv4l2bufferpool.c:1260:gst_v4l2_buffer_pool_release_buffer:<v4l2video8dec 0:pool:sink> release buffer 0xb561caa0 0:00:02.246828916 293 0xe8150 LOG v4l2 gstv4l2bufferpool.c:1329:gst_v4l2_buffer_pool_release_buffer:<v4l2video8dec 0:pool:sink> buffer 1 not queued, putting on free list 0:00:02.246867499 293 0xe8150 LOG v4l2 gstv4l2object.c:2882:gst_v4l2_object_unlock_stop:<v4l2video8dec0> flush sto p poll
(In reply to comment #13) > I have modified the gst_v4l2_allocator_new() > to set flags |= GST_V4L2_ALLOCATOR_FLAG_MMAP_REQBUFS; directly but it doesn't Flags where never set in the allocator, was reported in the devel list last week. It's fixed now: http://cgit.collabora.com/git/user/nicolas/gst-plugins-good.git/commit/?h=v4l2-allocator&id=d6b8f5fd28ca0922b29a1bd0f3716220a0c9649a
I review my code, I don't set the request min equal to the max, so causing the failure. Then I met the really problem, when I try to active the pool for OUTPUT, it will do actually VIDIOC_REQBUFS(I have removed probe in gst_v4l2_allocator_new) [ 908.185000] s5p_mfc_queue_setup:1639: inavlid state: 0 [ 908.185000] vidioc_reqbufs:1075: error in vb2_reqbufs() for E(S) the error shows that the queue is not MFCINST_GOT_INST, the status of the driver is still MFCINST_FREE, I don't know what cause that, as s_fmt should change the driver status(ctx->state) to MFCINST_GOT_INST. Although the allocator should both work on encoder and decoder, but the implement actually doesn't.
I suspect it's waiting for more instruction before letting allocated the queue. Encoders need specific settings, I would look if there is a CID that may change the state.
(In reply to comment #16) > I suspect it's waiting for more instruction before letting allocated the queue. > Encoders need specific settings, I would look if there is a CID that may change > the state. As I have done a ugly version of mfc-encoder(modified from mfc-decoder in -ugly), it doesn't needed. I don't know what cause that, I will try to make sure it. Here is the new branch https://github.com/hizukiayaka/gst-plugins-good/tree/v4l2-videoenc I still comment the probe code in it, as I want to it do the least work to find out the problem.
I have fixed the requesting buffer in OUTPUT side https://github.com/hizukiayaka/gst-plugins-good/commit/1ad98247291ff9d5c65a60c66123a1caa252dc21 but I got the problem below, it seems that it has some problem in CAPTURE side and my code is not clean so may be your code is correct. 0:00:01.734592376 235 0xe0d80 DEBUG v4l2 gstv4l2bufferpool.c:438:gst_v4l2_buffer_pool_set_config:<v4l2video9enc0:poo l:src> config GstBufferPoolConfig, caps=(GstCaps)"video/x-h264\,\ stream-format\=\(string\)byte-stream\,\ alignment\=\(string\)au\,\ width\ =\(int\)320\,\ height\=\(int\)240\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ framerate\=\(fraction\)30/1", size=(uint)0, min-buffers=(uint)0 , max-buffers=(uint)0, allocator=(GstAllocator)"NULL", params=(GstAllocationParams)NULL; 0:00:01.734828335 235 0xe0d80 DEBUG v4l2 gstv4l2bufferpool.c:451:gst_v4l2_buffer_pool_set_config:<v4l2video9enc0:poo l:src> can allocate is 0 0:00:01.734851876 235 0xe0d80 INFO v4l2 gstv4l2bufferpool.c:474:gst_v4l2_buffer_pool_set_config:<v4l2video9enc0:poo l:src> increasing minimum buffers to 2 0:00:01.734870835 235 0xe0d80 INFO v4l2 gstv4l2bufferpool.c:480:gst_v4l2_buffer_pool_set_config:<v4l2video9enc0:poo l:src> reducing maximum buffers to 32 0:00:01.734888751 235 0xe0d80 INFO v4l2 gstv4l2bufferpool.c:491:gst_v4l2_buffer_pool_set_config:<v4l2video9enc0:poo l:src> can't allocate, setting maximum to minimum 0:00:01.734978418 235 0xe0d80 DEBUG v4l2 gstv4l2object.c:3118:gst_v4l2_object_decide_allocation:<v4l2video9enc0> all ocation: size:0 min:0 max:0 pool:(NULL) 0:00:01.735018751 235 0xe0d80 DEBUG v4l2 gstv4l2object.c:3171:gst_v4l2_object_decide_allocation:<v4l2video9enc0> str eaming mode: using our own pool <v4l2video9enc0:pool:src> 0:00:01.735177335 235 0xe0d80 WARN v4l2 gstv4l2object.c:3318:gst_v4l2_object_decide_allocation:<v4l2video9enc0> err or: Video device did not suggest any buffer size. ERROR: from element /GstPipeline:pipeline0/v4l2video9enc:v4l2video9enc0: Video device did not suggest any buffer size. Additional debug info: gstv4l2object.c(3318): gst_v4l2_object_decide_allocation (): /GstPipeline:pipeline0/v4l2video9enc:v4l2video9enc0 ERROR: pipeline doesn't want to preroll. Setting pipeline to NULL ...
(In reply to comment #18) > 0:00:01.735177335 235 0xe0d80 WARN v4l2 > gstv4l2object.c:3318:gst_v4l2_object_decide_allocation:<v4l2video9enc0> err > or: Video device did not suggest any buffer size. > ERROR: from element /GstPipeline:pipeline0/v4l2video9enc:v4l2video9enc0: Video > device did not suggest any buffer size. You'll have to debug why the sizeimage didn't got set to ENCODED_BUFFER_SIZE. The MFC does not propose anything.
I find gst_query_get_n_allocation_pools in gstreamer cause the problem, it doesn't return is not > 0, but I don't what does that function do
the actually problem happened in ensure_array () gstquery.c:833 of gstreamer for decoder in line 837 value = gst_structure_id_get_value (s, quark); value is not NULL but for encoder value is NULL.
I wonder does encoder could be merged, the allocate will be different to decoder, it can't stream on on all side, if you don't queued any buffer into devices in both sides. And for OUTPUT, you must queue all the buffers after requested, when you want to use it you must dquf it first then req-qbuf it again. It is the same in CAPTURE.
I was wrong, the encoder doesn't need to enqueue all the buffers after requesting the buffers, the difference is just the when to start the STREAMON. I meet a problem now, when I call the allocator for the CAPTURE side, it seems that the allocator doesn't see properly. I wonder where it is set. Breakpoint 7, gst_v4l2_allocator_start (allocator=0xb5e2c208, count=count@entry=2, memory=memory@entry=1) at gstv4l2allocator.c:659 659 { (gdb) n 660 struct v4l2_requestbuffers breq = { count, allocator->type, memory }; (gdb) 659 { (gdb) 664 g_return_val_if_fail (count != 0, 0); (gdb) print allocator->type $39 = 0 (gdb) print count $40 = 2
We should try not make this bug a forum. Maybe you want to ask questions on IRC ?
https://github.com/hizukiayaka/gst-plugins-good/commit/3732f35e180725207e079d0cc1bd985497fb0a18 this commit is rebase and package commit. One merge problem: about start stream, in encoder, it can't start stream as the other device. One function problem: The encoded data is only black and white, as I using the videotestsrc, only the snow at the right button of screen making me sure that the encoder work at lease.
Using those patch https://chromium-review.googlesource.com/#/c/65926/2/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c https://chromium-review.googlesource.com/#/c/66624/2/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c instead. I don't know which branch of ChromeOS those patches belonged to.
(In reply to comment #26) > Using those patch > https://chromium-review.googlesource.com/#/c/65926/2/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c > https://chromium-review.googlesource.com/#/c/66624/2/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c > instead. > I don't know which branch of ChromeOS those patches belonged to. Ah, these make more sense, have they been submitted upstream ?
Apply this patch in 3.16-rc5 http://news.gmane.org/gmane.linux.drivers.video-input-infrastructure Also a kernel for tiny4412 could be found here. git clone git.soulik.info:public/linux-kernel This step has made the encoder basic work, the next step is to make it as a class. The screen mess may blame the kernel 3.5 although my old code doesn't have any problem. The v4l2src solved with gstreamer 0d5ddc96f214f41f10faf87bceeca0bb7b1ad93c and gst-plugins-base a0dd71ad805db89725ab8265bab7c047ff515501. But I don't know what cause that.
https://github.com/hizukiayaka/gst-plugins-good/tree/v4l2-videoenc-review There is also a branch called v4l2-videoenc which split echo patches.
https://github.com/hizukiayaka/gst-plugins-good/tree/v4l2-videoenc-review Actually, it just a update version of below.
I have update it to latest version at 20th Nov 2014. I have split the patches into three parts as Nicolas Dufresne (stormer) reqested. create a base encoder class(the change for v4l2 decoder is also here) + create an h264 en coder element + all the other small unrelated required changes. The commit 45a1f60df064e68fca64e1e5af76518e44d38f46 v4l2: add GstV4l2VideoCData to v4l2-utils.h should be merged into the first part but it broke the code style making diff file strange, that is why it is split.
I have rebased ayaka's work on recent master and fixed some bugs Here is my tree: https://github.com/Vodalys/gst-plugins-good/tree/v4l2-videoenc I can make the encoder work using CODA driver However, framerate needs to be transfered to the driver (needed for bitrate) which is not implemented for now.
(In reply to comment #32) > I have rebased ayaka's work on recent master and fixed some bugs > > Here is my tree: > https://github.com/Vodalys/gst-plugins-good/tree/v4l2-videoenc > I have reviewed your code, I know that I am not good at glib, I appreciate what you do. > I can make the encoder work using CODA driver > > However, framerate needs to be transfered to the driver (needed for bitrate) > which is not implemented for now. But I don't know this part, I don't see that in your code. Could tell me more detail about that. Thank you
The GStreamer plugin needs to provide the negociated framerate to the driver, because this information is needed by the encoder to fulfill the bitrate Here is the discussion on linux-media ML: http://www.mail-archive.com/linux-media@vger.kernel.org/msg83541.html
(In reply to comment #34) > The GStreamer plugin needs to provide the negociated framerate to the driver, > because this information is needed by the encoder to fulfill the bitrate > Yes, right now we only set this on capture device, but it's more natural to set this on the output device for encoders. So we should also have a patch along these line merged with the encoder.
Created attachment 299793 [details] [review] Patch to set video framerate on output
What would need to be done now for the encoder to be merged?
Review of attachment 299793 [details] [review]: The commit log is of wrong form, should be: v4l2videoenc: ... Long descriptions, this is needed here. Bug URI ::: sys/v4l2/gstv4l2object.c @@ +2868,3 @@ + v4l2object->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE || + v4l2object->type == V4L2_BUF_TYPE_VIDEO_OUTPUT || + v4l2object->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { Please, remove the if, this covers all the buffer type we support.
Created attachment 299821 [details] [review] v4l2videoenc: Set framerate on output interface v4l2videoenc: Set framerate on output interface Framerate is only set on capture interface. For the encoder, it is needed to set it on the output interface too, for the driver to be notified of the negotiated framerate value.
B(In reply to Frédéric Sureau from comment #39) > Created attachment 299821 [details] [review] [review] > v4l2videoenc: Set framerate on output interface > > v4l2videoenc: Set framerate on output interface > > Framerate is only set on capture interface. > For the encoder, it is needed to set it on the output interface too, > for the driver to be notified of the negotiated framerate value. But the main problem is that the document of v4l2 doesn't support us to do that.
(In reply to ayaka from comment #40) > But the main problem is that the document of v4l2 doesn't support us to do > that. It's a fair point, but I would expect the driver to update the field if not supported. As we read-back the value it should be fine. Or did I miss something ?
(In reply to Frédéric Sureau from comment #32) > I have rebased ayaka's work on recent master and fixed some bugs > > Here is my tree: > https://github.com/Vodalys/gst-plugins-good/tree/v4l2-videoenc > > I can make the encoder work using CODA driver > > However, framerate needs to be transfered to the driver (needed for bitrate) > which is not implemented for now. Sorry to review your work so late, about 076fc1b2054bb99cceb3ab69f5d9915cc6da4537 I accept. It is my fault and careless. about 85a377f36dbc329c5c8d00fa7a5ee31184d69348 I leave those functions to future extend. about 56046da2069990b22ea0e50480cf0a3c94c31787 I can't make any agreement this time. If the v4l2 upstream doesn't change the document, your modification will break the document. Thank you for you work again.
Just letting you know that this bug is a mess. Can we get a patch series attached here and ready for review ?
Created attachment 307178 [details] [review] modify GstV4l2VideoDecCData to GstV4l2VideoCDat
Created attachment 307179 [details] [review] move GstVideoCData to v4l2-utils.h because I broke the code style, so I make this patch separately. It should be merge into first patch.
Created attachment 307180 [details] [review] the main patch
Why not implement _propose_allocation ()?
Review of attachment 307180 [details] [review]: ::: sys/v4l2/gstv4l2h264enc.c @@ +117,3 @@ + gobject_class->get_property = + GST_DEBUG_FUNCPTR (gst_v4l2_h264_enc_get_property); + /* FIXME propose_allocation or not ? */ Yes, add that (also very simple to do, just forward the call to gst_v4l2_object_propose_allocation()). I don't think you need to add anything to the default function.
ayaka: we have a propose_allocation in our gitlab repo (https://gitlab.com/veo-labs/gst-plugins-good/commit/52d1aa7d9ba751866c6e5a7b42d1dc0131044071). Are you planning on working on this again ? If so, then, right now there is a bug with the coda encoder (but probably others) which makes it stop after 32 start/stop. The real issue is that the encoder instance is not closed, because a buffer is leaked. I still have not found the correction, but if you have some time to take a look, it could help.
Created attachment 310074 [details] [review] videoencoder: Adding propose_allocation method Attached gitlab propose allocation patch.
(In reply to Jean-Michel Hautbois (jmleo) from comment #49) > ayaka: we have a propose_allocation in our gitlab repo > (https://gitlab.com/veo-labs/gst-plugins-good/commit/ > 52d1aa7d9ba751866c6e5a7b42d1dc0131044071). > Are you planning on working on this again ? > If so, then, right now there is a bug with the coda encoder (but probably > others) which makes it stop after 32 start/stop. > > The real issue is that the encoder instance is not closed, because a buffer > is leaked. > I still have not found the correction, but if you have some time to take a > look, it could help. I am very sorry about that. I didn't meet that problem in my exynos 4412(I have long time not update my git repo), do you meet the same problem when you use the same version as mine? And which hardware do you use? Could you tell me more detail in IRC, I will be there for a few days. Thank you
(In reply to ayaka from comment #51) > (In reply to Jean-Michel Hautbois (jmleo) from comment #49) > > ayaka: we have a propose_allocation in our gitlab repo > > (https://gitlab.com/veo-labs/gst-plugins-good/commit/ > > 52d1aa7d9ba751866c6e5a7b42d1dc0131044071). > > Are you planning on working on this again ? > > If so, then, right now there is a bug with the coda encoder (but probably > > others) which makes it stop after 32 start/stop. > > > > The real issue is that the encoder instance is not closed, because a buffer > > is leaked. > > I still have not found the correction, but if you have some time to take a > > look, it could help. > > I am very sorry about that. > I didn't meet that problem in my exynos 4412(I have long time not update my > git repo), do you meet the same problem when you use the same version as > mine? I don't know, I have a modified version for videoaggregator as well. The issue is reproducible only if you switch the pipeline from NULL to PLAYING, then NULL, PLAYING, etc. You can't reproduce it with gst-launch as it will close all open files. > And which hardware do you use? i.MX6 with coda driver. I don't think it is a driver issue. > Could you tell me more detail in IRC, I will be there for a few days. Of course, please ping me.
Please Jean-Michel, consider merging this pull request in your repo as part of the process to get a working V4L2 encoder that can be merged upstream in gst-plugins-good: https://gitlab.com/veo-labs/gst-plugins-good/merge_requests/1 It configures the output resolution of the encoder to the same one used for input, instead of leaving it unchanged in its default value. Thanks!
There is the other kind of Hardware Video Processor, which is stateless. That means that it request some encoding/decoding settings to be set before a frame every times. Although the Linux kernel will support this kind of device in V4L2. But it is not a good idea to do it in this element. The glue layer is still needed, the VA-API would be a good choice for this.
Thanks for everyone's work on this. I've applied some patches (ayaka, I think?) to current master and 1.10.2 here (1.10.2-patched and master branches): https://github.com/fnoop/gst-plugins-good They work perfectly for me on odroid xu4 to use the MFC for encoding. It would be great to get this working in the main tree somehow. Even if it's not perfect, it will at least do something rather than the current nothing, and give something to work on and improve in the future.
(In reply to Fnoop from comment #55) > Thanks for everyone's work on this. I've applied some patches (ayaka, I > think?) to current master and 1.10.2 here (1.10.2-patched and master > branches): > https://github.com/fnoop/gst-plugins-good > > They work perfectly for me on odroid xu4 to use the MFC for encoding. It > would be great to get this working in the main tree somehow. Even if it's > not perfect, it will at least do something rather than the current nothing, > and give something to work on and improve in the future. Thank you for you caring, Nicolas is merged my patches. And I am busy with the rockchip platform.
Thanks for rebasing this work. I'm still trying to find some time to review and correct few things in there. I can provide details in Jan if requested.
(In reply to Fnoop from comment #55) > Thanks for everyone's work on this. I've applied some patches (ayaka, I > think?) to current master and 1.10.2 here (1.10.2-patched and master > branches): > https://github.com/fnoop/gst-plugins-good > > They work perfectly for me on odroid xu4 to use the MFC for encoding. It > would be great to get this working in the main tree somehow. Even if it's > not perfect, it will at least do something rather than the current nothing, > and give something to work on and improve in the future. Thank you for your work on this. I’ve applied your patches (1.10.2-patched branch of https://github.com/fnoop/gst-plugins-good) on my environment (GStreamer 1.8.0) to test them with STMicroelectronics HVA (Hardware Video Accelerator) V4L2 driver (H.264 encoding). The patches applied easily, and the resulting V4L2 video encoder plugin works correctly on HVA V4L2 driver, even if there are some troubles with some configurations (e.g. management of the alignment between the decoder and the encoder, last frame not encoded). Note that these troubles might be due to the fact that I used an “old” version of GStreamer. The control parameters for the encoder driver are correctly set through the "extra-controls" property of the GStreamer encoder plugin. Example: gst-launch-1.0 videotestsrc num-buffers=100 ! "video/x-raw(ANY),format=NV12" ! v4l2video1h264enc extra-controls="ctrl,h264_profile=4,h264_entropy_mode=1" ! h264parse ! qtmux ! filesink location=./out.mp4 In short, from the tests that I ran on HVA, I agree with you that “it would be great to get this working in the main tree somehow”. It would give the opportunity to improve this common plugin.
Hi, I can report using successfully this video encoder plugin too on Dragonboard 410C (Qualcomm APQ8016). I'm running Debian with Gstreamer 1.10.2, I have applied the 12 patches from: https://github.com/fnoop/gst-plugins-good/commits/master I have drivers for camera (Linaro tree) and Venus video encoder (linux-media kernel mail list) and I'm able to do video encoding with the following pipeline: gst-launch-1.0 -v -e v4l2src device=/dev/video3 ! video/x-raw,format=NV12,width=1280,height=960,framerate=30/1 ! v4l2video5h264enc extra-controls="controls,h264_profile=4,video_bitrate=200000;" ! h264parse ! mp4mux ! filesink location=enc.h264.04.mp4 I agree that it will be good to have this plugin merged in the official tree and if there is something that we can do about this please let us know.
Thanks for those feedback. Nicolas, could you tell the progress of the that.
Nothing on my side, I got an x4, a 410c, an stm, all still waiting to be bootstrapped. For the record, what's missing is: - Profile negotiation - _finish bug fix - code cleanup mention in the reviews - subclassing fixes - Expose a minimum of properties, extra-controls is not ideal - some leak fixes (I think jmleo got that in his branch) - properly attached patches, for review (git bz?)
I only have the exynos 4412, but I may have some time this week, I will try to fix those reminding problems.
ayaka Do you feel like pushing that branch forward? I would like to get the ball rolling and get that merge, probably soon in the 1.14 cycle. I could start and review/fix mentionned issues and properly propose in bz if no one as time for that, just let me know if you are interested.
Yes, I will. The problem is I have not used my exynos board for a long time, it is also most brick. I have tried compiling the plugin in the last weekend, but failed. Without a valid board is the main problem, but I will try to fix my board. The only problem I have with the code is the propose_allocation(), I didn't understand it clearly.
(In reply to ayaka from comment #64) > Yes, I will. > The problem is I have not used my exynos board for a long time, it is also > most brick. I have tried compiling the plugin in the last weekend, but > failed. > Without a valid board is the main problem, but I will try to fix my board. > The only problem I have with the code is the propose_allocation(), I didn't > understand it clearly. If h/w availability becomes a bottleneck, i should be able to provide a couple of Dragonboard 410c. This board comes with decent mainline support by now (include mesa/freedreno) and we have a proper v4l2 m2m driver for decoder and encoder which is on its way to mainline. This is the driver we use to validate the encoder patches from above (see Todor message). We also have a proper CSI camera driver in review on the mailing list , which can be used with the gst video encoder for h/w accelerated video record pipeline. at any rate, we are very interested to help get this plugin merged.
For a customer I gathered and rebased the patches for this v4l2 encoder in iMX6. I fixed many of the issues aimed by Nicolas in comment 61. Though, it was backported for gstreamer 1.4 As ayaka mentioned in comment 64, I also need time to setup a gstreamer master environment and port my code.
Hi, I continue to use the video encoder plugin on DB410c. I'm trying to use dmabuf - to allocate buffers for video encoder sink, then export them and import on v4l2src (camera). The pipeline is: v4l2src device=/dev/video3 io-mode=dmabuf-import ! video/x-raw,format=NV12,width=1280,height=960,framerate=30/1 ! v4l2video4h264enc output-io-mode=dmabuf extra-controls="controls,h264_profile=4,video_bitrate=200000;" ! h264parse ! mp4mux ! filesink location=/home/linaro/enc.h264.24.mp4 The full log is on: https://hastebin.com/birigavera.rb (there are also some additional prints there) It fails with errors: gstv4l2allocator.c:1131:gst_v4l2_allocator_import_dmabuf:<v4l2src0:pool:src:allocator> Memory 0 is not of DMABUF gstv4l2bufferpool.c:366:gst_v4l2_buffer_pool_import_dmabuf:<v4l2src0:pool:src> failed to import dmabuf Investigating this I have reached the point to understand that there are no buffers requested (via v4l2) from the video encoder at the moment when import on v4l2src is done. Further, I can see that there are two buffer pools created: v4l2video4h264enc0:pool:sink and videobufferpool0. When v4l2src enters in "decide allocation", it receives videobufferpool0 as other_pool and allocates the buffers from it. It seems to me that this is wrong and instead it should receive v4l2video4h264enc0:pool:sink and allocate the buffers from it, which will actually request buffers from the video encoder driver. Is this correct? Any idea why this happens? Thank you, Todor
It's more complicated then that. To import a buffer, you still need to request buffers with the special type DMABUF. To v4l2src will "allocate" v4l2 buffer with request type DMABUF. While v4l2enc should allocate with type MMAP. v4l2enc will export those mmap buffer to DMABuf Fds if you have set the right mode. So you have two buffer pools, what v4l2src will do, is allocate from the downstream pool, and then import the FD into the v4l2src pool memory, keep a reference on the downstream buffer and then Queue that buffer to be filled (capture). When it comes out, we detach the FD from the v4l2src buffer, and push the filled downstream buffer. The allocation happens when the pools are activated. In you case, what fails is the importation. It fails because the downstream encoder is not producing exporting dmabuf. The encoder should support output-io-mode, and you should have set that to "dmabuf". Another future note, this exportation/importation should be negotiated, and should happen automatically. Final note, upstream importation like this does only works if by chance the selected stride in your source matches the selected encoder stride. We are missing code to setup the source to use the downstream stride here.
Thank you for the explanation Nicolas. (In reply to Nicolas Dufresne (stormer) from comment #68) > It's more complicated then that. To import a buffer, you still need to > request buffers with the special type DMABUF. To v4l2src will "allocate" > v4l2 buffer with request type DMABUF. Yes, this is done in my case. When I enable v4l2 debug I can see that DMABUF buffers are requested (video3 is the camera): video3: VIDIOC_REQBUFS: count=2, type=vid-cap-mplane, memory=dmabuf > While v4l2enc should allocate with > type MMAP. v4l2enc will export those mmap buffer to DMABuf Fds if you have > set the right mode. Well, this doesn't happen in my case and I'm searching for the reason.. > > So you have two buffer pools, what v4l2src will do, is allocate from the > downstream pool, It seems to me that v4l2src receives a wrong downstream pool to allocate from (videobufferpool0 instead of v4l2video4h264enc0:pool:sink). > and then import the FD into the v4l2src pool memory, keep a > reference on the downstream buffer and then Queue that buffer to be filled > (capture). When it comes out, we detach the FD from the v4l2src buffer, and > push the filled downstream buffer. > > The allocation happens when the pools are activated. In you case, what fails > is the importation. It fails because the downstream encoder is not producing > exporting dmabuf. The encoder should support output-io-mode, and you should > have set that to "dmabuf". gst-inspect reports that the encoder plugin supports output-io-mode. I do set this to "dmabuf", this is my pipeline: v4l2src device=/dev/video3 io-mode=dmabuf-import ! video/x-raw,format=NV12,width=1280,height=960,framerate=30/1 ! v4l2video4h264enc output-io-mode=dmabuf extra-controls="controls,h264_profile=4,video_bitrate=200000;" ! h264parse ! mp4mux ! filesink Is it the case that the encoder plugin does not support "output-io-mode=dmabuf" correctly? > Another future note, this exportation/importation > should be negotiated, and should happen automatically. Final note, upstream > importation like this does only works if by chance the selected stride in > your source matches the selected encoder stride. We are missing code to > setup the source to use the downstream stride here. In my case both match. But in general case it could be a problem, thanks for this info.
(In reply to Todor Tomov from comment #69) > > v4l2src device=/dev/video3 io-mode=dmabuf-import ! > video/x-raw,format=NV12,width=1280,height=960,framerate=30/1 ! > v4l2video4h264enc output-io-mode=dmabuf > extra-controls="controls,h264_profile=4,video_bitrate=200000;" ! h264parse ! > mp4mux ! filesink > > Is it the case that the encoder plugin does not support > "output-io-mode=dmabuf" correctly? I think so, it should support userptr, but I don't think it is necessary to do that as it would invoke the IOMMU which decrease the speed. > > > Another future note, this exportation/importation > > should be negotiated, and should happen automatically. Final note, upstream > > importation like this does only works if by chance the selected stride in > > your source matches the selected encoder stride. We are missing code to > > setup the source to use the downstream stride here. > In my case both match. But in general case it could be a problem, thanks for > this info.
(In reply to ayaka from comment #70) > (In reply to Todor Tomov from comment #69) > > > > > v4l2src device=/dev/video3 io-mode=dmabuf-import ! > > video/x-raw,format=NV12,width=1280,height=960,framerate=30/1 ! > > v4l2video4h264enc output-io-mode=dmabuf > > extra-controls="controls,h264_profile=4,video_bitrate=200000;" ! h264parse ! > > mp4mux ! filesink > > > > Is it the case that the encoder plugin does not support > > "output-io-mode=dmabuf" correctly? > I think so, it should support userptr, but I don't think it is necessary to > do that as it would invoke the IOMMU which decrease the speed. Ok, so as I understand it is not a bug but missing implementation. Do you know in more details what needs to be done for the output-io-mode=dmabuf support?
During the hackfest in A Coruna, I have conducted a large rework of this element. I believe the "form" is now correct without any known major hacks like it use to be. Though, it only had few hours of testing on the DB410c. I'm sharing my branch now, though expect this to only land in a week or two. I'm still undecided if I squash all the commits or keep it this way. One note, I have remove the patch "adding" extra controls, as this was not useful, it is already handled by the "default:" case of the get_property function. Other then that, it does implement h264 profile and level negotiation and does not seem to present padding artifact like it use to. Please test on your respective platform in case it has any regression. I'll merge on-time in any case so we can continue fixing in the repository. Other CODEC are coming. https://git.collabora.com/cgit/user/nicolas/gst-plugins-good.git/log/?h=v4l2-next Note that we need to teach the v4l2object to only probe a specific format, to avoid wasting time creating large caps for all the codecs.
Created attachment 352517 [details] [review] v4l2: increase pre-allocated encoded buffer size As of today, the MFC encoder often need to exceed that 1 MB size for encoded buffer we fixed earlier for decoding.
Created attachment 352518 [details] [review] v4l2pool: Fix wrong error message
Created attachment 352519 [details] [review] v4l2videodec: Remove unused forward declaration
Created attachment 352520 [details] [review] v4l2: Add Video Encoder support This implements H264 encoding support using generic V4L2 interface. It is reported to work with Samsung MFC driver, IXM.6 CODA driver and Qualcomm mainline Venus driver. Other platform should be supported as none of this work is platform specific. The implementation consist of a GstV4l2VideoEnc base class, which implements the core streaming functionality. This base class is implemented by GstV4l2H264Enc class that implements the caps negotiation specific to H264 profiles and level. This implementation supports hardware with multiple H264 encoder. Though, to make it simplier to use, the first discovered H264 encoder will be named v4l2h264enc. Other encoder found during discovery will have a unique name like v4l2video0h264enc. This work is the combined work of multiple developpers in the last 3 years. Thanks to all of the contributors: Ayaka <ayaka@soulik.info> Frédéric Sureau <frederic.sureau@vodalys.com> Jean-Michel Hautbois <jean-michel.hautbois@veo-labs.com> Nicolas Dufresne <nicolas.dufresne@collabora.com> Pablo Anton <pablo.anton@vodalys-labs.com>
Created attachment 352521 [details] [review] v4l2: Don't redefine __bitwise if already set
I know this might not all been perfect, but the recent cleanup fixes the issues that prevented this patchset from being merge during the last 3 years. I will add more codecs in the next few weeks, and introduce properties to control various aspect of the encoder. I do think that having this in mainline will help further coordinate effort. I am closing this bug now and would like any further issues to be filed under it's own bug. Thanks to everyone who have contributed to this element. Attachment 352517 [details] pushed as 342de49 - v4l2: increase pre-allocated encoded buffer size Attachment 352518 [details] pushed as 17e52f6 - v4l2pool: Fix wrong error message Attachment 352519 [details] pushed as 7dc57e3 - v4l2videodec: Remove unused forward declaration Attachment 352520 [details] pushed as 2731036 - v4l2: Add Video Encoder support Attachment 352521 [details] pushed as 9f936c9 - v4l2: Don't redefine __bitwise if already set