After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 720031 - omxh264enc: key frame interval missing
omxh264enc: key frame interval missing
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
1.2.1
Other Linux
: Normal enhancement
: 1.2.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-07 17:01 UTC by m][sko
Modified: 2014-07-23 08:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add sps pps switch for rpi and add idr frames interval + fix h264 level parsing problem (9.62 KB, patch)
2014-03-12 20:57 UTC, m][sko
needs-work Details | Review
only IDR interval , SPS PPS on rpi related (9.07 KB, patch)
2014-03-15 21:32 UTC, m][sko
none Details | Review
only IDR interval , SPS PPS on rpi related (8.96 KB, patch)
2014-03-15 21:41 UTC, m][sko
needs-work Details | Review
level caps problem (502.53 KB, application/octet-stream)
2014-03-15 21:51 UTC, m][sko
  Details
IDR interval , SPS PPS on rpi related (9.38 KB, patch)
2014-03-16 16:21 UTC, m][sko
committed Details | Review

Description m][sko 2013-12-07 17:01:19 UTC
current omxh264enc missing keyframe intrval options

http://pastebin.com/XD6KBvwr

this patch can help but as I don't know how to add property in gobject model somebody will need to do it :)
Comment 1 Nicolas Dufresne (ndufresne) 2013-12-08 16:26:07 UTC
You need to add the property definition in _class_init(), have a look at x264enc code in -ugly.

It is called max-key-interval.

max-key-interval    : Maximum number of frames between two keyframes (< 0 is in sec)
                        flags: readable, writable
                        Integer. Range: -100 - 2147483647 Default: 0

Which means -1 -2, would mean every 1, 2 seconds. Though the documentation does not indicated the unit for values higher then zero, please verify this to ensure we are concistent. LibAV encoder also have this same options, same definition.
Comment 2 m][sko 2014-03-12 20:57:51 UTC
Created attachment 271652 [details] [review]
add sps pps switch for rpi and add idr frames interval + fix h264 level parsing problem

1.sps pps switch for rpi(as rpi don't generate sps pss default for every IDR) add 2.idr frames interval
3. fix h264 level parsing problem
Comment 3 Sebastian Dröge (slomo) 2014-03-15 12:26:41 UTC
Review of attachment 271652 [details] [review]:

::: gst-omx_old/omx/gstomxh264enc.c
@@ +59,3 @@
+/**< reference: OMX_CONFIG_PORTBOOLEANTYPE*/
+#define OMX_IndexParamBrcmVideoAVCInlineHeaderEnable  0x7f0000df
+#define OMX_IndexParamBrcmPixelAspectRatio        0x7f00004d

Isn't this defined by OMX_Broadcom.h?

@@ +208,3 @@
+                                          &config_inline_header);
+  if (err != OMX_ErrorNone) {
+    GST_ERROR_OBJECT (self, "can't get OMX_IndexParamBrcmVideoAVCInlineHeaderEnable %s (0x%08x)", gst_omx_error_to_string (err), err);

Please run all this through gst-indent :)

@@ +311,3 @@
     }
+
+    if (gst_structure_get_double(s, "level", &leveldouble)) {

"level" is always a string field, not an integer or double.

@@ +312,3 @@
+
+    if (gst_structure_get_double(s, "level", &leveldouble)) {
+      level_string = g_strdup_printf("%0.1lf", leveldouble);

Please put this into a separate commit
Comment 4 m][sko 2014-03-15 21:32:07 UTC
Created attachment 272035 [details] [review]
only IDR interval , SPS PPS on rpi related
Comment 5 m][sko 2014-03-15 21:41:44 UTC
Created attachment 272039 [details] [review]
only IDR interval , SPS PPS on rpi related
Comment 6 m][sko 2014-03-15 21:51:41 UTC
Created attachment 272040 [details]
level caps problem
Comment 7 m][sko 2014-03-15 22:00:54 UTC
I clean up code with gst tool
OMX_IndexParamBrcmVideoAVCInlineHeaderEnable
is from OMX_Index.h
but from Broadcom version

I removed problem with level in caps.
But it is still there
level_string = gst_structure_get_string (s, "level");

gst_structure_get_string simply don't work on strings like "4.1", "1.1", ...
but works fine for "4"
I don't understand as it shouldn't be LOCALES related

And If you really add level="4" all go wrong 
look for this problem in log file in attachments
I added string "all done" 
at the end of 
gst_omx_h264_enc_set_format 
return TRUE;

it looks like problem is related to error
"pushing all sticky events"

this is example how I start my pipeline

GST_DEBUG="*:3,queue2:0,GST_REGISTRY:0,audiodecoder:0,mpegaudioparse:0,audioencoder:0,GST_MEMORY:0,libav:0" \
gst-launch-1.0  uridecodebin uri="file:///home/pi/test2/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 \
 ! omxh264enc target-bitrate=2000000 control-rate=1 inline-header=true periodicty-idr=250 interval-intraframes=250 ! video/x-h264, profile="high", level="4" \
 ! h264parse \
 ! mpegtsmux name=mux0 \
 ! multifilesink next-file="key-frame" location="/dev/shm/l%010d.ts" sync=true \
 demux. ! audio/x-raw \
 ! avenc_libfdk_aac bitrate=65536 ! aacparse ! audio/mpeg, stream-format="adts" ! queue2 ! mux0. \
 2>&1
Comment 8 Sebastian Dröge (slomo) 2014-03-16 10:24:51 UTC
Make sure to "cast" the level if you provide the caps as string, e.g. use:
level=(string)4.1

Does it work for you then?
Comment 9 Sebastian Dröge (slomo) 2014-03-16 10:27:47 UTC
Comment on attachment 272039 [details] [review]
only IDR interval , SPS PPS on rpi related 

Looks good but can you provide it as "git format-patch" style patch, including your real name and a mail address?
Comment 10 m][sko 2014-03-16 16:21:34 UTC
Created attachment 272072 [details] [review]
IDR interval , SPS PPS on rpi related
Comment 11 Sebastian Dröge (slomo) 2014-03-16 16:32:46 UTC
commit 922d036ae7ea1d6d6b0a193b8930ce5972ee1b68
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Sun Mar 16 17:32:05 2014 +0100

    omxh264enc: Fix compiler warnings

commit e55bf0a4c5d5936cd45c377d3369721bd140ce58
Author: Michal Lazo <xlazom00@gmail.com>
Date:   Sun Mar 16 17:19:08 2014 +0100

    omxh264enc: IDR interval, SPS and PPS headers for rpi
    
    https://bugzilla.gnome.org/show_bug.cgi?id=720031
Comment 12 m][sko 2014-03-16 16:34:16 UTC
git patch in attachments

level related

none of this work
1. ! video/x-h264, profile="high", level="4.1" !
2. ! video/x-h264, profile="high", level=(string)4.1 !
3. ! video/x-h264, profile="high", level=(string)"4.1" !

only this one
4. ! 'video/x-h264, profile=high, level=(string)4.1' !

I don't know anything about pipeline syntax 
but 1. option would be nice for gst-launch prototyping
and syntax is similar to elements parameters