[Bug 739333] gst-omx leaks a huge memory when it works with amr h/w decoder

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=739333
  GStreamer | gst-omx | 1.2.4

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #290128|none                        |needs-work
             status|                            |

--- Comment #10 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-11-07 09:24:18 UTC ---
Review of attachment 290128:
 --> (https://bugzilla.gnome.org/review?bug=739333&attachment=290128)

Thanks for the patch, looks good :) Just some minor comments

::: omx/gstomxamrdec.c
@@ +1,2 @@
+/*
+ * Copyright (C) 2014, Sebastian Dröge <sebastian at centricular.com>

You probably want to add your or your company's name here as a second line :)

@@ +64,3 @@
+  audiodec_class->cdata.default_sink_template_caps = "audio/AMR, "
+      "rate=(int)[8000,48000], "
+      "channels=(int)[1,9]";

It's audio/AMR for narrow-band and audio/AMR-WB for wide-band. The narrow-band
only supports 8000Hz, wide-band only supports 16000Hz

@@ +79,3 @@
+{
+  /* FIXME: Other values exist too! */
+  self->spf = -1;

Remove the FIXME comment here, it is wrong

@@ +131,3 @@
+
+  amr_param.nChannels = channels;
+  amr_param.nBitRate = rate;       /* unknown */

Not really unknown, isn't it? :)

@@ +197,3 @@
+static gboolean
+gst_omx_amr_dec_get_channel_positions (GstOMXAudioDec * dec,
+    GstOMXPort * port, GstAudioChannelPosition
position[OMX_AUDIO_MAXCHANNELS])

AMR only supports mono, so add assertions (g_return_val_if_fail
(pcm_param.nChannels == 1, GST_AUDIO_CHANNEL_POSITION_MONO))  for all the other
cases here and always return MONO

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.