[vlc-devel] [PATCH] demux: adaptive: workaround successive setPosition failed by flush EsOutAddCommand

[Zhao Zhili]

> On 18 Oct 2017, at 9:51 AM, Zhao Zhili <quinkblack at foxmail.com> wrote:
> 
> Hi Francois,
> 
> 
> On 2017年10月18日 01:08, Francois Cartegnie wrote:
>> Le 07/10/2017 à 08:03, Zhao Zhili a écrit :
>>> ---
>>>  modules/demux/adaptive/Streams.cpp                |  8 ++++++
>>>  modules/demux/adaptive/plumbing/CommandsQueue.cpp | 33 +++++++++++++++++++++++
>>>  modules/demux/adaptive/plumbing/CommandsQueue.hpp |  1 +
>>>  3 files changed, 42 insertions(+)
>>> 
>>> diff --git a/modules/demux/adaptive/Streams.cpp b/modules/demux/adaptive/Streams.cpp
>>> index 96543d3187..d1e97df358 100644
>>> --- a/modules/demux/adaptive/Streams.cpp
>>> +++ b/modules/demux/adaptive/Streams.cpp
>>> @@ -498,7 +498,15 @@ bool AbstractStream::setPosition(mtime_t time, bool tryonly)
>>>              setTimeOffset(segmentTracker->getPlaybackTime());
>>>                if( !restartDemux() )
>>> +            {
>>>                  dead = true;
>>> +            }
>>> +            else
>>> +            {
>>> +                /* To avoid a successive setPosition failed because fakeesout is restarting, we
>>> +                 * flush EsOutAddCommand here. */
>>> +                commandsqueue->flushAddCommand(p_realdemux->out);
>>> +            }
>>>          }
>>>          else commandsqueue->Abort( true );
>> I don't understand the issue.
>> 
>> Since modifying commands queue is pretty risky and can easily break
>> something and debugging then would take multiple hours,
>> that needs a detailed explanation and test case.
>> 
> 
> The issue is like this:
> 
> 1. AbstractStream::seekAble() check whether fakeesout->restarting()
> 
> bool AbstractStream::seekAble() const
> {
>    return (demuxer &&
>            !fakeesout->restarting() &&
>            !discontinuity &&
>            !commandsqueue->isDraining() );
> }
> 
> 2. AbstractStream::setPosition() --> AbstractStream::restartDemux() -->
> fakeesout->recycleAll()
> 
> 3. If AbstractStream::setPosition() is called again before
> FakeESOut::createOrRecycleRealEsID() is called for all es id,
> fakeesout->restarting() is true and AbstractStream::seekAble() is false, so
> AbstractStream::setPosition() is failed.
> 
> 4. It's very easy to reproduce this issue by seek again and again, especially
> for a slow network connection.
> 
> My solution is:
> 
> After AbstractStream::restartDemux() --> demuxer->create(), EsOutAddCommand is
> pushed to CommandsQueue. Flush EsOutAddCommand will force
> FakeESOut::createOrRecycleRealEsID() to be called. So it has a large chance to
> prevent the next AbstractStream::seekAble() check fail.
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


I have rechecked the flow. CommandsQueue::Abort() is called inside
FakeESOut::recycleAll(). So after

AbstractStream::setPosition() --> AbstractStream::restartDemux() --> FakeESOut::recycleAll() --> Demuxer::create()

the commands inside commandsqueue are those commands sent during demuxer
open. Flush EsOutAddCommand should not lead to disorder.