Packt is implicitly shared to reduce AVPacket copy

src/Packet.cpp

@@ -27,6 +27,22 @@ namespace QtAV {
 
 const qreal Packet::kEndPts = -0.618;
 
+class PacketPrivate : public QSharedData
+{
+public:
+    PacketPrivate()
+        : initialized(false)
+    {
+        av_init_packet(&avpkt);
+    }
+    ~PacketPrivate() {
+        av_free_packet(&avpkt); // free old side data and ref
+    }
+
+    bool initialized;
+    AVPacket avpkt;
+};
+
 Packet Packet::fromAVPacket(const AVPacket *avpkt, double time_base)
 {
     Packet pkt;
@@ -56,10 +72,8 @@ bool Packet::fromAVPacket(Packet* pkt, const AVPacket *avpkt, double time_base)
         pkt->pts = 0; // TODO: init value
     if (avpkt->dts != AV_NOPTS_VALUE) //has B-frames
         pkt->dts = avpkt->dts * time_base;
-    else if (avpkt->pts != AV_NOPTS_VALUE) // is it ok?
-        pkt->dts = avpkt->pts * time_base;
     else
-        pkt->dts = 0; // TODO: init value
+        pkt->dts = pkt->pts;
     //TODO: pts must >= 0? look at ffplay
     pkt->pts = qMax<qreal>(0, pkt->pts);
     pkt->dts = qMax<qreal>(0, pkt->dts);
@@ -100,9 +114,9 @@ bool Packet::fromAVPacket(Packet* pkt, const AVPacket *avpkt, double time_base)
 
     // TODO: pkt->avpkt. data is not necessary now. see mpv new_demux_packet_from_avpacket
     // copy properties and side data. does not touch data, size and ref
-    // libav has no av_copy_packet_side_data() in public api
-    AVPacket *p = new AVPacket();
-    av_init_packet(p);
+    pkt->d = QSharedDataPointer<PacketPrivate>(new PacketPrivate());
+    pkt->d->initialized = true;
+    AVPacket *p = &pkt->d->avpkt;
     av_packet_copy_props(p, avpkt);
     if (!pkt->data.isEmpty()) {
         p->data = (uint8_t*)pkt->data.constData();
@@ -112,7 +126,6 @@ bool Packet::fromAVPacket(Packet* pkt, const AVPacket *avpkt, double time_base)
     p->pts = pkt->pts * 1000.0;
     p->dts = pkt->dts * 1000.0;
     p->duration = pkt->duration * 1000.0;
-    pkt->avpkt = p;
     return true;
 }
 
@@ -123,30 +136,27 @@ Packet::Packet()
     , duration(0)
     , dts(0)
     , position(0)
-    , avpkt(0)
 {
 }
 
+
 Packet::Packet(const Packet &other)
+    : hasKeyFrame(other.hasKeyFrame)
+    , isCorrupt(other.isCorrupt)
+    , data(other.data)
+    , pts(other.pts)
+    , duration(other.duration)
+    , dts(other.dts)
+    , position(other.position)
+    , d(other.d)
 {
-    if (this == &other)
-        return;
-    hasKeyFrame = other.hasKeyFrame;
-    isCorrupt = other.isCorrupt;
-    pts = other.pts;
-    duration = other.duration;
-    dts = other.dts;
-    position = other.position;
-    data = other.data;
-    avpkt = new AVPacket();
-    av_init_packet(avpkt);
-    av_packet_copy_props(avpkt, other.toAVPacket());
 }
 
 Packet& Packet::operator =(const Packet& other)
 {
     if (this == &other)
         return *this;
+    d = other.d;
     hasKeyFrame = other.hasKeyFrame;
     isCorrupt = other.isCorrupt;
     pts = other.pts;
@@ -154,25 +164,11 @@ Packet& Packet::operator =(const Packet& other)
     dts = other.dts;
     position = other.position;
     data = other.data;
-    if (!avpkt) {
-        avpkt = new AVPacket();
-        av_init_packet(avpkt);
-    } else {
-        av_free_packet(avpkt); // free old side data and ref
-    }
-    av_packet_copy_props(avpkt, other.toAVPacket());
     return *this;
 }
 
 Packet::~Packet()
 {
-    if (avpkt) {
-        // only free side data and ref, does not release data
-        av_free_packet(avpkt);
-
-        delete avpkt;
-        avpkt = 0;
-    }
 }
 
 void Packet::markEnd()
@@ -181,25 +177,30 @@ void Packet::markEnd()
     pts = kEndPts;
 }
 
-AVPacket* Packet::toAVPacket() const
+const AVPacket *Packet::asAVPacket() const
 {
-    if (avpkt)
-        return avpkt;
-
-    avpkt = new AVPacket();
-    av_init_packet(avpkt);
-    avpkt->pts = pts * 1000.0;
-    avpkt->dts = dts * 1000.0;
-    avpkt->duration = duration * 1000.0;
-    if (isCorrupt)
-        avpkt->flags |= AV_PKT_FLAG_CORRUPT;
-    if (hasKeyFrame)
-        avpkt->flags |= AV_PKT_FLAG_KEY;
-    if (!data.isEmpty()) {
-        avpkt->data = (uint8_t*)data.constData();
-        avpkt->size = data.size();
+    if (d.constData()) {
+        if (d->initialized) //d.data() was 0 if d has not been accessed. now only contains avpkt, check d.constData() is engough
+            return &d->avpkt;
+    } else {
+        d = QSharedDataPointer<PacketPrivate>(new PacketPrivate());
     }
-    return avpkt;
+
+    d->initialized = true;
+    AVPacket *p = &d->avpkt;
+    p->pts = pts * 1000.0;
+    p->dts = dts * 1000.0;
+    p->duration = duration * 1000.0;
+    p->pos = position;
+    if (isCorrupt)
+        p->flags |= AV_PKT_FLAG_CORRUPT;
+    if (hasKeyFrame)
+        p->flags |= AV_PKT_FLAG_KEY;
+    if (!data.isEmpty()) {
+        p->data = (uint8_t*)data.constData();
+        p->size = data.size();
+    }
+    return p;
 }
 
 } //namespace QtAV

src/QtAV/Packet.h

@@ -23,30 +23,37 @@
 #define QAV_PACKET_H
 
 #include <QtCore/QByteArray>
+#include <QtCore/QSharedData>
 #include <QtAV/QtAV_Global.h>
 
 struct AVPacket;
 
 namespace QtAV {
 
+class PacketPrivate;
 class Q_AV_EXPORT Packet
 {
 public:
-    //const avpkt? if no use ref
     static Packet fromAVPacket(const AVPacket* avpkt, double time_base);
     static bool fromAVPacket(Packet *pkt, const AVPacket *avpkt, double time_base);
 
     Packet();
-    Packet(const Packet& other);
     ~Packet();
 
+    // required if no defination of PacketPrivate
+    Packet(const Packet& other);
     Packet& operator =(const Packet& other);
 
     inline bool isValid() const;
     inline bool isEnd() const;
     void markEnd();
-    /// Packet takes the owner ship. time unit is ms
-    AVPacket* toAVPacket() const;
+    /*!
+     * \brief asAVPacket
+     * If Packet is constructed from AVPacket, then data and properties are the same as that AVPacket.
+     * Otherwise, Packet's data and properties are used and no side data.
+     * Packet takes the owner ship. time unit is always ms even constructed from AVPacket.
+     */
+    const AVPacket* asAVPacket() const;
 
     bool hasKeyFrame;
     bool isCorrupt;
@@ -59,7 +66,8 @@ public:
 private:
     static const qreal kEndPts;
     // TODO: implicity shared. can not use QSharedData
-    mutable AVPacket *avpkt;
+    // we must define  default/copy ctor, dtor and operator= so that we can provide only forward declaration of PacketPrivate
+    mutable QSharedDataPointer<PacketPrivate> d;
 };
 
 bool Packet::isValid() const