gst-plugins-good: videocrop mark crop properties as mutable in playing state

[Nicolas Dufresne]

Le mardi 24 mai 2016 à 10:03 +0100, Tim-Philipp Müller a écrit :
> On Mon, 2016-05-23 at 20:24 -0400, Nicolas Dufresne wrote:
> 
> > 
> > > 
> > >  
> > > videocrop mark crop properties as mutable in playing state
> > > 
> > Last time I tested, changing those in playing state lead to a crash
> > or spurious negotiation error. I had to use a probe in front to
> > safely be able to change this.
> It will depend on the pipeline I think? Were the crashes in
> videocrop?
> 
> ./videocrop2-test changes these properties at runtime and that seems
> to
> work fine for me. Not that I'd be surprised if there are bugs given
> the
> async nature of this all.
> 
> I see there's https://bugzilla.gnome.org/show_bug.cgi?id=761163 of
> course, but it's a bit all over the place really - as far as I can
> tell
> vieocrop already tries to queue the changes, but perhaps not in all
> places where it should? 

There is code that tries indeed. But it needs some more work. Basically
it needs to queue the changes until they are applied, while atm they
are applied at multiple places, which can lead to inconstancy (and
sometimes crash). Another issue, is that changing one crop property at
the time lead to bunch of caps renegotiation, to be really useful, we'd
be able to change all of these properties atomically.

> 
> Are you suggesting we don't mark them as mutable because it's not
> necessarily 100% bulletproof in all scenarios even though it mostly
> works fine? I don't really mind, I was just under the impression that
> it should work in principle and remembered the test programs we had
> worked fine, and someone asked about it on irc :)

I'm suggesting that marking them as mutable is a bad indication for the
user. As least without, it's clear that it's an untested domain. You
could have fixed the bugs first, that would have been nicer. At the
same time, I suppose this won't crash with controllers, since their
behaviors are similar to using a buffer probe on the sink pad, and
setting the values within that probe callback.

regards,
Nicolas