Thread: [Mlt-devel] [PATCH] compositing optimization
Brought to you by:
ddennedy,
lilo_booter
From: Maksym V. <ve...@m1...> - 2015-01-29 16:14:46
|
Hi, provided patch is a second attempt to introduce subj. thees pathset makes two things: 1. disable creating alpha channel for frame if it does not exist 2. implement 8 variants of blending/compositing function: 0| dest_a == NULL | src_a == NULL | weight == 256 | blit 1| dest_a == NULL | src_a == NULL | weight != 256 | blend: with given alpha 2| dest_a == NULL | src_a != NULL | weight == 256 | blend: only src alpha 3| dest_a == NULL | src_a != NULL | weight != 256 | blend: premultiply src alpha 4| dest_a != NULL | src_a == NULL | weight == 256 | blit: blit and set dst alpha to FF 5| dest_a != NULL | src_a == NULL | weight != 256 | blend: with given alpha 6| dest_a != NULL | src_a != NULL | weight == 256 | blend: full blend without src alpha premutiply 7| dest_a != NULL | src_a != NULL | weight != 256 | blend: full (origin version) i found a serious boost of performance after applying that patchset. please review. -- ________________________________________ Maksym Veremeyenko |
From: Dan D. <da...@de...> - 2015-02-03 05:03:15
|
On Thu, Jan 29, 2015 at 8:14 AM, Maksym Veremeyenko <ve...@m1...> wrote: > Hi, > > provided patch is a second attempt to introduce subj. > > thees pathset makes two things: > > 1. disable creating alpha channel for frame if it does not exist > > 2. implement 8 variants of blending/compositing function: > > 0| dest_a == NULL | src_a == NULL | weight == 256 | blit > 1| dest_a == NULL | src_a == NULL | weight != 256 | blend: with given alpha > 2| dest_a == NULL | src_a != NULL | weight == 256 | blend: only src alpha > 3| dest_a == NULL | src_a != NULL | weight != 256 | blend: premultiply src > alpha > 4| dest_a != NULL | src_a == NULL | weight == 256 | blit: blit and set dst > alpha to FF > 5| dest_a != NULL | src_a == NULL | weight != 256 | blend: with given alpha > 6| dest_a != NULL | src_a != NULL | weight == 256 | blend: full blend > without src alpha premutiply > 7| dest_a != NULL | src_a != NULL | weight != 256 | blend: full (origin > version) > > i found a serious boost of performance after applying that patchset. > > please review. > > With the last patch applied, a couple of things in demo/ are segfaulting in composite_line_yuv_sse2_simple.c:blend_case7(). See demo/mlt_news and mlt_title_over_gfx. Also, I want to change the name of mlt_frame_get_alpha_mask_nc() to simply mlt_frame_get_alpha(). Thanks |
From: Maksym V. <ve...@m1...> - 2015-02-03 11:07:07
|
03.02.15 07:03, Dan Dennedy написав(ла): > On Thu, Jan 29, 2015 at 8:14 AM, Maksym Veremeyenko <ve...@m1... > <mailto:ve...@m1...>> wrote: > > Hi, > > provided patch is a second attempt to introduce subj. > > thees pathset makes two things: > > 1. disable creating alpha channel for frame if it does not exist > > 2. implement 8 variants of blending/compositing function: > > 0| dest_a == NULL | src_a == NULL | weight == 256 | blit > 1| dest_a == NULL | src_a == NULL | weight != 256 | blend: with > given alpha > 2| dest_a == NULL | src_a != NULL | weight == 256 | blend: only src > alpha > 3| dest_a == NULL | src_a != NULL | weight != 256 | blend: > premultiply src alpha > 4| dest_a != NULL | src_a == NULL | weight == 256 | blit: blit and > set dst alpha to FF > 5| dest_a != NULL | src_a == NULL | weight != 256 | blend: with > given alpha > 6| dest_a != NULL | src_a != NULL | weight == 256 | blend: full > blend without src alpha premutiply > 7| dest_a != NULL | src_a != NULL | weight != 256 | blend: full > (origin version) > > i found a serious boost of performance after applying that patchset. > > please review. > > > With the last patch applied, a couple of things in demo/ are segfaulting > in composite_line_yuv_sse2_simple.c:blend_case7(). See demo/mlt_news and > mlt_title_over_gfx. i had a typo with jnz instruction that jump from blend_case6 into the blend_case7 func. > Also, I want to change the name of mlt_frame_get_alpha_mask_nc() to > simply mlt_frame_get_alpha(). done please review updated patches PS there are other places where returned value of mlt_frame_get_alpha_mask been checked for NULL, like: filter_rescale.c: <------>uint8_t *input = mlt_frame_get_alpha_mask( frame ); <------>if ( input != NULL ) <------>{ <------><------> filter_avcolour_space.c: <------><------><------>uint8_t *alpha = mlt_frame_get_alpha_mask( frame ); <------><------><------>mlt_properties_get_data( properties, "alpha", &alpha_size ); <------><------><------>if ( alpha && alpha_size >= len ) should we use mlt_frame_get_alpha there instead of mlt_frame_get_alpha_mask in next rework steps? -- ________________________________________ Maksym Veremeyenko |
From: Maksym V. <ve...@m1...> - 2015-02-03 13:58:22
|
03.02.15 13:06, Maksym Veremeyenko написав(ла): [...] > PS > > there are other places where returned value of mlt_frame_get_alpha_mask > been checked for NULL, like: > > > filter_rescale.c: > > <------>uint8_t *input = mlt_frame_get_alpha_mask( frame ); > > <------>if ( input != NULL ) > <------>{ > <------><------> > > > filter_avcolour_space.c: > > <------><------><------>uint8_t *alpha = mlt_frame_get_alpha_mask( frame ); > <------><------><------>mlt_properties_get_data( properties, "alpha", > &alpha_size ); > > <------><------><------>if ( alpha && alpha_size >= len ) > > should we use mlt_frame_get_alpha there instead of > mlt_frame_get_alpha_mask in next rework steps? > i also attached patch that start using mlt_frame_get_alpha for some cases... -- ________________________________________ Maksym Veremeyenko |
From: Dan D. <da...@de...> - 2015-02-04 05:09:03
|
On Tue, Feb 3, 2015 at 3:06 AM, Maksym Veremeyenko <ve...@m1...> wrote: > 03.02.15 07:03, Dan Dennedy написав(ла): > >> >> With the last patch applied, a couple of things in demo/ are segfaulting >> in composite_line_yuv_sse2_simple.c:blend_case7(). See demo/mlt_news and >> mlt_title_over_gfx. >> > i had a typo with jnz instruction that jump from blend_case6 into the > blend_case7 func. > > Also, I want to change the name of mlt_frame_get_alpha_mask_nc() to >> simply mlt_frame_get_alpha(). >> > done > > please review updated patches > > These are applied. Thank you. -- +-DRD-+ |
From: Maksym V. <ve...@m1...> - 2015-02-04 17:29:59
|
04.02.15 07:08, Dan Dennedy написав(ла): > On Tue, Feb 3, 2015 at 3:06 AM, Maksym Veremeyenko <ve...@m1... > <mailto:ve...@m1...>> wrote: > > 03.02.15 07:03, Dan Dennedy написав(ла): > > > With the last patch applied, a couple of things in demo/ are > segfaulting > in composite_line_yuv_sse2___simple.c:blend_case7(). See > demo/mlt_news and > mlt_title_over_gfx. > > i had a typo with jnz instruction that jump from blend_case6 into > the blend_case7 func. > > Also, I want to change the name of mlt_frame_get_alpha_mask_nc() to > simply mlt_frame_get_alpha(). > > done > > please review updated patches > > > These are applied. Thank you. thanks. i also had a question about composite transition operator property. there are some functions: composite_line_yuv_xor composite_line_yuv_and composite_line_yuv_or that will be used for handling appropriate methods, so the question: are they usable? -- ________________________________________ Maksym Veremeyenko |
From: Dan D. <da...@de...> - 2015-02-04 17:33:18
|
On Wed, Feb 4, 2015 at 9:29 AM, Maksym Veremeyenko <ve...@m1...> wrote: > 04.02.15 07:08, Dan Dennedy написав(ла): > >> On Tue, Feb 3, 2015 at 3:06 AM, Maksym Veremeyenko <ve...@m1... >> <mailto:ve...@m1...>> wrote: >> >> 03.02.15 07:03, Dan Dennedy написав(ла): >> >> >> With the last patch applied, a couple of things in demo/ are >> segfaulting >> in composite_line_yuv_sse2___simple.c:blend_case7(). See >> demo/mlt_news and >> mlt_title_over_gfx. >> >> i had a typo with jnz instruction that jump from blend_case6 into >> the blend_case7 func. >> >> Also, I want to change the name of mlt_frame_get_alpha_mask_nc() >> to >> simply mlt_frame_get_alpha(). >> >> done >> >> please review updated patches >> >> >> These are applied. Thank you. >> > > thanks. > > i also had a question about composite transition operator property. there > are some functions: > > composite_line_yuv_xor > composite_line_yuv_and > composite_line_yuv_or > > that will be used for handling appropriate methods, so the question: are > they usable? I have not found them useful and from brief feedback over the years, I do not think many others find them useful either. I think Charlie added them because he could. In other words, I do not see much value in optimizing them. I hope that answers your question. -- +-DRD-+ |
From: Maksym V. <ve...@m1...> - 2015-02-04 17:35:55
|
04.02.15 19:33, Dan Dennedy написав(ла): > On Wed, Feb 4, 2015 at 9:29 AM, Maksym Veremeyenko <ve...@m1... > <mailto:ve...@m1...>> wrote: > > 04.02.15 07:08, Dan Dennedy написав(ла): > > On Tue, Feb 3, 2015 at 3:06 AM, Maksym Veremeyenko > <ve...@m1... <mailto:ve...@m1...> > <mailto:ve...@m1... <mailto:ve...@m1...>>> wrote: > > 03.02.15 07:03, Dan Dennedy написав(ла): > > > With the last patch applied, a couple of things in > demo/ are > segfaulting > in composite_line_yuv_sse2_____simple.c:blend_case7(). See > demo/mlt_news and > mlt_title_over_gfx. > > i had a typo with jnz instruction that jump from > blend_case6 into > the blend_case7 func. > > Also, I want to change the name of > mlt_frame_get_alpha_mask_nc() to > simply mlt_frame_get_alpha(). > > done > > please review updated patches > > > These are applied. Thank you. > > > thanks. > > i also had a question about composite transition operator property. > there are some functions: > > composite_line_yuv_xor > composite_line_yuv_and > composite_line_yuv_or > > that will be used for handling appropriate methods, so the question: > are they usable? > > > > I have not found them useful and from brief feedback over the years, I > do not think many others find them useful either. I think Charlie added > them because he could. In other words, I do not see much value in > optimizing them. I hope that answers your question. may i prepare patch from dropping it? -- ________________________________________ Maksym Veremeyenko |
From: Dan D. <da...@de...> - 2015-02-04 18:10:50
|
On Wed, Feb 4, 2015 at 9:35 AM, Maksym Veremeyenko <ve...@m1...> wrote: > 04.02.15 19:33, Dan Dennedy написав(ла): > >> On Wed, Feb 4, 2015 at 9:29 AM, Maksym Veremeyenko <ve...@m1... >> <mailto:ve...@m1...>> wrote: >> >> 04.02.15 07:08, Dan Dennedy написав(ла): >> >> On Tue, Feb 3, 2015 at 3:06 AM, Maksym Veremeyenko >> <ve...@m1... <mailto:ve...@m1...> >> <mailto:ve...@m1... <mailto:ve...@m1...>>> wrote: >> >> 03.02.15 07:03, Dan Dennedy написав(ла): >> >> >> With the last patch applied, a couple of things in >> demo/ are >> segfaulting >> in composite_line_yuv_sse2_____simple.c:blend_case7(). >> See >> demo/mlt_news and >> mlt_title_over_gfx. >> >> i had a typo with jnz instruction that jump from >> blend_case6 into >> the blend_case7 func. >> >> Also, I want to change the name of >> mlt_frame_get_alpha_mask_nc() to >> simply mlt_frame_get_alpha(). >> >> done >> >> please review updated patches >> >> >> These are applied. Thank you. >> >> >> thanks. >> >> i also had a question about composite transition operator property. >> there are some functions: >> >> composite_line_yuv_xor >> composite_line_yuv_and >> composite_line_yuv_or >> >> that will be used for handling appropriate methods, so the question: >> are they usable? >> >> >> >> I have not found them useful and from brief feedback over the years, I >> do not think many others find them useful either. I think Charlie added >> them because he could. In other words, I do not see much value in >> optimizing them. I hope that answers your question. >> > > may i prepare patch from dropping it? > > No, but you may add a composite2 (or yuvcomposite or composite_lite) transition that has less features and is easier to maintain. -- +-DRD-+ |