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 665003 - [0.11] GstAudioEncoder shouldn't keep adapter mapped while calling handle_frame
[0.11] GstAudioEncoder shouldn't keep adapter mapped while calling handle_frame
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
0.11.x
Other Mac OS
: Normal major
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-11-28 04:54 UTC by Matej Knopp
Modified: 2012-01-14 20:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.34 KB, patch)
2011-11-28 04:54 UTC, Matej Knopp
none Details | Review

Description Matej Knopp 2011-11-28 04:54:51 UTC
Created attachment 202269 [details] [review]
patch

handle_frame implementation can call gst_audio_encoder_finish_frame, which potentially clears the adapter making gst_adapter_unmap cause crash.


void
gst_adapter_unmap (GstAdapter * adapter)
{
  g_return_if_fail (GST_IS_ADAPTER (adapter));

  if (adapter->priv->cdata) {
    GstBuffer *cur = adapter->buflist->data;
    ^^^^^ - crashes because priv->cdata is not null on mapped buffer
                  while adapter->buflist is null - cleared in gst_audio_encoder_finish frame
                  which was called from handle_frame
  
    gst_buffer_unmap (cur, adapter->priv->cdata, adapter->priv->csize);
    adapter->priv->cdata = NULL;
  }
}
Comment 1 Vincent Penquerc'h 2011-11-28 18:07:34 UTC
See https://bugzilla.gnome.org/show_bug.cgi?id=664133 for another solution. If this is applied, 664133 should be closed.
Comment 2 Mark Nauwelaerts 2012-01-06 11:47:48 UTC
FWIW, the suggested fix here introduces another copy, which is a bit sub-optimal.

Even if that is not particular cared about, tend to favour fixing adapter probably to deal with such situation anyway even if the audiocodec classes would not do so.  That would also somewhat maintain 0.10 semantics where it was possible to do a _peek, and then a _flush which would invalidate the obtained pointer anyway, and same would then be with a _map followed by _flush (and _unmap, all fixed to handle this sequence preferably/obviously).
Comment 3 Matej Knopp 2012-01-13 22:56:14 UTC
Has the adapter been fixed? Is this bug still relevant?
Comment 4 Mark Nauwelaerts 2012-01-14 19:58:13 UTC
Adapter has been fixed in bug #644133, and IMO that renders this obsolete.
Comment 5 Matej Knopp 2012-01-14 20:00:23 UTC
Obsolete due #664133