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

[Zhao Zhili]

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.