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 783829 - omxh264enc: 'interval-intraframes' property not working on the pi
omxh264enc: 'interval-intraframes' property not working on the pi
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
git master
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-15 14:37 UTC by Guillaume Desmottes
Modified: 2017-06-28 21:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
omxh264enc: factor out set_avc_intra_perdiod() (4.41 KB, patch)
2017-06-28 13:16 UTC, Guillaume Desmottes
committed Details | Review
omxh264enc: use OMX_IndexConfigBrcmVideoIntraPeriod on pi (2.75 KB, patch)
2017-06-28 13:16 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2017-06-15 14:37:11 UTC
The omxh264enc element has this property:

  interval-intraframes: Interval of coding Intra frames (0xffffffff=component default)

If set, it will override the OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames setting (section 4.3.27 in the spec, https://www.khronos.org/registry/OpenMAX-IL/specs/OpenMAX_IL_1_1_2_Specification.pdf ) which is defined as:
"nPFrames specifies coding of a frame as Intra (non-inclusive of the first frame)
after every nPFrames of Inter frames."
So the number of P frames between each I frame basically.

A couple of things aren't clear:

- Why is it setting OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames and not OMX_VIDEO_PARAM_AVCTYPE.nPFrames? This setting is defined in the spec (4.3.18) as:
"nPFrames is the number of P frames between I frames."
The OMX spec isn't very clear about the difference between these two settings actually. If someone knows more about it please let me know.

- This definition ("Interval of coding Intra frames") doesn't account for potential B frames. The interval between 2 I frame is actually nPFrames + nBFrames.

We'd need a gst setting to set nBFrames as well so we could make things clearer for the 'interval-intraframes' property in the meantime. But first we should clarify the OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames vs OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames distinction.
Comment 1 Nicolas Dufresne (ndufresne) 2017-06-15 17:50:08 UTC
(In reply to Guillaume Desmottes from comment #0)
> The omxh264enc element has this property:
> 
>   interval-intraframes: Interval of coding Intra frames
> (0xffffffff=component default)
> 
> If set, it will override the OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames
> setting (section 4.3.27 in the spec,
> https://www.khronos.org/registry/OpenMAX-IL/specs/
> OpenMAX_IL_1_1_2_Specification.pdf ) which is defined as:
> "nPFrames specifies coding of a frame as Intra (non-inclusive of the first
> frame)
> after every nPFrames of Inter frames."
> So the number of P frames between each I frame basically.

No, it says as Intra. Which means on how many P-Frames the keyframe will be encoded into. In intra-refresh, we remove key-frame and add some key line to each delta frames. This interval should control how many P-Frame a decoder will need before it gets a complete image when starting from scratch or after data lost.

> 
> A couple of things aren't clear:
> 
> - Why is it setting OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames and not
> OMX_VIDEO_PARAM_AVCTYPE.nPFrames? This setting is defined in the spec
> (4.3.18) as:

Because it's two completely different settings.

> "nPFrames is the number of P frames between I frames."
> The OMX spec isn't very clear about the difference between these two
> settings actually. If someone knows more about it please let me know.
> 
> - This definition ("Interval of coding Intra frames") doesn't account for
> potential B frames. The interval between 2 I frame is actually nPFrames +
> nBFrames.

This is quite clear to me, if you understand what Intra Frame coding is.

> 
> We'd need a gst setting to set nBFrames as well so we could make things
> clearer for the 'interval-intraframes' property in the meantime. But first
> we should clarify the OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames vs
> OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames distinction.

Done.
Comment 2 Nicolas Dufresne (ndufresne) 2017-06-15 17:50:34 UTC
Also, please use the mailing list for these questions.
Comment 3 Guillaume Desmottes 2017-06-16 09:31:00 UTC
(In reply to Nicolas Dufresne (stormer) from comment #1)
> (In reply to Guillaume Desmottes from comment #0)
> > The omxh264enc element has this property:
> > 
> >   interval-intraframes: Interval of coding Intra frames
> > (0xffffffff=component default)
> > 
> > If set, it will override the OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames
> > setting (section 4.3.27 in the spec,
> > https://www.khronos.org/registry/OpenMAX-IL/specs/
> > OpenMAX_IL_1_1_2_Specification.pdf ) which is defined as:
> > "nPFrames specifies coding of a frame as Intra (non-inclusive of the first
> > frame)
> > after every nPFrames of Inter frames."
> > So the number of P frames between each I frame basically.
> 
> No, it says as Intra. Which means on how many P-Frames the keyframe will be
> encoded into. In intra-refresh, we remove key-frame and add some key line to
> each delta frames. This interval should control how many P-Frame a decoder
> will need before it gets a complete image when starting from scratch or
> after data lost.

Where did you see that this setting was related to intra-refresh? The specs says:

"The OMX_VIDEO_CONFIG_AVCINTRAPERIOD structure is used to enable and
configure the IDR and Intra periodicity for the AVC encoder during an encoding session."

I don't see anything related to intra-refresh there.
That's the job of OMX_VIDEO_PARAM_INTRAREFRESHTYPE (and OMX_CONFIG_INTRAREFRESHVOPTYPE) which has settings to control intra-refresh parameters.
Comment 4 Nicolas Dufresne (ndufresne) 2017-06-16 12:51:33 UTC
Ok, then find what it actually do on RPi to find out what the expected semantic is. That is the ABI I would suppose.
Comment 5 Guillaume Desmottes 2017-06-27 11:13:12 UTC
I did some testing on the pi and the 'interval-intraframes' property on the omxh264enc element.
The default value of config_avcintraperiod.nPFrames is 60 which matches the observed GOP pattern. But changing it doesn't seem to have any influence, we always get 60 delta between key frames, regardless of its value.

I tried using OMX_VIDEO_PARAM_AVCTYPE.nPFrames as well and it doesn't seem to affect the GOP pattern either.
Changing OMX_VIDEO_PARAM_AVCTYPE.nBFrames doesn't seem to change it either.

Actually this property and those OMX settings have absolutely 0 impact. I tried various "videotestsrc ! omxh264enc ! filesink" pipelines with different settings and the generated file was always exactly the same.
Which is weird because this property seems to have been added for the pi: https://cgit.freedesktop.org/gstreamer/gst-omx/commit/?id=e55bf0a4c5d5936cd45c377d3369721bd140ce58

But what DOES work is the OMX_IndexConfigBrcmVideoIntraPeriod defined in the doc as "OMX_IndexConfigBrcmVideoIntraPeriod".

So my suggestion would to keep the 'interval-intraframes' property and allow platform specific implementation. We'd use OMX_IndexConfigBrcmVideoIntraPeriod on the pi.
I'm not sure for the default implementation:
- OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames (current) ?
- OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames ?
- both ?
- none (enable the property only on platform it's been tested) ?
Comment 6 Guillaume Desmottes 2017-06-27 12:55:45 UTC
(In reply to Guillaume Desmottes from comment #5)
> - OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames ?

I meant OMX_VIDEO_PARAM_AVCTYPE.nPFrames.
Comment 7 Nicolas Dufresne (ndufresne) 2017-06-27 13:16:04 UTC
Maybe you need to combine both "periodicty-idr" and "interval-intraframes", and maybe also the pps/sps thingy to get something to work. Those two properties looks so similar, what does that all mean ?
Comment 8 Guillaume Desmottes 2017-06-27 14:27:25 UTC
Disabling 'inline-header' doesn't seem to affect the GOP pattern.

'periodicty-idr' does indeed change the frequency of I frames, which makes sense as IDR frame are I frames.
But even when combining those 2 properties, changing OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames is a no-op.

So yeah I think we should just replace OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames by OMX_IndexConfigBrcmVideoIntraPeriod on the pi.
Comment 9 Nicolas Dufresne (ndufresne) 2017-06-27 14:34:38 UTC
Now I hope you understood why I thought that periodicty-idr was the distance between frame, and that interval-intraframes was for intra-refresh, otherwise we have two properties doing the same thing.
Comment 10 Guillaume Desmottes 2017-06-28 13:16:28 UTC
Created attachment 354621 [details] [review]
omxh264enc: factor out set_avc_intra_perdiod()
Comment 11 Guillaume Desmottes 2017-06-28 13:16:34 UTC
Created attachment 354622 [details] [review]
omxh264enc: use OMX_IndexConfigBrcmVideoIntraPeriod on pi

The OMX_VIDEO_CONFIG_AVCINTRAPERIOD.nPFrames setting isn't of any use on
the raspbery pi. Instead it uses a custom extension to define the I
frame period.
Comment 12 Guillaume Desmottes 2017-06-28 13:17:05 UTC
I kept the current behavior and just added the extra PI specific setting.
Comment 13 Nicolas Dufresne (ndufresne) 2017-06-28 21:14:06 UTC
Thanks, that should improve the experience on the Pi, I'm sure couple
of user have been hit by this in the past.

Attachment 354621 [details] pushed as 895086e - omxh264enc: factor out set_avc_intra_perdiod()
Attachment 354622 [details] pushed as 424776b - omxh264enc: use OMX_IndexConfigBrcmVideoIntraPeriod on pi