Implement media::VideoDecodeAccelerator on Mac

base/mac/scoped_cftyperef.h

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.

[Ami GONE FROM CHROMIUM, 2012/05/17 21:44:47]

Copyright year needs updating.

[Ami GONE FROM CHROMIUM, 2012/05/17 21:44:47]

My comments on this file are purely FYI; you'll need an OWNERS review (Nico, I
guess).

[sail, 2012/05/23 23:02:29]

Done.

 // Use of this source code is governed by a BSD-style license that can be

[Ami GONE FROM CHROMIUM, 2012/05/17 21:44:47]

...BUT, that said, this change seems not worth it to me.
Without this change, the rest of your CL changes only in one line:
info.image.reset(image, base::mac::RETAIN);
becomes
info.image.reset(CFRetain(image));

ISTM the latter is actually better (again, knowing zilch about objc
idioms/conventions/style).

[sail, 2012/05/23 23:02:29]

The other thing I need here is the copy constructor and assignment operator.
This makes it easy to store the object in a map and vector.

 // found in the LICENSE file.
 
 #ifndef BASE_MAC_SCOPED_CFTYPEREF_H_
 #define BASE_MAC_SCOPED_CFTYPEREF_H_
 #pragma once
 
 #include <CoreFoundation/CoreFoundation.h>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 
 namespace base {
 namespace mac {
 
+enum OwnershipPolicy {

[Ami GONE FROM CHROMIUM, 2012/05/17 21:44:47]

Any reason not to live inside ScopedCFTypeRef?
(in the interest of keeping the namespace slightly less polluted)

[sail, 2012/05/23 23:02:29]

Moving this would make the calling code look like this:
  info.image.reset(image, base::mac::ScopedCFTypeRef<CVImageBufferRef>::RETAIN)
which seems too verbose to me.

+  ASSUME,
+  RETAIN
+};
+
 // ScopedCFTypeRef<> is patterned after scoped_ptr<>, but maintains ownership
 // of a CoreFoundation object: any object that can be represented as a
 // CFTypeRef.  Style deviations here are solely for compatibility with
 // scoped_ptr<>'s interface, with which everyone is already familiar.
 //
 // When ScopedCFTypeRef<> takes ownership of an object (in the constructor or
 // in reset()), it takes over the caller's existing ownership claim.  The
 // caller must own the object it gives to ScopedCFTypeRef<>, and relinquishes
 // an ownership claim to that object.  ScopedCFTypeRef<> does not call
 // CFRetain().

[Ami GONE FROM CHROMIUM, 2012/05/17 21:44:47]

This paragraph seems like the doco for ASSUME and at least half of its
assertions are incorrect for RETAIN :)

[sail, 2012/05/23 23:02:29]

Done.

 template<typename CFT>
 class ScopedCFTypeRef {
  public:
   typedef CFT element_type;
 
-  explicit ScopedCFTypeRef(CFT object = NULL)
+  explicit ScopedCFTypeRef(CFT object = NULL,
+                           OwnershipPolicy policy = ASSUME)

[Ami GONE FROM CHROMIUM, 2012/05/17 21:44:47]

I don't know that this style-guide violation (default param value) is justified
by the compatibility sentence at l.24-25...

[sail, 2012/05/23 23:02:29]

This matches the scoped_nsprotocol constructor.
If you think this should really change then mind if I do it a separate CL?

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:29]

Like I said, my opinions about this file are low-priority advisory.  If
base/mac/OWNERS are pleased to approve then more power to you.

       : object_(object) {
+    if (object_ && policy == RETAIN)
+      CFRetain(object_);
+  }
+
+  ScopedCFTypeRef(const ScopedCFTypeRef<CFT>& that)
+      : object_(that.object_) {
+    if (object_)
+      CFRetain(object_);
   }
 
   ~ScopedCFTypeRef() {
     if (object_)
       CFRelease(object_);
   }
 
-  void reset(CFT object = NULL) {
+  ScopedCFTypeRef& operator=(const ScopedCFTypeRef<CFT>& that) {
+    reset(that.get(), RETAIN);
+    return *this;
+  }
+
+  void reset(CFT object = NULL,
+             OwnershipPolicy policy = ASSUME) {
+    if (object && policy == RETAIN)
+      CFRetain(object);
     if (object_)
       CFRelease(object_);
     object_ = object;
   }
 
   bool operator==(CFT that) const {
     return object_ == that;
   }
 
   bool operator!=(CFT that) const {
@@ skipping 18 matching lines @@
   // a wrapper for CFRelease().  To force a ScopedCFTypeRef<> object to call
   // CFRelease(), use ScopedCFTypeRef<>::reset().
   CFT release() WARN_UNUSED_RESULT {
     CFT temp = object_;
     object_ = NULL;
     return temp;
   }
 
  private:
   CFT object_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedCFTypeRef);
 };
 
 }  // namespace mac
 }  // namespace base
 
 #endif  // BASE_MAC_SCOPED_CFTYPEREF_H_