[vlc-devel] [PATCHES] -- fix marquee failing to format input-dependent text ($N, $...)

[Rémi Denis-Courmont]

Le samedi 27 avril 2013 14:51:40, brezhoneg1 a écrit :
> Le 27/04/2013 07:48, Rémi Denis-Courmont a écrit :
> > Le samedi 27 avril 2013 03:21:51, brezhoneg1 a écrit :
> >> Please, review it and tell me if you see it as an
> >> acceptable fix for the time being ?
> > 
> > Holding an input thread with the input resource makes absolutely no
> > sense. An input thread is not reusable - across input items (and even if
> > it were, there is no point reusing it).
> 
> Yes, I know it's a hack, but I wouln't say "it makes absolutely no
> sense",

I would. The whole point of the input resource is to hold resources that 
survive across input threads. Putting the input thread there is as nonsensical 
as it gets: countersensical.

> because it does fix an issue that has been around for a very
> long time.

Adding the input there does not fix anything. It just moves the "problem" from 
finding the input to finding the input resource. Worse yet, the input resource, 
unlike VLC objects, is not designed to be shared in any way.

>   As a matter of fact, the input object is often needed by a lot of
> other objects. Some of them keep a so-called "weak" reference to it
> (access, demux, decoder, ...).

Access and demux does not keep any weak reference. They have a persistent 
reference to their parent input. Using it is unadvisable but it is not weak.

> Others don't.  Filters like marquee are
> so far away from the input that they have lost complete track of it if
> it ever existed in the first place. This input lookup is a real issue
> that a more general design needs to address.
> 
> This patch just tries to find a convenient and unintrusive place to
> retrieve the input for the short term.

Hacking the input resource is nowhere near unintrusive. Besides, the input 
resource is probably the most hairy piece of code in VLC in terms of 
synchronization and timing. Kludging it is entirely unacceptable.

> The resource manager happens to
> keep an updated "weak" reference to it, and unlike the input object, be
> easily reachable for the marquee object through the inheritance
> mechanism in today's dependencies of objects.

This is not even a valid use of variables with regards to object life cycle.

> So why not use it in
> str_format instead of the input provided by the playlist which is a lot
> more restrictive ?

Because it is broken? Antoine could not bothered to implement formatting of 
meta-data cleanly. It had always been partly broken and it finally broke 
completely when Laurent reworked the video output.

Laurent and I have spent countless hours fixing a lot of these design mistakes 
and too many corresponding bugs to keep track of. Nobody forces you to adhere 
to our coding standards, but that is the only way to get your code upstream. 
That is one of the most basic principles of OSS communities.

-- 
Rémi Denis-Courmont
http://www.remlab.net/