Moved codec-specific audio packet splitting into decoders.

webrtc/modules/audio_coding/codecs/split_by_samples.h

Index: webrtc/modules/audio_coding/codecs/split_by_samples.h
diff --git a/webrtc/modules/audio_coding/codecs/split_by_samples.h b/webrtc/modules/audio_coding/codecs/split_by_samples.h
new file mode 100644
index 0000000000000000000000000000000000000000..6b0ac15fb3eabcfe734b542d871250afaf19df7e
--- /dev/null
+++ b/webrtc/modules/audio_coding/codecs/split_by_samples.h
@@ -0,0 +1,37 @@
+/*
+ *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_MODULES_AUDIO_CODING_CODECS_SPLIT_BY_SAMPLES_H_
+#define WEBRTC_MODULES_AUDIO_CODING_CODECS_SPLIT_BY_SAMPLES_H_
+
+#include <vector>
+
+#include "webrtc/base/array_view.h"
+#include "webrtc/modules/audio_coding/codecs/audio_decoder.h"
+
+namespace webrtc {
+
+namespace internal {

[kwiberg-webrtc, 2016/09/12 02:11:02]

Why put this in the "internal" namespace? It isn't exposed in public headers (is
it?), and we're not in the habit of putting non-public things in general in this
namespace.

[ossu, 2016/09/12 11:26:37]

Hmm, that's true. I wanted to make sure this was non-public, so that it didn't
get relied on by external parties. I want it to be used by our existing decoders
for now, but to revisit and possibly get rid of it completely once the
restructuring is gone. I can live with having it in webrtc directly, though.

[kwiberg-webrtc, 2016/09/13 12:23:37]

Ack.

Hmmm. I wonder if it would be more useful than inconvenient to put *everything*
that isn't intended to be public in a namespace called "internal" or "impl" or
something. If I'm not mistaken, we wouldn't have to actually write internal:: in
all that many places.

Of course, we should probably make sure we have only one top-level namespace
before doing that.

+
+// This function is experimental and should not be relied upon by external
+// code. As with the SplitAudio method that utilizes it, it will likely be
+// removed in the near future.
+// Splits a payload consisting of inidividual samples into reasonable chunks.
+// Utility function for use by sample-based codecs to implement SplitPacket.
+std::vector<AudioDecoder::PacketSplit> SplitBySamples(
+    rtc::ArrayView<const uint8_t> payload,
+    size_t bytes_per_ms,
+    uint32_t timestamps_per_ms);

[kwiberg-webrtc, 2016/09/12 02:11:02]

Consider having an output argument instead of a return value to enable reuse of
the heap allocation for the vector.

[ossu, 2016/09/12 11:26:37]

I don't like using output arguments in general, since it leads to more
convoluted code, both for the caller and the implementation. I don't believe
this is a case where an allocation is going to be costly (it'll be a short
vector of small objects), so it should be handled quite efficiently. If it turns
out that SplitBySamples is actually much more costly due to this, then I'd
reconsider.

To get around the heap allocation here, you'd not only need to push the
allocation of the object to the caller, but it would have to retain that object
over time as some sort of working buffer. I'd rather we look at getting a
vector-type container with small-size optimization in WebRTC. That would allow
us to use more natural syntax without paying the overhead of heap allocations
for small sets of data like these.

[kwiberg-webrtc, 2016/09/13 12:23:37]

True.
I think it's doing an allocation at all that's the issue, not how big it is. The
heap metadata has to be fetched, searched, and updated, and we probably have to
take locks too.
Yes.
I don't really agree. Small-size optimizations are nice in that they can often
speed things up, but since the cutoff size isn't something you can rely on, the
optimization may fail to apply in any specific case. The only thing I know of
that solves the problem is some sort of pool, whether at the level of objects
(as in my recent proof-of-concept CL) or raw memory (such as the Allocator
argument to standard library containers).

An output argument is for all intents and purposes a simple object pool of size
1, whose lifetime is managed by the caller.

+
+}  // namespace internal
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_AUDIO_CODING_CODECS_SPLIT_BY_SAMPLES_H_