[Kdenlive-devel] [Mlt-devel] different videostab filter

[Dan Dennedy]

On Sun, Sep 11, 2011 at 1:34 AM, Marco Gittler <marco at gitma.de> wrote:
>
> Am 11.09.2011 um 01:29 schrieb Dan Dennedy:
>
>> 2011/9/2 Marco Gittler <marco at gitma.de>:
>>> Hi
>>>
>>> i integrated a second (very nice) videostab from http://public.hronopik.de/vid.stab/features.php?lang=en
>>> which seems to be make a bit better job. (https://github.com/gmarco/mlt/commits/videostab)
>>>
>>> This video http://www.youtube.com/watch?v=cRA5H1LYzM4&feature=mfu_in_order&list=UL
>>> is very shaky and using the filter videostab, the result has still some rolling in the video. Also it has some more "stretch areas" in den borders.
>>>
>>> The videostab2 filter seems to use a complete different algo.
>>> -It has much more parameter to be adjusted (not implemented yet)
>>> -is can handle yuv and rgb data (rgb is _much_ slower tha yuv , yuv nearly as fast as videostab filter )
>>> -is can remove the rolling of the camera much better
>>> -it will zoom in to avoid the "stretch borders"
>>>
>>> This is only a Test version. But i think, it would be worth to have this too in mlt (a bit work is to make it full working (parameters/debug log) )
>>>
>>> Usage of the filter is equal to videostab.
>>
>> You must address the following problems before I will merge it into MLT.
>>
>> 1) Why did you revert my change to uncomment the values.h include in
>> stab/estimate.c? It is needed to get MAXFLOAT; otherwise, it fails to
>> build for me. If you need to comment it out to make it build, then
>> provide a universal fix.
> This was needed because it did not compile on mac. (filter_videostab.c also)
>>
>> 2) Makefile includes flags -mmmx  -msse  -msse2. We cannot just
>> arbitrarily add these flags without respecting the configure --disable
>> options, and there is no reason to add them only for videostab and no
>> other code. Just remove them. Also, make gives 'cc1: error:
>> unrecognized command line option "-fasm-blocks"'
> Tt is not ready to merge now, it is only a quick an dirty test of the algo itself, to see first results.

OK, you can see I am eager to merge it. :-) I did not have a chance
yet to actually use it.

> Makefile must be changed to respect these settings later.

I prefer them not included at all and just let packagers add
optimizations (if they are feeling brave).

>> 3) Why do you always ignore the mlt coding conventions in all of the
>> code that you personally contribute? I am not talking about other
>> peoples' code files that you copy in - that should retain original
>> formatting. For example, in filter_videostab2.c, right in the middle
>> of the code I wrote consistent with the conventions, you put things
>> like
>>    if (dat && dat->data){
>> and
>>    (*format==mlt_image_rgb24?0:1)
>
> This is only a first quick test. i had problems with framesize of yuv420 data.
> The size*3/2 seems to be to small and cause segfault (must be investigated)
>>
>> 4) Remove the commented code that obviously no longer belongs.
>
> This should be all done if the rest works fully.
>>
>> 5) There is no filter_videostab2.yml.
>>
>> 6) This is optional, but I think you should set the interpolation
>> (TransformData.interpoltype) based on the frame property
>> "rescale.interp" See
>> src/modules/avformat/filter_swscale.c:filter_scale() how to interpret
>> that property. By default, the SDL consumers use low quality "nearest"
>> and avformat uses higher quality "bilinear."
>>
>
> This is a good hint. i will try to change these things later (after vacation )

take your time

>> 7) In filter_videostab2_init(), you allocate StabData, but so does
>> stabilize_init() later in the function. I suggest just remove the call
>> the stabilize_init().
>
> there must be al lot of code cleanup, since i was working on get the videostab2 code to work with mlt.
>>
>> 8) In filter_get_image(), you allocate TransformData with
>> mlt_pool_alloc() but tell the property to destroy it with
>> mlt_properties_close - should mlt_pool_release.
>
> Thanks, will be changed also when i'm working on it.
>>
>> 9) There is a printf() in filter_get_image().
>
> This will be changed into mlt_log… also.
>
>
>
> The whole code is still in testing stage and should me an others give a first look.
> After that look if found, that it would be a bit better code to stabilize the video.
> After resolving the last issues and change the code to have full mlt coding conversions, we could merge this later, but not this time.
>
> But very kind of you to find out some fundamental things that must be changed to.
> I will try to change most of this in 1-2 week(s).
> I hope it is also for others to see, that this code coudl be usefull.

I will take some moment to actually use it and give some feedback
about how it compares to first videostab for me.

-- 
+-DRD-+