[Bug 733405] Wrong channel mask in wav should be ignored

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=733405
  GStreamer | gst-plugins-base | 1.3.91

Nicolas Dufresne <nicolas.dufresne> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #281186|none                        |reviewed
             status|                            |

--- Comment #3 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2014-08-28 16:11:44 UTC ---
Review of attachment 281186:
 --> (https://bugzilla.gnome.org/review?bug=733405&attachment=281186)

This code could benefit some unit test, this is a behaviour change (e.g. this
code can hide existing layout information).

::: gst-libs/gst/riff/riff-media.c
@@ +1036,3 @@
+    gst_caps_set_simple (caps, "channel-mask", GST_TYPE_BITMASK, 0, NULL);
+    return TRUE;
+  }

Ok, this seems to match GST_AUDIO_CHANNEL_POSITION_NONE definition. Maybe a
better warning/comment would help direct people to the doc.

@@ +1045,3 @@
+            "are channels! Setting channel-mask to 0.");
+        channel_mask = 0;
+        break;

I'm not sure about this one. This would mean you tolerate a file with let's say
N channels having a position set, and M additional channels without, and expose
it without any layout information. Is that really wanted ?

I'm tend to say it's find to accept multi-channels file without layout
information, but when it contains partial layout information, dropping it does
not sound right to me.

@@ +1057,3 @@
+        "supposed to be %d channels! Setting channel-mask to 0.",
+        p, num_channels);
+    channel_mask = 0;

Isn't this check somewhat redundant (warning twice if p > num_chanels). This
code would gain a little cleanup. Though, same comment apply, shall we reject
the layout information completely ?

-- 
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.