[vlc-devel] [PATCH] demux: hls: fix consistent live delay (#21614 WIP2)
Thomas Guillem
thomas at gllm.fr
Mon Oct 7 09:28:48 CEST 2019
Ping Francois.
On Wed, Oct 2, 2019, at 16:18, Tristan Matthews wrote:
> Hi,
>
> On Wed, Oct 2, 2019 at 10:07 AM Abdullah Alansari <ahimta at gmail.com> wrote:
> >
> > From f9b9daf8d32134ca60c5af6c73be1a8fd49a73a5 Mon Sep 17 00:00:00 2001
> > From: Abdullah Alansari <ahimta at gmail.com>
> > Date: Mon, 30 Sep 2019 20:32:16 +0300
> > Subject: [PATCH] demux: hls: fix consistent live delay (#21614 WIP2)
> >
> > The bug was first observed on
> > https://www.youtube.com/watch?v=WI86J46VRUc
> >
> > This YouTube live-stream contains a wall-clock in the bottom-right
> > corner which makes it very convenient for testing.
> >
> > Before this patch/fix, playback with VLC is 1minute+ behind the YouTube
> > website:sweat_smile: and after the patch, YouTube is the one
> > behind:smiley:.
> >
> > This bug should manifest in all platforms in any YouTube live video.
> >
> > The initial analysis of this bug showed that the reason for this delay
> > is that VLC plays the first video in the M38U file which can be 1minute+
> > before the current wall-clock time:sweat_smile:.
> >
> > This was observed on http://www.tv.wataan.com/?video=show&show=104
> > where the delay was non-existent on Firefox and existent on VLC
> > :sweat_smile:.
> >
> > On Firefox, the second-to-last video in the M3U8 was played while on
> > VLC the first video was played. And upon downloading both videos,
> > Firefox's downloaded has the correct wall-clock time while VLC's one
> > was 1minute+ behind:sweat_smile:.
> >
> > And thus this patch does something similar to the Firefox observed
> > behavior above (which may just be from the website's own code).
> >
> > I have almost the whole related RFC and the 6.3.3 sections is probably
> > the single most relevent:
> > https://tools.ietf.org/html/rfc8216#section-6.3.3
> > Specifically "the client SHOULD NOT choose a segment that
> > starts less than three target durations from the end of the
> > Playlist file."
> >
> > This patch may possibly break non-HLS and not fix other adaptive-live
> > stuff (e.g: DASH), if they are already affected:sweat_smile:.
> >
> > A list of tested samples that had the delay before this patch and don't
> > anymore:
> > 1. https://www.youtube.com/watch?v=WI86J46VRUc
> > 2. http://livecdnh3.tvanywhere.ae/hls/jadeed/03.m3u8
> >
> > Related to https://trac.videolan.org/vlc/ticket/21614#attachments
> > ---
> > modules/demux/adaptive/SegmentTracker.cpp | 8 ++++----
> > .../adaptive/playlist/BaseRepresentation.cpp | 5 ++++-
> > .../adaptive/playlist/BaseRepresentation.h | 5 ++++-
> > modules/demux/hls/playlist/Parser.cpp | 17 +++++++++++++----
> > modules/demux/hls/playlist/Parser.hpp | 6 ++++--
> > modules/demux/hls/playlist/Representation.cpp | 7 +++++--
> > modules/demux/hls/playlist/Representation.hpp | 5 ++++-
> > 7 files changed, 38 insertions(+), 15 deletions(-)
> >
> > diff --git a/modules/demux/adaptive/SegmentTracker.cpp b/modules/demux/adaptive/SegmentTracker.cpp
> > index 583d977681..d54dcaf2a3 100644
> > --- a/modules/demux/adaptive/SegmentTracker.cpp
> > +++ b/modules/demux/adaptive/SegmentTracker.cpp
> > @@ -111,7 +111,7 @@ StreamFormat SegmentTracker::getCurrentFormat() const
> > {
> > /* Ensure ephemere content is updated/loaded */
> > if(rep->needsUpdate())
> > - (void) rep->runLocalUpdates(resources, 0, curNumber, false);
> > + (void) rep->runLocalUpdates(resources, 0, curNumber, false, true);
> > return rep->getStreamFormat();
> > }
> > return StreamFormat();
> > @@ -203,7 +203,7 @@ SegmentChunk * SegmentTracker::getNextChunk(bool switch_allowed,
> > bool b_updated = false;
> > /* Ensure ephemere content is updated/loaded */
> > if(rep->needsUpdate())
> > - b_updated = rep->runLocalUpdates(resources, getPlaybackTime(), curNumber, false);
> > + b_updated = rep->runLocalUpdates(resources, getPlaybackTime(), curNumber, false, false);
> >
> > if(prevRep && !rep->consistentSegmentNumber())
> > {
> > @@ -310,7 +310,7 @@ bool SegmentTracker::setPositionByTime(vlc_tick_t time, bool restarted, bool try
> >
> > /* Stream might not have been loaded at all (HLS) or expired */
> > if(rep && rep->needsUpdate() &&
> > - !rep->runLocalUpdates(resources, time, curNumber, false))
> > + !rep->runLocalUpdates(resources, time, curNumber, false, false))
> > {
> > msg_Err(rep->getAdaptationSet()->getPlaylist()->getVLCObject(),
> > "Failed to update Representation %s", rep->getID().str().c_str());
> > @@ -391,7 +391,7 @@ void SegmentTracker::updateSelected()
> > {
> > if(curRepresentation && curRepresentation->needsUpdate())
> > {
> > - curRepresentation->runLocalUpdates(resources, getPlaybackTime(), curNumber, true);
> > + curRepresentation->runLocalUpdates(resources, getPlaybackTime(), curNumber, true, false);
> > curRepresentation->scheduleNextUpdate(curNumber);
> > }
> > }
> > diff --git a/modules/demux/adaptive/playlist/BaseRepresentation.cpp b/modules/demux/adaptive/playlist/BaseRepresentation.cpp
> > index 49b8cfcee0..17d33d6f24 100644
> > --- a/modules/demux/adaptive/playlist/BaseRepresentation.cpp
> > +++ b/modules/demux/adaptive/playlist/BaseRepresentation.cpp
> > @@ -94,7 +94,10 @@ bool BaseRepresentation::needsUpdate() const
> > }
> >
> > bool BaseRepresentation::runLocalUpdates(SharedResources *,
> > - vlc_tick_t, uint64_t, bool)
> > + vlc_tick_t,
> > + uint64_t,
> > + bool,
> > + const bool isFirstPlaylist)
>
> There's not really a need for this to be const but ok. I would however
> drop the parameter name here, and in every other instance where the
> variable is unused (including the header declaration below).
>
> > {
> > return false;
> > }
> > diff --git a/modules/demux/adaptive/playlist/BaseRepresentation.h b/modules/demux/adaptive/playlist/BaseRepresentation.h
> > index 089ede1299..eeaff8fbdb 100644
> > --- a/modules/demux/adaptive/playlist/BaseRepresentation.h
> > +++ b/modules/demux/adaptive/playlist/BaseRepresentation.h
> > @@ -67,7 +67,10 @@ namespace adaptive
> > virtual vlc_tick_t getMinAheadTime (uint64_t) const;
> > virtual bool needsUpdate () const;
> > virtual bool runLocalUpdates (SharedResources *,
> > - vlc_tick_t, uint64_t, bool);
> > + vlc_tick_t,
> > + uint64_t,
> > + bool,
> > + const bool isFirstPlaylist);
> > virtual void scheduleNextUpdate (uint64_t);
> >
> > virtual void debug (vlc_object_t *,int = 0) const;
> > diff --git a/modules/demux/hls/playlist/Parser.cpp b/modules/demux/hls/playlist/Parser.cpp
> > index fddcc9931a..bb72428bb7 100644
> > --- a/modules/demux/hls/playlist/Parser.cpp
> > +++ b/modules/demux/hls/playlist/Parser.cpp
> > @@ -144,7 +144,7 @@ void M3U8Parser::createAndFillRepresentation(vlc_object_t *p_obj, BaseAdaptation
> > Representation *rep = createRepresentation(adaptSet, tag);
> > if(rep)
> > {
> > - parseSegments(p_obj, rep, tagslist);
> > + parseSegments(p_obj, rep, tagslist, true);
> > if(rep->isLive())
> > {
> > /* avoid update playlist immediately */
> > @@ -155,7 +155,9 @@ void M3U8Parser::createAndFillRepresentation(vlc_object_t *p_obj, BaseAdaptation
> > }
> > }
> >
> > -bool M3U8Parser::appendSegmentsFromPlaylistURI(vlc_object_t *p_obj, Representation *rep)
> > +bool M3U8Parser::appendSegmentsFromPlaylistURI(vlc_object_t *p_obj,
> > + Representation *rep,
> > + const bool isFirstPlaylist)
> > {
> > block_t *p_block = Retrieve::HTTP(p_obj, resources->getAuthStorage(),
> > rep->getPlaylistUrl().toString());
> > @@ -167,7 +169,7 @@ bool M3U8Parser::appendSegmentsFromPlaylistURI(vlc_object_t *p_obj, Representati
> > std::list<Tag *> tagslist = parseEntries(substream);
> > vlc_stream_Delete(substream);
> >
> > - parseSegments(p_obj, rep, tagslist);
> > + parseSegments(p_obj, rep, tagslist, isFirstPlaylist);
> >
> > releaseTagsList(tagslist);
> > }
> > @@ -212,7 +214,10 @@ static bool parseEncryption(const AttributesTag *keytag, const Url &playlistUrl,
> > }
> > }
> >
> > -void M3U8Parser::parseSegments(vlc_object_t *, Representation *rep, const std::list<Tag *> &tagslist)
> > +void M3U8Parser::parseSegments(vlc_object_t *,
> > + Representation *rep,
> > + const std::list<Tag *> &tagslist,
> > + const bool isFirstPlaylist)
> > {
> > SegmentList *segmentList = new (std::nothrow) SegmentList(rep);
> >
> > @@ -365,6 +370,10 @@ void M3U8Parser::parseSegments(vlc_object_t *, Representation *rep, const std::l
> >
> > if(rep->isLive())
> > {
> > + if (isFirstPlaylist) {
> > + segmentList->pruneBySegmentNumber(sequenceNumber - 1);
> > + }
> > +
> > rep->getPlaylist()->duration.Set(0);
> > }
> > else if(totalduration > rep->getPlaylist()->duration.Get())
> > diff --git a/modules/demux/hls/playlist/Parser.hpp b/modules/demux/hls/playlist/Parser.hpp
> > index fc0ac6314f..824bfef360 100644
> > --- a/modules/demux/hls/playlist/Parser.hpp
> > +++ b/modules/demux/hls/playlist/Parser.hpp
> > @@ -58,13 +58,15 @@ namespace hls
> > virtual ~M3U8Parser ();
> >
> > M3U8 * parse (vlc_object_t *p_obj, stream_t *p_stream, const std::string &);
> > - bool appendSegmentsFromPlaylistURI(vlc_object_t *, Representation *);
> > + bool appendSegmentsFromPlaylistURI(vlc_object_t *,
> > + Representation *,
> > + const bool isFirstPlaylist);
> >
> > private:
> > Representation * createRepresentation(BaseAdaptationSet *, const AttributesTag *);
> > void createAndFillRepresentation(vlc_object_t *, BaseAdaptationSet *,
> > const AttributesTag *, const std::list<Tag *>&);
> > - void parseSegments(vlc_object_t *, Representation *, const std::list<Tag *>&);
> > + void parseSegments(vlc_object_t *, Representation *, const std::list<Tag *>&, const bool);
> > std::list<Tag *> parseEntries(stream_t *);
> > adaptive::SharedResources *resources;
> > };
> > diff --git a/modules/demux/hls/playlist/Representation.cpp b/modules/demux/hls/playlist/Representation.cpp
> > index 6be82ac9e8..b2cce3f376 100644
> > --- a/modules/demux/hls/playlist/Representation.cpp
> > +++ b/modules/demux/hls/playlist/Representation.cpp
> > @@ -137,14 +137,17 @@ bool Representation::needsUpdate() const
> > }
> >
> > bool Representation::runLocalUpdates(SharedResources *res,
> > - vlc_tick_t, uint64_t, bool)
> > + vlc_tick_t,
> > + uint64_t,
> > + bool,
> > + const bool isFirstPlaylist)
> > {
> > const time_t now = time(NULL);
> > AbstractPlaylist *playlist = getPlaylist();
> > if(!b_loaded || (isLive() && nextUpdateTime < now))
> > {
> > M3U8Parser parser(res);
> > - parser.appendSegmentsFromPlaylistURI(playlist->getVLCObject(), this);
> > + parser.appendSegmentsFromPlaylistURI(playlist->getVLCObject(), this, isFirstPlaylist);
> > b_loaded = true;
> >
> > return true;
> > diff --git a/modules/demux/hls/playlist/Representation.hpp b/modules/demux/hls/playlist/Representation.hpp
> > index 444b2a1481..13f9068881 100644
> > --- a/modules/demux/hls/playlist/Representation.hpp
> > +++ b/modules/demux/hls/playlist/Representation.hpp
> > @@ -51,7 +51,10 @@ namespace hls
> > virtual bool needsUpdate() const; /* reimpl */
> > virtual void debug(vlc_object_t *, int) const; /* reimpl */
> > virtual bool runLocalUpdates(SharedResources *,
> > - vlc_tick_t, uint64_t, bool); /* reimpl */
> > + vlc_tick_t,
> > + uint64_t,
> > + bool,
> > + const bool isFirstPlaylist); /* reimpl */
> > virtual uint64_t translateSegmentNumber(uint64_t, const SegmentInformation *) const; /* reimpl */
> >
> > private:
> > --
> > 2.20.1
> >
> >
> > --
> > Abdullah Alansari,
> >
> > LinkedIn: https://linkedin.com/in/ahimta
> > GitHub: https://github.com/Ahimta
> > Twitter: https://twitter.com/Ahymta
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list