Motown in-proc streaming framework used to implement media services.

services/media/framework/ostream.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SERVICES_MEDIA_FRAMEWORK_OSTREAM_H_
+#define SERVICES_MEDIA_FRAMEWORK_OSTREAM_H_
+
+#include <ostream>
+
+#include "services/media/framework/models/demand.h"
+#include "services/media/framework/packet.h"
+#include "services/media/framework/result.h"
+#include "services/media/framework/stream_type.h"
+
+//
+// This file declares a bunch of << operator overloads for dumping media stuff.
+// Unless you want to add new operators, it's sufficient to know that you can
+// just use the operators as expected, except that some of the overloads can
+// produce multiple lines and therefore provide their own newlines.
+//
+// These operators are intended to be called after a label has been added to
+// the stream with a trailing space. If the text generated by an operator is
+// sufficiently short, the operator may add that text with no preamble and
+// terminate it with std::endl. If the text has to be multiline, the operator
+// first adds std::endl, then the multiline text with std::endl termination.
+// Each line starts with begl in order to apply the appropriate indentation.
+// The Indenter class is provided to adjust the identation level. Operators
+// that take pointers need to handle nullptr.
+//
+
+namespace mojo {
+namespace media {
+
+class Indenter {
+ public:
+  static std::string by;

[johngro, 2016/01/26 01:32:39]

Indenter::by could be constexpr.  At the very least, it should be const (which
means that it should be named BY, instead of by).

Re: level; this is not thread safe, but I assume that you know this.  This needs
to be thread local, not static global, in order to be thread safe.  I don't know
our policy on the C++11 threadlocal keyword and if it is permitted or not.

A hack which would keep the counts consistent (by the indenting sometimes
incorrect) would be to use std::atomic to manipulate the level in the
constructor/destructors, instead of ++/--

Finally, level should probably be private with a static accessor, and suffixed
with _.  The only way to manipulate the value level should be via the
constructor/destructor.

[dalesat, 2016/01/26 21:17:51]

This stuff is for debugging. I'd rather leave it out of the CL than spend a lot
of time getting it perfect, though copying it around from branch to branch would
be a hassle. I messed with it for awhile without great results. I switched to
using custom stream manipulators, which is probably a better pattern, but that
doesn't really address your concerns.

[johngro, 2016/01/27 22:35:22]

So, I don't really understand xalloc/iword/pword super well (I'm not a fan of
ostreams) and I needed to do some reading, but I think you are correct that this
does not address my original concerns.  If I am understanding correctly, xalloc
will create a unique index which can be used with the iword member of any
ostream instance to retrieve a long integer (initialized to zero) which is
unique to that ostream instance.

If I am understanding the base::log ostreams correctly, that means that we have
implemented an indentation level per log stream.  The thread safety concerns
remain.  Also, xalloc is apparently not thread safe until C++14 (and we are
currently on C++11).

I agree that this is probably a moot point, as I don't know what level of thread
safety exists in the base or mojo log streams (we are likely to get either
interleaved characters or lines anyway).

That said, I checked with James regarding thread local storage.  What he told me
was...

1) Apple prohibits the thread_local keyword (introduced in C++11), but google
and chrome toolchains support it.  We allow its use provided your code does not
need to run in an apple environment.  For our purposed, this means that if you
do not expect this code to be linked against Flutter, you can use thread_local.
2) If you want to be double sure, you can use base::ThreadLocalPointer; this
should work on all platforms.

So, your original code would be perfectly fine and thread safe (no atomics
needed) by just saying

static thread_local int level;

instead (provided you didn't need to link against flutter).  A small amount of
more work would be needed to make the base::ThreadLocalPointer class work, but
from glancing at the code, it seems like this would be pretty easy to manage
either manually or with a small templated wrapper class (so you could say
something like ThreadLocal<uint32_t> level(0))

It's fine if you want to leave it like this, just trying to give some options.

[dalesat, 2016/01/28 18:49:15]

Thanks for digging in.

+  static int level;
+  Indenter() { level++; }
+  ~Indenter() { level--; }
+};
+
+template<class charT, class traits> std::basic_ostream<charT, traits>&
+    begl(std::basic_ostream<charT, traits>& os) {
+  for (int i = 0; i < Indenter::level; i++) {
+    os << Indenter::by;
+  }
+  return os;
+}
+
+// The following overloads don't add newlines.
+
+std::ostream& operator<<(std::ostream& os, Result value);
+std::ostream& operator<<(std::ostream& os, Demand value);
+std::ostream& operator<<(std::ostream& os, const PacketPtr& value);
+std::ostream& operator<<(std::ostream& os, StreamType::Scheme value);
+std::ostream& operator<<(std::ostream& os, LpcmStreamType::SampleFormat value);
+std::ostream& operator<<(
+    std::ostream& os,
+    CompressedAudioStreamType::AudioEncoding value);
+std::ostream& operator<<(
+    std::ostream& os,
+    VideoStreamType::VideoEncoding value);
+std::ostream& operator<<(std::ostream& os, VideoStreamType::VideoProfile value);
+std::ostream& operator<<(std::ostream& os, VideoStreamType::PixelFormat value);
+std::ostream& operator<<(std::ostream& os, VideoStreamType::ColorSpace value);
+std::ostream& operator<<(std::ostream& os, const BytesPtr& value);
+
+template<typename T>
+std::ostream& operator<<(std::ostream& os, Range<T> value) {
+  return os << value.min << ".." << value.max;
+}
+
+std::ostream& operator<<(std::ostream& os, Range<bool> value);
+
+// The following overloads add newlines.
+
+std::ostream& operator<<(std::ostream& os, const StreamTypePtr& value);
+std::ostream& operator<<(std::ostream& os, const StreamTypeSetPtr& value);
+std::ostream& operator<<(std::ostream& os, const StreamTypesPtr& value);
+std::ostream& operator<<(std::ostream& os, const StreamTypeSetsPtr& value);
+
+} // namespace media
+} // namespace mojo
+
+#endif // SERVICES_MEDIA_FRAMEWORK_OSTREAM_H_