[vlc-devel] [PATCH 4/7] playlist: cancel preparsing upon playback

[Filip Roséen]

Hi Rémi,

On 2017-03-29 22:46, Rémi Denis-Courmont wrote:

> Le keskiviikkona 29. maaliskuuta 2017, 21.38.43 EEST Filip Roséen a écrit :
> > Hi Rémi,
> > 
> > On 2017-03-29 22:13, Rémi Denis-Courmont wrote:
> > > Le perjantaina 24. maaliskuuta 2017, 3.28.31 EEST Filip Roséen a écrit :
> > > > This will cancel any pending request for preparsing the relevant
> > > > 
> > > > playlist_item_t as preparsing the entity:
> > > >  - is redundant since we are about to start playback,
> > > 
> > > Yes but I don´t see how this prevents the next two problems short of
> > > unwarranted sunny day case timing assumptions.
> > 
> > There are other paths where the problem might arise, but for all of
> > those cases the preparsing does not originate from the relevant code
> > of the below listed files:
> > 
> >  - `src/playlist/thread.c`
> >  - `src/playlist/engine.c`
> >  - `src/playlist/item.c`
> > 
> > What is described in the commit message you are referring to is what
> > it addresses, not cases from outside of the scope of the changes.
> 
> Sure, it addresses those cases. But those look like the sunny day cases of a 
> more general problem, which this does not fix.
> 
> I don´t like to add complexity to conceal a bug instead of fixing it. We have 
> had a LOT of problems with that approach and race conditions in VLC in 
> general, and in the playlist in particular.

I do not think that these changes conceals the bug in any case, it
addresses the issue as it is described. It also does not prevent any
future fixes of the relevant problem.

I have branches that refactor the playlist implementations far more
extensively than what has seen the light of day during the past year
in terms of VLC development; I am however saving that for after
`3.0.0` due to the changes being (as aspected) far more intrusive.

There is nothing in my mind that would ever stand behind hiding a bug
further down in the stack, but as stated I cannot see how the relevant
changes in this patch-batch does that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170329/fb6791e9/attachment.html>