[vlc-devel] [PATCH] picture_pool: do not unconditionally free the pictures

Luca Barbato lu_zero at gentoo.org
Thu Jul 11 11:26:45 CEST 2013


On 07/11/2013 11:15 AM, Rémi Denis-Courmont wrote:
> Then where and how do you think the pictures gets freed?

By the code holding the additional references, that does exactly what it
is supposed to do =)

The whole thing is a mess since the assumption (wrong) is that you call
that function when there aren't any other active user holding a
reference (assuming something went really wrong and whoever had that
additional reference died an horrible and dirty death).

That's not the case and the whole assumption looks wrong from start to me.

> I don't disagree that overloading the picture reference count and
> destroy callback is ugly, but you can't just remove it. Instead you
> should assert that the picture is not referenced.

The picture are referenced, twice. One by the vout, one by the threaded
decoder. The vout is being closed and reopened, the decoder left
churning frames. Calling that function as it is pulls the pictures from
the decoder hands, thus causing a double free at least.

> That will possibly expose hidden bugs though. I am not volunteering
> to rewrite or fix the video output core, so I won't do that myself.

Understandable.

> Either the decoder and the filter chains should be flushed, or the
> picture pools should deal with pictures outliving their pools. The
> latter is less prone to deadlocks but it would require some changes
> to the video output plugins, especially the ones with "invalid"
> pictures.

> But regardless, I don't think your "fix" works any better than what
> we have now.

My fix at least for a number of valid samples prevents a crash or so it
appears =)

>> Looks like there is more stuff behaving unexpectedly.
> 
> I don't know what your problem is, so I can't say.

If you have time we can discuss, you have surely better insights than me
on this part of the code.

lu



More information about the vlc-devel mailing list