[vlc-devel] [PATCH] demux: hls: fix consistent live delay (#21614 WIP2)
Tristan Matthews
tmatth at videolan.org
Wed Oct 2 16:18:45 CEST 2019
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
More information about the vlc-devel
mailing list