Re: [Mlt-devel] Mlt::Properties pass_list not triggering property-changed
On Mon, Jul 27, 2020 at 6:15 PM Dan Dennedy wrote: > JB, I want to revert your change here temporarily and make a release. It > is time to make a new release, but this change is too fundamental to > include in a release right now. Then, I would restore the change after > release. Do you agree? > > I did not get an answer, so I went ahead and reverted. After a successful nightly build, I will make the next release by the end of July and restore your change. > > On Fri, Jul 24, 2020 at 7:48 PM Dan Dennedy wrote: > >> On Fri, Jul 24, 2020 at 1:36 PM jb wrote: >> >>> Hi all, >>> >>> Looking at a timewarp producer issue in Kdenlive, I noticed that the MLT >>> properties method "mlt_properties_pass_list" does not trigger a >>> "property- >>> changed" event. >>> >>> In Kdenlive, we use this "mlt_properties_pass_list" method to pass some >>> properties on the producer. However, since the "property-changed" event >>> is not >>> triggered, the properties are not applied on the Timewarp producer that >>> applies changes to the original avformat producer on the >>> "property-changed" >>> event. >>> >>> Is there any reason to not trigger a property-changed event in >>> mlt_properties_pass_list ? >>> >> >> I do not recall there being a reason to not fire this event, and I do not >> see anything in git log on this file about removing it. It is most likely >> an oversight in the contributor's submission and review. As you can see >> from the code comment and git blame it came from an infrequent contributor. >> We can add this change and then if something is negatively affected try >> blocking events in that area. >> >> >>> >>> If this is meant to be, I can change Kdenlive to use a property set >>> method, >>> but otherwise, it seems reasonable to fix it with this 1 line patch : >>> >>> >>> >>> diff --git a/src/framework/mlt_properties.c >>> b/src/framework/mlt_properties.c >>> index 94afe45f..f3e419b5 100644 >>> --- a/src/framework/mlt_properties.c >>> +++ b/src/framework/mlt_properties.c >>> @@ -614,6 +614,7 @@ void mlt_properties_pass_property( mlt_properties >>> self, >>> mlt_properties that, con >>> return; >>> >>> mlt_property_pass( mlt_properties_fetch( self, name ), that_prop >>> ); >>> + mlt_events_fire( self, "property-changed", name, NULL ); >>> } >>> >>> /** Copy all properties specified in a comma-separated list to another >>> properties list. >>> >>> >>> >>> Thanks in advance for your opinion on this. >>> >>> Regards, >>> Jean-Baptiste >>> >>> >>> >>> >>> >>> >>> >>> ___ >>> Mlt-devel mailing list >>> Mlt-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/mlt-devel >>> >> ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
Re: [Mlt-devel] Mlt::Properties pass_list not triggering property-changed
JB, I want to revert your change here temporarily and make a release. It is time to make a new release, but this change is too fundamental to include in a release right now. Then, I would restore the change after release. Do you agree? On Fri, Jul 24, 2020 at 7:48 PM Dan Dennedy wrote: > On Fri, Jul 24, 2020 at 1:36 PM jb wrote: > >> Hi all, >> >> Looking at a timewarp producer issue in Kdenlive, I noticed that the MLT >> properties method "mlt_properties_pass_list" does not trigger a "property- >> changed" event. >> >> In Kdenlive, we use this "mlt_properties_pass_list" method to pass some >> properties on the producer. However, since the "property-changed" event >> is not >> triggered, the properties are not applied on the Timewarp producer that >> applies changes to the original avformat producer on the >> "property-changed" >> event. >> >> Is there any reason to not trigger a property-changed event in >> mlt_properties_pass_list ? >> > > I do not recall there being a reason to not fire this event, and I do not > see anything in git log on this file about removing it. It is most likely > an oversight in the contributor's submission and review. As you can see > from the code comment and git blame it came from an infrequent contributor. > We can add this change and then if something is negatively affected try > blocking events in that area. > > >> >> If this is meant to be, I can change Kdenlive to use a property set >> method, >> but otherwise, it seems reasonable to fix it with this 1 line patch : >> >> >> >> diff --git a/src/framework/mlt_properties.c >> b/src/framework/mlt_properties.c >> index 94afe45f..f3e419b5 100644 >> --- a/src/framework/mlt_properties.c >> +++ b/src/framework/mlt_properties.c >> @@ -614,6 +614,7 @@ void mlt_properties_pass_property( mlt_properties >> self, >> mlt_properties that, con >> return; >> >> mlt_property_pass( mlt_properties_fetch( self, name ), that_prop >> ); >> + mlt_events_fire( self, "property-changed", name, NULL ); >> } >> >> /** Copy all properties specified in a comma-separated list to another >> properties list. >> >> >> >> Thanks in advance for your opinion on this. >> >> Regards, >> Jean-Baptiste >> >> >> >> >> >> >> >> ___ >> Mlt-devel mailing list >> Mlt-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/mlt-devel >> > ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
Re: [Mlt-devel] Mlt::Properties pass_list not triggering property-changed
On Fri, Jul 24, 2020 at 1:36 PM jb wrote: > Hi all, > > Looking at a timewarp producer issue in Kdenlive, I noticed that the MLT > properties method "mlt_properties_pass_list" does not trigger a "property- > changed" event. > > In Kdenlive, we use this "mlt_properties_pass_list" method to pass some > properties on the producer. However, since the "property-changed" event is > not > triggered, the properties are not applied on the Timewarp producer that > applies changes to the original avformat producer on the > "property-changed" > event. > > Is there any reason to not trigger a property-changed event in > mlt_properties_pass_list ? > I do not recall there being a reason to not fire this event, and I do not see anything in git log on this file about removing it. It is most likely an oversight in the contributor's submission and review. As you can see from the code comment and git blame it came from an infrequent contributor. We can add this change and then if something is negatively affected try blocking events in that area. > > If this is meant to be, I can change Kdenlive to use a property set > method, > but otherwise, it seems reasonable to fix it with this 1 line patch : > > > > diff --git a/src/framework/mlt_properties.c > b/src/framework/mlt_properties.c > index 94afe45f..f3e419b5 100644 > --- a/src/framework/mlt_properties.c > +++ b/src/framework/mlt_properties.c > @@ -614,6 +614,7 @@ void mlt_properties_pass_property( mlt_properties > self, > mlt_properties that, con > return; > > mlt_property_pass( mlt_properties_fetch( self, name ), that_prop ); > + mlt_events_fire( self, "property-changed", name, NULL ); > } > > /** Copy all properties specified in a comma-separated list to another > properties list. > > > > Thanks in advance for your opinion on this. > > Regards, > Jean-Baptiste > > > > > > > > ___ > Mlt-devel mailing list > Mlt-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/mlt-devel > ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
[Mlt-devel] Mlt::Properties pass_list not triggering property-changed
Hi all, Looking at a timewarp producer issue in Kdenlive, I noticed that the MLT properties method "mlt_properties_pass_list" does not trigger a "property- changed" event. In Kdenlive, we use this "mlt_properties_pass_list" method to pass some properties on the producer. However, since the "property-changed" event is not triggered, the properties are not applied on the Timewarp producer that applies changes to the original avformat producer on the "property-changed" event. Is there any reason to not trigger a property-changed event in mlt_properties_pass_list ? If this is meant to be, I can change Kdenlive to use a property set method, but otherwise, it seems reasonable to fix it with this 1 line patch : diff --git a/src/framework/mlt_properties.c b/src/framework/mlt_properties.c index 94afe45f..f3e419b5 100644 --- a/src/framework/mlt_properties.c +++ b/src/framework/mlt_properties.c @@ -614,6 +614,7 @@ void mlt_properties_pass_property( mlt_properties self, mlt_properties that, con return; mlt_property_pass( mlt_properties_fetch( self, name ), that_prop ); + mlt_events_fire( self, "property-changed", name, NULL ); } /** Copy all properties specified in a comma-separated list to another properties list. Thanks in advance for your opinion on this. Regards, Jean-Baptiste ___ Mlt-devel mailing list Mlt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mlt-devel
