[FFmpeg-devel] [PATCH 4/4] avfilter/vf_colormatrix: add 10 & 12 bit depth support

[Thomas Mundt]

>>>Kieran Kunhya <kierank at obe.tv> schrieb am So, 27.3.2016:
> On Sun, 27 Mar 2016 at 13:22 Thomas Mundt <loudmax-at-yahoo.de at ffmpeg.org> wrote:
> 
>> Ronald S. Bultje <rsbultje at gmail.com> schrieb am So, 27.3.2016:
>> > On Sun, Mar 27, 2016 at 7:09 AM, Thomas Mundt <loudmax-at-yahoo.de at
>> ffmpeg.org> wrote:
>> >
>> >> >>>Ronald S. Bultje <rsbultje at gmail.com> schrieb am Sa, 26.3.2016:
>> >> > On Sat, Mar 26, 2016 at 10:04 AM, Thomas Mundt <loudmax-at-yahoo.de
>> at
>> >> ffmpeg.org> wrote:
>> >> >
>> >> >>>>>Kieran Kunhya <kierank at obe.tv> schrieb am Sa, 26.3.2016:
>> >> >> >> On Fri, 25 Mar 2016 at 22:32 Thomas Mundt <loudmax-at-yahoo.de
>> at>
>> >> ffmpeg.org> wrote:
>> >> >> >
>> >> >> >> Signed-off-by: Thomas Mundt <loudmax at yahoo.de>
>> >> >> >> ---
>> >> >> >>  libavfilter/vf_colormatrix.c | 182
>> >> >> >> ++++++++++++++++++++++++++++++++++++++++++-
>> >> >> >>
>> >> >>> >
>> >> >> > These functions are basically the same, have you considered
>> factoring
>> >> the
>> >> >> > code out?
>> >> >> >
>> >> >> > Kieran
>> >> >>
>> >> >> I thought keeping it seperate would be easier to review.
>> >> >> But I can do that. Maybe as a subsequent patch?
>> >> >
>> >> >
>> >> > I think he means templating out typelessly like h264/hevc/vp9 do. I
>> agree
>> >> > that would probably be nicer. Binary size is same but less duplicated
>> >> code.
>> >> >
>> >> > Ronald
>> >>
>> >> Okay, I tried to solve this with the use of macros. The attached file is
>> >> the result (interesting part starting at line 203).
>> >> It works, but I´m not sure if this is what you expect.
>> >> Also I didn´t find a way factoring out the two different av_clip.
>> >> One could replace av_clip_uint8 by av_clip with limits 0 and 255. But
>> this
>> >> would have an impact on speed.
>> >> If there is a more elegant way, could you please provide a short
>> example?!
>> >
>> >
>> > We don't like macros, but you're surprisingly close. Have a look at
>> > libavcodec/bit_depth_template.c and grep for its usage in libavcodec.
>> >
>> > You'll end up with a vf_colormatrixdsp_template.c file, which includes
>> > bit_depth_template.c for types/clips etc., and it will then define full
>> > functions per bit depth. This is included in vf_colormatrixdsp.c, and
>> that
>> > defines the actual dsp functions which do the core of what this filter
>> does.
>> >
>> > Since it's now in a dsp file, it'll be trivial to write x86 simd for it
>> > (which I have also in the works).
>> >
>> > Ronald
>>
>> I looked into this and sadly have to say that this would take much more
>> time than I have atm.
>> I´m not a professional programmer so I would have to fiddle out the most
>> things.
>> Sorry, but then I think it´s maybe better to drop the high bit depht
>> support patch for now.
>>
> 
> This may help:
> http://blog.pkh.me/p/20-templating-in-c.html
> 
> Kieran

Okay, thanks. Maybe I´ll find some time tonight...