[vlc-commits] [Git][videolan/vlc][master] 11 commits: demux: adaptive: remove arbitrary read size

Hugo Beauzée-Luyssen (@chouquette) gitlab at videolan.org
Sat Sep 18 15:27:22 UTC 2021



Hugo Beauzée-Luyssen pushed to branch master at VideoLAN / VLC


Commits:
451f6242 by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: remove arbitrary read size

- - - - -
a0115366 by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: improve webvtt probing

- - - - -
76fd9ead by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: remove null adaptationSet checks

now checked in parsers

- - - - -
af72f0bb by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: use enum class for Role

- - - - -
e71b1648 by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: deduplicate attribute lookup

- - - - -
f36a0c55 by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: order sets on insert

- - - - -
33fb9022 by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: fix firstchunk, peek and restart

- - - - -
cdd1dda3 by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: clear discontinuity on new demux first chunk

- - - - -
b85ef41f by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: set segment source chunk type

- - - - -
3e32bbeb by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: pass source back to manager for deletion

- - - - -
7b4eff1c by Francois Cartegnie at 2021-09-18T15:08:12+00:00
demux: adaptive: split download queues

prioritize manifests over data chunks

- - - - -


15 changed files:

- modules/demux/adaptive/StreamFormat.cpp
- modules/demux/adaptive/Streams.cpp
- modules/demux/adaptive/http/Chunk.cpp
- modules/demux/adaptive/http/Chunk.h
- modules/demux/adaptive/http/HTTPConnectionManager.cpp
- modules/demux/adaptive/http/HTTPConnectionManager.h
- modules/demux/adaptive/playlist/BasePeriod.cpp
- modules/demux/adaptive/playlist/Role.cpp
- modules/demux/adaptive/playlist/Role.hpp
- modules/demux/adaptive/playlist/Segment.cpp
- modules/demux/adaptive/tools/Retrieve.cpp
- modules/demux/dash/mpd/IsoffMainParser.cpp
- modules/demux/hls/playlist/Parser.cpp
- modules/demux/smooth/playlist/MemoryChunk.cpp
- modules/demux/smooth/playlist/MemoryChunk.hpp


Changes:

=====================================
modules/demux/adaptive/StreamFormat.cpp
=====================================
@@ -108,6 +108,20 @@ static int ID3Callback(uint32_t, const uint8_t *, size_t, void *)
     return VLC_EGENERIC;
 }
 
+static bool IsWebVTT(const char *p, size_t sz)
+{
+    /* match optional U+FEFF BOM */
+    const uint8_t webvtt[] = { 0xEF, 0xBB, 0xBF, 0x57, 0x45, 0x42, 0x56, 0x54, 0x54 };
+    for(int i=3; i>=0; i-=3)
+    {
+        if(sz > (size_t)(10 - i) &&
+           !memcmp(webvtt + i, p, 9 - i) &&
+           (p[9 - i] == '\n' || p[9 - i] == '\r' || p[9 - i] == ' ' || p[9 - i] == '\t'))
+            return true;
+    }
+    return false;
+}
+
 StreamFormat::StreamFormat(const void *data_, size_t sz)
 {
     const uint8_t *data = reinterpret_cast<const uint8_t *>(data_);
@@ -120,8 +134,7 @@ StreamFormat::StreamFormat(const void *data_, size_t sz)
                        !memcmp(&moov[4], &data[4], 4) ||
                        !memcmp(&moov[8], &data[4], 4)))
         type = StreamFormat::Type::MP4;
-    else if(sz > 7 && !memcmp("WEBVTT", data, 6) &&
-            std::isspace(static_cast<unsigned char>(data[7])))
+    else if(IsWebVTT((const char *)data, sz))
         type = StreamFormat::Type::WebVTT;
     else if(sz > 4 && !memcmp("\x1A\x45\xDF\xA3", data, 4))
         type = StreamFormat::Type::WebM;


=====================================
modules/demux/adaptive/Streams.cpp
=====================================
@@ -250,14 +250,15 @@ bool AbstractStream::startDemux()
     {
         currentChunk = getNextChunk();
         needrestart = false;
+        discontinuity = false;
     }
 
     demuxersource->Reset();
+    demuxfirstchunk = true;
     demuxer = createDemux(format);
     if(!demuxer && format != StreamFormat())
         msg_Err(p_realdemux, "Failed to create demuxer %p %s", (void *)demuxer,
                 format.str().c_str());
-    demuxfirstchunk = true;
 
     return !!demuxer;
 }
@@ -506,13 +507,13 @@ block_t * AbstractStream::readNextBlock()
     if (currentChunk == nullptr && !eof)
         currentChunk = getNextChunk();
 
-    if(discontinuity && demuxfirstchunk)
+    if(demuxfirstchunk)
     {
         /* clear up discontinuity on demux start (discontinuity on start segment bug) */
         discontinuity = false;
+        needrestart = false;
     }
-
-    if(discontinuity || needrestart)
+    else if(discontinuity || needrestart)
     {
         msg_Info(p_realdemux, "Ending demuxer stream. %s%s",
                  discontinuity ? "[discontinuity]" : "",


=====================================
modules/demux/adaptive/http/Chunk.cpp
=====================================
@@ -81,7 +81,7 @@ AbstractChunk::AbstractChunk(AbstractChunkSource *source_)
 
 AbstractChunk::~AbstractChunk()
 {
-    delete source;
+    source->recycle();
 }
 
 std::string AbstractChunk::getContentType() const
@@ -247,6 +247,11 @@ block_t * HTTPChunkSource::read(size_t readsize)
     return p_block;
 }
 
+void HTTPChunkSource::recycle()
+{
+    connManager->recycleSource(this);
+}
+
 std::string HTTPChunkSource::getContentType() const
 {
     mutex_locker locker {lock};
@@ -518,6 +523,7 @@ HTTPChunk::HTTPChunk(const std::string &url, AbstractConnectionManager *manager,
                      const adaptive::ID &id, ChunkType type, const BytesRange &range):
     AbstractChunk(manager->makeSource(url, id, type, range))
 {
+    manager->start(source);
 }
 
 HTTPChunk::~HTTPChunk()


=====================================
modules/demux/adaptive/http/Chunk.h
=====================================
@@ -67,15 +67,18 @@ namespace adaptive
 
         class AbstractChunkSource : public ChunkInterface
         {
+            friend class AbstractConnectionManager;
+
             public:
-                AbstractChunkSource(ChunkType, const BytesRange & = BytesRange());
-                virtual ~AbstractChunkSource();
                 const BytesRange &  getBytesRange   () const;
                 ChunkType           getChunkType    () const;
                 virtual std::string getContentType  () const override;
                 virtual RequestStatus getRequestStatus() const override;
+                virtual void        recycle() = 0;
 
             protected:
+                AbstractChunkSource(ChunkType, const BytesRange & = BytesRange());
+                virtual ~AbstractChunkSource();
                 ChunkType           type;
                 RequestStatus       requeststatus;
                 size_t              contentLength;
@@ -119,6 +122,7 @@ namespace adaptive
                 virtual bool        hasMoreData     () const  override;
                 virtual size_t      getBytesRead    () const  override;
                 virtual std::string getContentType  () const  override;
+                virtual void        recycle() override;
 
                 static const size_t CHUNK_SIZE = 32768;
 


=====================================
modules/demux/adaptive/http/HTTPConnectionManager.cpp
=====================================
@@ -65,19 +65,26 @@ void AbstractConnectionManager::setDownloadRateObserver(IDownloadRateObserver *o
     rateObserver = obs;
 }
 
+void AbstractConnectionManager::deleteSource(AbstractChunkSource *source)
+{
+    delete source;
+}
 
 HTTPConnectionManager::HTTPConnectionManager    (vlc_object_t *p_object_)
     : AbstractConnectionManager( p_object_ ),
       localAllowed(false)
 {
     vlc_mutex_init(&lock);
-    downloader = new (std::nothrow) Downloader();
+    downloader = new Downloader();
+    downloaderhp = new Downloader();
     downloader->start();
+    downloaderhp->start();
 }
 
 HTTPConnectionManager::~HTTPConnectionManager   ()
 {
     delete downloader;
+    delete downloaderhp;
     this->closeAllConnections();
     while(!factories.empty())
     {
@@ -115,7 +122,7 @@ AbstractConnection * HTTPConnectionManager::reuseConnection(ConnectionParams &pa
 
 AbstractConnection * HTTPConnectionManager::getConnection(ConnectionParams &params)
 {
-    if(unlikely(factories.empty() || !downloader))
+    if(unlikely(factories.empty() || !downloader || !downloaderhp))
         return nullptr;
 
     if(params.isLocal())
@@ -160,26 +167,46 @@ AbstractChunkSource *HTTPConnectionManager::makeSource(const std::string &url,
         case ChunkType::Init:
         case ChunkType::Index:
         case ChunkType::Segment:
+
+        case ChunkType::Key:
+        case ChunkType::Playlist:
+        default:
             return new HTTPChunkBufferedSource(url, this, id, type, range);
+    }
+}
+
+void HTTPConnectionManager::recycleSource(AbstractChunkSource *source)
+{
+    deleteSource(source);
+}
+
+Downloader * HTTPConnectionManager::getDownloadQueue(const AbstractChunkSource *source) const
+{
+    switch(source->getChunkType())
+    {
+        case ChunkType::Init:
+        case ChunkType::Index:
+        case ChunkType::Segment:
+            return downloader;
         case ChunkType::Key:
         case ChunkType::Playlist:
         default:
-            return new HTTPChunkSource(url, this, id, type, range, true);
+            return downloaderhp;
     }
 }
 
 void HTTPConnectionManager::start(AbstractChunkSource *source)
 {
     HTTPChunkBufferedSource *src = dynamic_cast<HTTPChunkBufferedSource *>(source);
-    if(src)
-        downloader->schedule(src);
+    if(src && !src->isDone())
+        getDownloadQueue(src)->schedule(src);
 }
 
 void HTTPConnectionManager::cancel(AbstractChunkSource *source)
 {
     HTTPChunkBufferedSource *src = dynamic_cast<HTTPChunkBufferedSource *>(source);
     if(src)
-        downloader->cancel(src);
+        getDownloadQueue(src)->cancel(src);
 }
 
 void HTTPConnectionManager::setLocalConnectionsAllowed()


=====================================
modules/demux/adaptive/http/HTTPConnectionManager.h
=====================================
@@ -55,6 +55,7 @@ namespace adaptive
                 virtual AbstractChunkSource *makeSource(const std::string &,
                                                         const ID &, ChunkType,
                                                         const BytesRange &) = 0;
+                virtual void recycleSource(AbstractChunkSource *) = 0;
 
                 virtual void start(AbstractChunkSource *) = 0;
                 virtual void cancel(AbstractChunkSource *) = 0;
@@ -64,6 +65,7 @@ namespace adaptive
                 void setDownloadRateObserver(IDownloadRateObserver *);
 
             protected:
+                void deleteSource(AbstractChunkSource *);
                 vlc_object_t                                       *p_object;
 
             private:
@@ -81,6 +83,7 @@ namespace adaptive
                 virtual AbstractChunkSource *makeSource(const std::string &,
                                                         const ID &, ChunkType,
                                                         const BytesRange &) override;
+                virtual void recycleSource(AbstractChunkSource *) override;
 
                 virtual void start(AbstractChunkSource *)  override;
                 virtual void cancel(AbstractChunkSource *)  override;
@@ -90,11 +93,13 @@ namespace adaptive
             private:
                 void    releaseAllConnections ();
                 Downloader                                         *downloader;
+                Downloader                                         *downloaderhp;
                 vlc_mutex_t                                         lock;
                 std::vector<AbstractConnection *>                   connectionPool;
                 std::list<AbstractConnectionFactory *>              factories;
                 bool                                                localAllowed;
                 AbstractConnection * reuseConnection(ConnectionParams &);
+                Downloader * getDownloadQueue(const AbstractChunkSource *) const;
         };
     }
 }


=====================================
modules/demux/adaptive/playlist/BasePeriod.cpp
=====================================
@@ -33,6 +33,7 @@
 
 #include <vlc_common.h>
 #include <vlc_arrays.h>
+#include <algorithm>
 #include <cassert>
 
 using namespace adaptive::playlist;
@@ -63,16 +64,11 @@ const std::vector<BaseAdaptationSet*>&  BasePeriod::getAdaptationSets() const
 
 void BasePeriod::addAdaptationSet(BaseAdaptationSet *adaptationSet)
 {
-    if ( adaptationSet != nullptr )
-    {
-        if(adaptationSet->getRepresentations().empty())
-        {
-            assert(!adaptationSet->getRepresentations().empty());
-            return; /* will leak */
-        }
-        adaptationSets.push_back(adaptationSet);
-        childs.push_back(adaptationSet);
-    }
+    auto p = std::find_if(adaptationSets.begin(), adaptationSets.end(),
+        [adaptationSet](BaseAdaptationSet *s){
+            return adaptationSet->getRole() < s->getRole(); });
+    adaptationSets.insert(p, adaptationSet);
+    childs.push_back(adaptationSet);
 }
 
 BaseAdaptationSet *BasePeriod::getAdaptationSetByID(const adaptive::ID &id) const


=====================================
modules/demux/adaptive/playlist/Role.cpp
=====================================
@@ -26,11 +26,16 @@
 using namespace adaptive;
 using namespace adaptive::playlist;
 
-Role::Role(unsigned r)
+Role::Role(Value r)
 {
     value = r;
 }
 
+bool Role::operator <(const Role &other) const
+{
+    return value > other.value;
+}
+
 bool Role::operator ==(const Role &other) const
 {
     return value == other.value;
@@ -38,13 +43,13 @@ bool Role::operator ==(const Role &other) const
 
 bool Role::isDefault() const
 {
-    return value == ROLE_MAIN;
+    return value == Value::Main;
 }
 
 bool Role::autoSelectable() const
 {
-    return value == ROLE_MAIN ||
-           value == ROLE_ALTERNATE ||
-           value == ROLE_SUBTITLE ||
-           value == ROLE_CAPTION;
+    return value == Value::Main ||
+           value == Value::Alternate ||
+           value == Value::Subtitle ||
+           value == Value::Caption;
 }


=====================================
modules/demux/adaptive/playlist/Role.hpp
=====================================
@@ -27,20 +27,24 @@ namespace adaptive
         class Role
         {
             public:
-                static const unsigned ROLE_MAIN           = 0;
-                static const unsigned ROLE_ALTERNATE      = 1;
-                static const unsigned ROLE_SUPPLEMENTARY  = 2;
-                static const unsigned ROLE_COMMENTARY     = 3;
-                static const unsigned ROLE_DUB            = 4;
-                static const unsigned ROLE_CAPTION        = 5;
-                static const unsigned ROLE_SUBTITLE       = 6;
-                Role(unsigned = ROLE_ALTERNATE);
+                enum class Value
+                {
+                    Main,
+                    Alternate,
+                    Supplementary,
+                    Commentary,
+                    Dub,
+                    Caption,
+                    Subtitle,
+                };
+                Role(Value = Value::Main);
+                bool operator<(const Role &) const;
                 bool operator==(const Role &) const;
                 bool isDefault() const;
                 bool autoSelectable() const;
 
             private:
-                unsigned value;
+                Value value;
         };
     }
 }


=====================================
modules/demux/adaptive/playlist/Segment.cpp
=====================================
@@ -82,9 +82,16 @@ SegmentChunk* ISegment::toChunk(SharedResources *res, AbstractConnectionManager
     BytesRange range;
     if(startByte != endByte)
         range = BytesRange(startByte, endByte);
+    ChunkType chunkType;
+    if(dynamic_cast<InitSegment *>(this))
+        chunkType = ChunkType::Init;
+    else if(dynamic_cast<IndexSegment *>(this))
+        chunkType = ChunkType::Index;
+    else
+        chunkType = ChunkType::Segment;
     AbstractChunkSource *source = connManager->makeSource(url,
                                                           rep->getAdaptationSet()->getID(),
-                                                          ChunkType::Segment,
+                                                          chunkType,
                                                           range);
     if(source)
     {
@@ -103,7 +110,7 @@ SegmentChunk* ISegment::toChunk(SharedResources *res, AbstractConnectionManager
         }
         else
         {
-            delete source;
+            connManager->recycleSource(source);
         }
     }
     return nullptr;


=====================================
modules/demux/adaptive/tools/Retrieve.cpp
=====================================
@@ -29,6 +29,8 @@
 #include "../http/Chunk.h"
 #include "../SharedResources.hpp"
 
+#include <vlc_block.h>
+
 using namespace adaptive;
 using namespace adaptive::http;
 
@@ -44,7 +46,16 @@ block_t * Retrieve::HTTP(SharedResources *resources, ChunkType type,
         return nullptr;
     }
 
-    block_t *block = datachunk->read(1<<25);
+    block_t *p_head = nullptr;
+    block_t **pp_tail = &p_head;
+    for(;;)
+    {
+        block_t *p_block = datachunk->readBlock();
+        if(!p_block)
+            break;
+        block_ChainLastAppend(&pp_tail, p_block);
+    }
     delete datachunk;
-    return block;
+
+    return p_head ? block_ChainGather(p_head) : nullptr;
 }


=====================================
modules/demux/dash/mpd/IsoffMainParser.cpp
=====================================
@@ -322,19 +322,19 @@ void    IsoffMainParser::parseAdaptationSets  (MPD *mpd, Node *periodNode, BaseP
                 const std::string &rolevalue = role->getAttributeValue("value");
                 adaptationSet->description.Set(rolevalue);
                 if(rolevalue == "main")
-                    adaptationSet->setRole(Role::ROLE_MAIN);
+                    adaptationSet->setRole(Role::Value::Main);
                 else if(rolevalue == "alternate")
-                    adaptationSet->setRole(Role::ROLE_ALTERNATE);
+                    adaptationSet->setRole(Role::Value::Alternate);
                 else if(rolevalue == "supplementary")
-                    adaptationSet->setRole(Role::ROLE_SUPPLEMENTARY);
+                    adaptationSet->setRole(Role::Value::Supplementary);
                 else if(rolevalue == "commentary")
-                    adaptationSet->setRole(Role::ROLE_COMMENTARY);
+                    adaptationSet->setRole(Role::Value::Commentary);
                 else if(rolevalue == "dub")
-                    adaptationSet->setRole(Role::ROLE_DUB);
+                    adaptationSet->setRole(Role::Value::Dub);
                 else if(rolevalue == "caption")
-                    adaptationSet->setRole(Role::ROLE_CAPTION);
+                    adaptationSet->setRole(Role::Value::Caption);
                 else if(rolevalue == "subtitle")
-                    adaptationSet->setRole(Role::ROLE_SUBTITLE);
+                    adaptationSet->setRole(Role::Value::Subtitle);
             }
         }
 


=====================================
modules/demux/hls/playlist/Parser.cpp
=====================================
@@ -515,17 +515,19 @@ M3U8 * M3U8Parser::parse(vlc_object_t *p_object, stream_t *p_stream, const std::
                     desc += pair.second->getAttributeByName("NAME")->quotedString();
                 }
 
+                const Attribute *typeattr = pair.second->getAttributeByName("TYPE");
+
                 if(pair.second->getAttributeByName("CODECS"))
                 {
                     rep->addCodecs(pair.second->getAttributeByName("CODECS")->quotedString());
                 }
-                else
+                else if(typeattr)
                 {
-                    if(pair.second->getAttributeByName("TYPE")->value == "AUDIO")
+                    if(typeattr->value == "AUDIO")
                         rep->addCodecs("mp4a");
-                    else if(pair.second->getAttributeByName("TYPE")->value == "VIDEO")
+                    else if(typeattr->value == "VIDEO")
                         rep->addCodecs("avc1");
-                    else if(pair.second->getAttributeByName("TYPE")->value == "SUBTITLES")
+                    else if(typeattr->value == "SUBTITLES")
                         rep->addCodecs("wvtt");
                 }
 
@@ -539,23 +541,22 @@ M3U8 * M3U8Parser::parse(vlc_object_t *p_object, stream_t *p_stream, const std::
                 if(pair.second->getAttributeByName("DEFAULT"))
                 {
                     if(pair.second->getAttributeByName("DEFAULT")->value == "YES")
-                        altAdaptSet->setRole(Role(Role::ROLE_MAIN));
+                        altAdaptSet->setRole(Role(Role::Value::Main));
                     else
-                        altAdaptSet->setRole(Role(Role::ROLE_ALTERNATE));
+                        altAdaptSet->setRole(Role(Role::Value::Alternate));
                 }
 
                 if(pair.second->getAttributeByName("AUTOSELECT"))
                 {
                     if(pair.second->getAttributeByName("AUTOSELECT")->value == "NO" &&
                        !pair.second->getAttributeByName("DEFAULT"))
-                        altAdaptSet->setRole(Role(Role::ROLE_SUPPLEMENTARY));
+                        altAdaptSet->setRole(Role(Role::Value::Supplementary));
                 }
 
                 /* Subtitles unsupported for now */
-                const Attribute *typeattr = pair.second->getAttributeByName("TYPE");
                 if(typeattr->value == "SUBTITLES")
                 {
-                    altAdaptSet->setRole(Role(Role::ROLE_SUBTITLE));
+                    altAdaptSet->setRole(Role(Role::Value::Subtitle));
                     rep->streamFormat = StreamFormat(StreamFormat::Type::Unsupported);
                 }
                 else if(typeattr->value != "AUDIO" && typeattr->value != "VIDEO")


=====================================
modules/demux/smooth/playlist/MemoryChunk.cpp
=====================================
@@ -51,6 +51,11 @@ size_t MemoryChunkSource::getBytesRead() const
     return i_read;
 }
 
+void MemoryChunkSource::recycle()
+{
+    delete this;
+}
+
 block_t * MemoryChunkSource::readBlock()
 {
     block_t *p_block = nullptr;


=====================================
modules/demux/smooth/playlist/MemoryChunk.hpp
=====================================
@@ -38,6 +38,7 @@ namespace smooth
                 virtual block_t * read(size_t) override;
                 virtual bool      hasMoreData() const override;
                 virtual size_t    getBytesRead() const  override;
+                virtual void      recycle() override;
 
             private:
                 block_t *data;



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/285ea79612d956c4a7fcb882bc80f85467e83e1c...7b4eff1ccc01302a057f8348c626514b2ed4207d

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/285ea79612d956c4a7fcb882bc80f85467e83e1c...7b4eff1ccc01302a057f8348c626514b2ed4207d
You're receiving this email because of your account on code.videolan.org.




More information about the vlc-commits mailing list