Fix issue 160529 in a nice way

media/filters/ffmpeg_demuxer.cc

 // Copyright (c) 2009 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.
 
 #include "base/scoped_ptr.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "media/base/filter_host.h"
 #include "media/filters/ffmpeg_common.h"
 #include "media/filters/ffmpeg_demuxer.h"
 #include "media/filters/ffmpeg_glue.h"
 
-namespace {
-
-// Helper function to deep copy an AVPacket's data, size and timestamps.
-// Returns NULL if a packet could not be cloned (i.e., out of memory).
-AVPacket* ClonePacket(AVPacket* packet) {
-  scoped_ptr<AVPacket> clone(new AVPacket());
-  if (!clone.get() || av_new_packet(clone.get(), packet->size) < 0) {

[fbarchard, 2009/08/18 22:34:16]

removing this breaks the unittest
C:\chrome-svn\src\media\filters\ffmpeg_demuxer_unittest.cc

gmock is rather complicated, so I've fixed it up and will submit this as a new
patch

-    return NULL;
-  }
-  DCHECK_EQ(clone->size, packet->size);
-  clone->dts = packet->dts;
-  clone->pts = packet->pts;
-  clone->duration = packet->duration;
-  memcpy(clone->data, packet->data, clone->size);
-  return clone.release();
-}
-
-}  // namespace
-
 namespace media {
 
 //
 // AVPacketBuffer
 //
 class AVPacketBuffer : public Buffer {
  public:
   AVPacketBuffer(AVPacket* packet, const base::TimeDelta& timestamp,
                  const base::TimeDelta& duration)
       : packet_(packet) {
@@ skipping 445 matching lines @@
   }
 
   // Queue the packet with the appropriate stream.
   // TODO(scherkus): should we post this back to the pipeline thread?  I'm
   // worried about downstream filters (i.e., decoders) executing on this
   // thread.
   DCHECK(packet->stream_index >= 0);
   DCHECK(packet->stream_index < static_cast<int>(packet_streams_.size()));
   FFmpegDemuxerStream* demuxer_stream = packet_streams_[packet->stream_index];
   if (demuxer_stream) {
-    // Duplicate the entire packet to avoid aliasing.
-    // Directly affects MP3, but do all formats to be safe.
-    scoped_ptr<AVPacket> clone(ClonePacket(packet.get()));
-    if (!clone.get()) {
-      NOTREACHED();
-      return;
-    }
-    // Free FFmpeg-allocated memory and swap original packet into |clone| so
-    // that it gets deleted as |clone| goes out of scope.
-    av_free_packet(packet.get());
-    packet.swap(clone);
+    // If a packets is returned by FFmpeg's av_parser_parse2()
+    // the packet will reference an inner memory of FFmpeg
+    // In this case, the packet's "destruct" member is NULL, it MUST be duplicated.

[fbarchard, 2009/08/18 22:34:16]

nit more than 80 columns... break comment into 2 lines.

[ffmpeg, 2009/08/19 01:05:25]

OK, Let's focus on issue: 174027 which is the enhanced version

+    av_dup_packet(packet.get());
 
     // Queue the packet with the appropriate stream.  The stream takes
     // ownership of the AVPacket.
     current_timestamp_ = demuxer_stream->EnqueuePacket(packet.release());
   } else {
     av_free_packet(packet.get());
   }
 
   // Create a loop by posting another task.  This allows seek and message loop
   // quit tasks to get processed.
@@ skipping 57 matching lines @@
   read_event_.Wait();
   return last_read_bytes_;
 }
 
 void FFmpegDemuxer::SignalReadCompleted(size_t size) {
   last_read_bytes_ = size;
   read_event_.Signal();
 }
 
 }  // namespace media