On Thu, Jun 26, 2014 at 1:43 AM, Maksym Veremeyenko <ve...@m1stereo.tv> wrote:
> 26.06.14 09:31, Dan Dennedy написав(ла):
>
>> On Tue, Jun 24, 2014 at 11:36 PM, Maksym Veremeyenko<ve...@m1stereo.tv>
>> wrote:
>>>
>>> Hi,
>>>
>>> attached patch implement *matte* transition. it perform replacing alpha
>>> channel of track A with luma of track B. Luma of track B will be scaled
>>> if
>>> it required.
>>
>>
>> Hi Maksym, excellent contribution!
>> Have you seen the shape filter in the vmfx directory? It basically
>> does something similar when you set use_luminance=1, but it seems to
>> have been designed for some more odd and silly use cases whereas your
>> transition simply satisfies a more common need.
>>
>> Some comments.
>> +/*
>> + * transition_luma.c -- a generic dissolve/wipe processor
>>
>> Fix the file name and description
>
> done
>
>
>>
>> + * Copyright (C) 2003-2014 Ushodaya Enterprises Limited
>> + * Author: Dan Dennedy<d...@dennedy.org>
>>
>> Make yourself the author!
>
> replaced only author string, copyright remain the same
>
>
>>
>> + * Adapted from Kino Plugin Timfx, which is
>> + * Copyright (C) 2002 Timothy M. Shead<tsh...@k-3d.com>
>>
>> Rremove those lines as they do not apply.
>>
>> +#include "transition_composite.h"
>>
>> Remove that include.
>>
>> +// mlt_service_lock( MLT_TRANSITION_SERVICE( transition ) );
>> ...
>> +// mlt_service_unlock( MLT_TRANSITION_SERVICE( transition ) );
>>
>> Services need to be locked if they modify the state of the service
>> (transition, here) because in parallel processing there can be
>> multiple threads in the same get_image function. In your case, you are
>> not modifying state. So, remove those lines, which leads to:
>>
>> + // Get the properties of the transition
>> + mlt_properties properties = MLT_TRANSITION_PROPERTIES( transition );
>>
>> You never use this properties! You can remove those lines. And..
>>
>> + // Get the transition object
>> + mlt_transition transition = mlt_frame_pop_service( a_frame );
>>
>> You can remove those lines and the corresponding
>> mlt_frame_push_service() in your process function.
>>
>> +// if ( b_frame == NULL )
>> +// {
>> +//
>> +// }
>> + if ( b_frame != NULL )
>> + {
>>
>> You push the b_frame from your process function. In the process
>> function, you will always get a valid b_frame. Those lines can be
>> removed and code shifted to left when removing the block braces.
>>
> done
>
>
>> +++ b/src/modules/core/transition_matte.yml
>> @@ -0,0 +1,12 @@
>> +schema_version: 0.1
>> +type: transition
>> +identifier: matte
>> +title: Matte
>> +version: 1
>> +copyright: Ushodaya Enterprises Limited
>> +creator: Dan Dennedy
>>
>> Again, make yourself the author.
>
> replaced only author string, copyright remain the same
>
>
>>
>> +license: LGPLv2.1
>> +language: en
>> +tags:
>> +  - Video
>> +description: Replace alphachannel with data from luma channel of other
>> track.
>>
>> Please document the full_luma property under a "parameters:" section.
>>
> i added more explanation and ffmpeg calls examples.
>

OK, still some minor problems. In transition_matte_init(), you make
property "factory" but that is never used. Remove it.

The bigger problems are now in the yml file. Why did you not properly
document the full_luma property using the yaml schema? All you have to
do is look at some other yml files to see how it is done. Then,
consult framework/metaschema.yaml for reference if needed. Also, there
are simple grammatical errors. If you feel your English is not strong,
just let me know. Then, there is "FILL and KEY (MATTE) files." I
understand what you mean, but that really needs to be expressed in MLT
terms; otherwise, people ask "how do I give this my fill and key
files?" You can probably just leave that out.

I do not know how much I like having the ffmpeg examples in there. If
MLT is so great, why are we telling people to run a different program
instead of melt, especially one whose command line syntax changes?
frei0r.alpha0ps can convert alpha channel to grayscale. The challenge
is to understand its parameters due to frei0r's API leading authors
into mapping enumerations into the [0, 1] range. :-\

melt sg_gm_2013_clip_title.avi -attach frei0r.alpha0ps 0=0.21

I think to make a full range:

melt sg_gm_2013_clip_title.avi -attach frei0r.alpha0ps 0=0.21
-consumer avformat:test.mp4 mlt_image_format=rgb24a pix_fmt=yuvj422p
crf=10 preset=placebo an=1

-- 
+-DRD-+

------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Mlt-devel mailing list
Mlt-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mlt-devel

Reply via email to