Motown: ffmpeg implementations of framework 'parts'

services/media/framework_ffmpeg/ffmpeg_type_converters.h

Index: services/media/framework_ffmpeg/ffmpeg_type_converters.h
diff --git a/services/media/framework_ffmpeg/ffmpeg_type_converters.h b/services/media/framework_ffmpeg/ffmpeg_type_converters.h
new file mode 100644
index 0000000000000000000000000000000000000000..f935f4e01ee21b7b2b8b63a14eb473334a190bb1
--- /dev/null
+++ b/services/media/framework_ffmpeg/ffmpeg_type_converters.h
@@ -0,0 +1,29 @@
+// 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_FFMPEG_FFMPEG_TYPE_CONVERTERS_H_
+#define SERVICES_MEDIA_FRAMEWORK_FFMPEG_FFMPEG_TYPE_CONVERTERS_H_
+
+#include "services/media/framework/stream_type.h"
+extern "C" {
+#include "third_party/ffmpeg/libavformat/avformat.h"
+}
+
+namespace mojo {
+namespace media {
+
+// We don't specialize TypeConverter here, because specializations have to
+// occur in the same namespace as the original template (mojo::, in this case).
+
+// Creates a MediaType from an AVCodecContext.
+std::unique_ptr<StreamType> StreamTypeFromAVCodecContext(AVCodecContext* from);

[johngro, 2016/03/01 01:31:39]

can we make AVCodecContext be a const unique_ptr<> & here (and a unique_ptr<> in
the return from AVCodecContextFromStreamType below?)

Because of the custom deleter, it might be best to hoist the deleter functor out
of the FfmpegDecoderBase class and place it at a utility level.  Generally, I
think it would be best to put its name in an internal {} namespace and
typedef/using the pointer type so that people don't have to write things like
unique_ptr<AVCodecContext, internal::AVCodecContextDeleter>.

I know that Jeff disagrees somewhat with the typedef approach (argument; it
removes clarity about the underlying type of the pointer), but perhaps he would
be OK with it if it was something like

using AVCodecContextUPtr = ...;

or even

using AVCodecContextUniquePtr = ...;

My counter to Jeff's point would be that users of a unique pointer's class
should not know about details like custom deleters.  Also, if in attempting to
avoid the need for knowledge of things like custom deleters, we end up returning
unmanaged pointers for newly created object from factory functions (things like
AVCodecContextFromStreamType below), then we are defeating the purpose of using
the managed pointer classes in the first place.

[dalesat, 2016/03/01 20:43:02]

When a custom deleter is in play, I too prefer to define the pointer type.

Even if we're always managing Foo with a unique_ptr, I prefer passing const Foo&
to passing const std::unique_ptr<Foo>&.

[johngro, 2016/03/01 22:07:26]

Sure.

If the target function demands a non-null Foo pointer, then a const& is
definitely the way to go.  The burden is now on the caller to make sure that
their instances are never null before the "de-reference" them to pass into the
const& target.

If, based on the API, it is OK to pass null, then you end up needing a const
uptr<>&

[dalesat, 2016/03/02 18:35:36]

Acknowledged.

+
+// Creates an AVCodecContext from a StreamType.
+AVCodecContext* AVCodecContextFromStreamType(
+    const StreamType& stream_type);
+
+}  // namespace media
+}  // namespace mojo
+
+#endif // SERVICES_MEDIA_FRAMEWORK_FFMPEG_FFMPEG_TYPE_CONVERTERS_H_