GNOME Bugzilla – Bug 732533
omxvideoenc: doesn't recover after aspect ratio change on RPI
Last modified: 2016-07-06 09:14:52 UTC
omxvideoenc don't recover after aspect ratio change on RPI test file http://www.mdragon.org/test_ar_problem2.ts GST_DEBUG="*:1,multifilesink:0,omxvideoenc:0,h264parse:4,mpegtsmux:0,omxvideodec:0" \ gst-launch-1.0 -v uridecodebin uri="http://www.mdragon.org/test_ar_problem2.ts" use-buffering=false name=demux \ ! "video/x-raw, format=I420" \ ! omxh264enc target-bitrate=1000000 control-rate=1 inline-header=true periodicty-idr=250 interval-intraframes=250 ! "video/x-h264, profile=high" \ ! h264parse ! "video/x-h264, alignment=au" \ ! mpegtsmux name=mux0 \ ! filesink location="test_ar_problem2_out.ts" sync=true \ demux. ! "audio/x-raw" ! fakesink \ 2>&1 pipeline simple hangup
Can you provide a debug log with GST_DEBUG=omx*:6 and also a backtrace of all threads when it hangs?
http://live.mdragon.org/debug.log Application don't crash omxvideoenc simple stop after aspect ratio change
Thanks
Created attachment 280015 [details] [review] omxenc reconfigure caps patch I fixed flushing problem but it still don't work on RPI(I don't have any other device) without no-component-reconfigure I rewrote some code base on decoder (flushing, draining,..)
anybody can look at my patch?
(In reply to comment #4) > Created an attachment (id=280015) [details] [review] > omxenc reconfigure caps patch > > I fixed flushing problem > but it still don't work on RPI(I don't have any other device) without > no-component-reconfigure > > I rewrote some code base on decoder (flushing, draining,..) You mean the issue can be fixed with no-component-reconfigure? Thanks.
Created attachment 284574 [details] [review] Patch to fix this problem on RPI I guess the OpenMAX implementation for RPI has some issue in such kind of special case. Actually I was not able to find clear explanation in OpenMAX specification about the component reconfiguration. My patch can be used to fix this problem, by the way.
Review of attachment 284574 [details] [review]: See how this is done in gstomxvideodec.c. Look for GST_OMX_HACK_NO_COMPONENT_RECONFIGURE It should be done optionally via that hack flag, and ideally with the same code as there. ::: omx/gstomxvideoenc.c @@ +1100,3 @@ return FALSE; + if (gst_omx_component_set_state (self->enc, OMX_StateIdle) != OMX_ErrorNone) + return FALSE; Why do you only go to Idle here and not Loaded too?
Comment on attachment 280015 [details] [review] omxenc reconfigure caps patch Is this patch still necessary when using the other one? If so it probably needs to be ported to latest git master... and also needs a better commit message that actually describes what the patch is trying to do
(In reply to comment #8) > Review of attachment 284574 [details] [review]: > > See how this is done in gstomxvideodec.c. Look for > GST_OMX_HACK_NO_COMPONENT_RECONFIGURE > > It should be done optionally via that hack flag, and ideally with the same code > as there. > Oh, I see. In that way, we need to change gstomx.conf for RPI to enable the hack for video encoder, right? Thanks.
Yes, it needs to be added to the config but first of all the code for it has to be added to gstomxvidenc.c :)
Created attachment 284631 [details] [review] Implement the hack flag GST_OMX_HACK_NO_COMPONENT_RECONFIGURE support for video encoder Thanks Sebastian! I updated the patch based on your comment and it can fix the problem in my test.
now we need to add support for this http://home.nouwen.name/RaspberryPi/documentation/ilcomponents/prop.html#OMX_IndexParamBrcmPixelAspectRatio Allowed values are: 1:1, 10:11, 16:11, 40:33, 59:54, and 118:81. ouch :( I don't really understand why broadcom don't support full range. Is it really that hard to generate proper SPS.
Created attachment 284643 [details] [review] rpi setup video encoder aspect ratio rpi setup video encoder aspect ratio as rpi encoder don't support all aspect ratios http://home.nouwen.name/RaspberryPi/documentation/ilcomponents/prop.html#OMX_IndexParamBrcmPixelAspectRatio I use closest value.
after aspect ratio change I get this: http://pastebin.com/UDcxixUa many gst_h264_parse_reset_frame Any idea?
Comment on attachment 284631 [details] [review] Implement the hack flag GST_OMX_HACK_NO_COMPONENT_RECONFIGURE support for video encoder commit cdb918aefba276a0df70052948ff846deb128bf1 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Aug 28 10:44:31 2014 +0300 omxaudioenc: Implement the hack flag GST_OMX_HACK_NO_COMPONENT_RECONFIGURE commit 2c98a6ab7ec2e67c6838ea84175eec50c4b02045 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Aug 28 10:43:22 2014 +0300 omxaudioenc: Use the base class' open/close vfuncs instead of calling them ourselves commit d3d0a82ba4d6dbb475759f895f5a5a41247bdc1c Author: Peng Liu <pengliu.mail@gmail.com> Date: Tue Aug 26 22:13:53 2014 -0500 omxvideoenc: Implement the hack flag GST_OMX_HACK_NO_COMPONENT_RECONFIGURE Fix a video encoder stall problem on RPi when changing the aspect ratio. https://bugzilla.gnome.org/show_bug.cgi?id=732533
Review of attachment 284643 [details] [review]: ::: omx/gstomxvideoenc.c @@ +931,3 @@ + guint par_n; + guint par_d; +} PAR; Just use OMX_CONFIG_POINTTYPE here @@ +933,3 @@ +} PAR; + +static PAR broadcom_supported_aspect_ratios[] = { Make the table const @@ +954,3 @@ + par = &broadcom_supported_aspect_ratios[0]; + best_par = par; + best_ar_diff = Instead of choosing the closest value, just put the only allowed values in the sinkpad template caps. You'll distort the frames if you choose a value different from the actual one, no matter how close it is @@ +1112,3 @@ +#ifdef USE_OMX_TARGET_RPI + // aspect ratio No C99 comments
broadcom don't support any other aspect ratios 1:1, 10:11, 16:11, 40:33, 59:54, and 118:81. If I don't choose one of supported, broadcom choose 1:1 and that isn't good at all. I prefer my solution. As plan B I have SPS generator base on current h264parser (1.2.3 version) but I don't know If anybody will maintain it. broadcom openmax encoder also don't generate AU delimiters and I think we should add it here to encoder instead of h264parse
(In reply to comment #18) > broadcom don't support any other aspect ratios > 1:1, 10:11, 16:11, 40:33, 59:54, and 118:81. > > If I don't choose one of supported, broadcom choose 1:1 and that isn't good at > all. > I prefer my solution. That's also not what I said. You would put those 6 PAR into the sinkpad template caps, and then do an exact match for setting the broadcom specific property. If no exact match is found it is considered an error instead of using the closest one (which would distort the picture).
(In reply to comment #19) > (In reply to comment #18) > > broadcom don't support any other aspect ratios > > 1:1, 10:11, 16:11, 40:33, 59:54, and 118:81. > > > > If I don't choose one of supported, broadcom choose 1:1 and that isn't good at > > all. > > I prefer my solution. > > That's also not what I said. You would put those 6 PAR into the sinkpad > template caps, and then do an exact match for setting the broadcom specific > property. If no exact match is found it is considered an error instead of using > the closest one (which would distort the picture). And benefit of you idea? encoder will not work in same unsupported aspect ratios?
Yes, it will not work with unsupported aspect ratios instead of claiming to work with them and then using a completely different one. That allows upstream elements to generate data with an appropriate aspect ratio, e.g. videoscale can do that.
What if we add property for this dilemma? aproximate_aspectratio_in_SPS Can we change sink caps dynamically or base on change in property?
What would this solve?
working encoder with approximate aspect ratio if you switch it on in all other cases you will need to handle it with videoscale
plan C I'am in contact with don from RPI. He has access to source codes and we will try to add support for all ARs. I hope it will be eazy
Plan C sounds best of course, but until then I would still prefer my solution. Just pretending to accept a PAR and using a completely different one is not a good idea, you produce broken videos.
Created attachment 284736 [details] demug log I still have problem with this http://pastebin.com/UDcxixUa after aspect ratio change h264 parser generate many gst_h264_parse_reset_frame If I add h264parse disable-passthrought=true all works fine GST_DEBUG="*:3,aacparse:3,tsmux:0,mpegtsmux:4,omx*:1,omxvideoenc:1,omxh264enc:1,h264parse:5" \ gst-launch-1.0 uridecodebin uri="file:///home/pi/test13_aspect_ratio_change/test_ar_problem2.ts" use-buffering=false name=demux caps="video/mpeg, parsed=true; audio/x-raw; text/x-raw" \ ! 'video/mpeg, parsed=true' \ ! omxmpeg2videodec ! tee name="decdata"\ ! omxh264enc target-bitrate=1000000 control-rate=1 periodicty-idr=50 interval-intraframes=50 inline-header=true ! 'video/x-h264, profile=high' \ ! h264parse disable-passthrough=true \ ! fakesink \ demux. ! 'audio/x-raw' \ ! fakesink \ 2>&1
http://pastebin.com/wPdRhezj 0:00:09.248553095 29707 0xb06cc780 INFO baseparse gstbaseparse.c:3583:gst_base_parse_set_passthrough:<h264parse0> passthrough: yes and whole parsing process stoped
Can you provide a sample stream for that and create another bug against h264parse for it?
Same goes for the RPi specific aspect ratio configuration, it's independent of this bug here.
(In reply to comment #29) > Can you provide a sample stream for that and create another bug against > h264parse for it? in my first post http://www.mdragon.org/test_ar_problem2.ts
bug report https://bugzilla.gnome.org/show_bug.cgi?id=735655
Created attachment 284956 [details] [review] rpi setup aspect ratio dan add support for all aspect rations in latest rpi firmware So if anybody want to use this patch on all aspect rations you will need firmware since 30.Aug.2014
commit bfeab29a398455d8c89af6d1bbd80817c5e3ad0e Author: Michal Lazo <michal.lazo@mdragon.org> Date: Sun Aug 31 20:30:13 2014 +0000 omxvideoenc: Setup aspect ratio on RPi Needs firmware from yesterday or newer to work with all possible aspect ratios. Before that it only supported a fixed list. https://bugzilla.gnome.org/show_bug.cgi?id=732533