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 652961 - [basevideoencoder] Set the 'force_keyframe' field to TRUE when a forced keyframe is required
[basevideoencoder] Set the 'force_keyframe' field to TRUE when a forced keyfr...
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-06-19 22:01 UTC by Andoni Morales
Modified: 2011-06-29 09:54 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
gst_base_video_encoder_finish_frame (1.05 KB, patch)
2011-06-19 22:01 UTC, Andoni Morales 		needs-work Details | Review

Description Andoni Morales 2011-06-19 22:01:19 UTC 
Created attachment 190232 [details] [review]
gst_base_video_encoder_finish_frame 

When a forced-keyframe is required, the frame should be be initialized with 'frame->force-keyframe=TRUE'. This way the GstForceKeyUnit event is then forwarded properly in gst_base_video_encoder_finish_frame()
Comment 1 David Schleef 2011-06-25 18:33:53 UTC 
Looks good, push it.
Comment 2 Sebastian Dröge (slomo) 2011-06-26 12:28:52 UTC 
Review of attachment 190232 [details] [review]:

::: gst-libs/gst/video/gstbasevideoencoder.c
@@ +431,3 @@
+  if (base_video_encoder->force_keyframe) {
+    frame->force_keyframe = TRUE;
+    base_video_encoder->force_keyframe = FALSE;

This should be protected with the object lock (GST_OBJECT_LOCK/UNLOCK)
Comment 3 Andoni Morales 2011-06-27 08:16:36 UTC 
Others variables are also modified in the same function without the lock, as well as in finish_frame or set_latency. Shouldn't they be protected with the object lock too?
In encoder_chain, should the lock be extended to handle_frame too so that subclasses doesn't need to worry about too?
Comment 4 Andoni Morales 2011-06-29 09:54:55 UTC 
Ths patch is now obsolete after this commit:
http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=16c6a49bd44e2389162ac3d67d7fd24f6f43df43