[Bug 761588] opusdec: no mono/stereo channel conversion

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=761588

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #26 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 322389:
 --> (https://bugzilla.gnome.org/review?bug=761588&attachment=322389)

And only a change in opusdec this time \o/

::: ext/opus/gstopusdec.c
@@ +261,3 @@
+          }
+        }
+      }

This might need an else case where we explicitly check for 48000 only and
ignore the structure otherwise we might select below a channel setup that is
not going to work

@@ +279,3 @@
+          }
+        }
+      }

And also an else case here I guess? What is if n_channels==0? What would we do
then later? Stereo? If so we should check for stereo here

@@ +284,3 @@
+      dec->n_channels = channels;
+      break;
+    }

All this loop might be possible to do easier with gst_caps_intersect() btw. You
could create new caps with only rate (if ->sample_rate != 0 then {x, 48000}
else 48000) and channels (if ->n_channels ...). And then just check the result
of the intersection with that. Or am I missing something?

@@ +884,3 @@
+    gst_caps_unref (filter);
+  if (caps) {
+    caps = gst_caps_copy (caps);

caps = gst_caps_make_writable(caps);

(you leak the original caps here and also make unnecessary copies most of the
time)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.