[Bug 773073] audioconvert: endian conversion optimization

[GStreamer (GNOME Bugzilla)]

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

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

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

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

Generally looks good, thanks :) Just some minor remarks below

::: gst-libs/gst/audio/audio-converter.c
@@ +863,3 @@
+ */
+static void
+converter_swap_endian_24 (gpointer dst, const gpointer src, gint count)

Worries me a bit that dst and src can alias (in place transform), a compiler
might optimize the code below to something broken in theory: it could get "x"
after writing to in[i] for optimization reasons under the assumption that they
don't alias

Maybe we need a version for in-place and not?

@@ +1002,3 @@
 }

+#define GST_AUDIO_FORMAT_IS_ENDIAN_CONVERSION(info1, info2) \

And next step, sign conversion \o/ :)

@@ +1080,3 @@
+            ("same formats, no resampler and passthrough mixing ->
passthrough");
+        convert->convert = converter_passthrough;
+        /* TODO: implement also in-place conversion */

Isn't this (converter_passthrough) never called anyway as basetransform will
work in passthrough mode then? And converter_passthrough could just check for
in==out and return then

@@ +1116,3 @@
+          default:
+            GST_ERROR ("unsupported sample width for endian conversion");
+            g_assert (0);

g_assert_not_reached()

::: gst/audioconvert/gstaudioconvert.c
@@ +791,3 @@
+gst_audio_convert_transform_ip (GstBaseTransform * base, GstBuffer * buf)
+{
+  return gst_audio_convert_transform (base, buf, buf);

Maybe needs a g_assert() for gst_audio_converter_supports_inplace() to return
TRUE :)

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