Quoting Diego Biurrun (2016-08-25 10:43:03) > On Thu, Aug 25, 2016 at 10:40:43AM +0200, Anton Khirnov wrote: > > Quoting Diego Biurrun (2016-08-25 10:36:31) > > > On Thu, Aug 25, 2016 at 10:07:03AM +0200, Anton Khirnov wrote: > > > > Quoting Vittorio Giovara (2016-08-24 16:58:12) > > > > > On Wed, Aug 24, 2016 at 6:06 AM, Diego Biurrun <[email protected]> > > > > > wrote: > > > > > > On Wed, Aug 24, 2016 at 11:24:01AM +0200, Anton Khirnov wrote: > > > > > >> --- /dev/null > > > > > >> +++ b/libavutil/x86/imgutils_init.c > > > > > >> @@ -0,0 +1,49 @@ > > > > > >> + > > > > > >> +void ff_image_copy_plane_uc_from_sse4(uint8_t *dst, ptrdiff_t > > > > > >> dst_linesize, > > > > > >> + const uint8_t *src, > > > > > >> ptrdiff_t src_linesize, > > > > > >> + ptrdiff_t bytewidth, int > > > > > >> height); > > > > > >> + > > > > > >> +int ff_image_copy_plane_uc_from_x86(uint8_t *dst, ptrdiff_t > > > > > >> dst_linesize, > > > > > >> + const uint8_t *src, ptrdiff_t > > > > > >> src_linesize, > > > > > >> + ptrdiff_t bytewidth, int > > > > > >> height) > > > > > >> +{ > > > > > >> + int cpu_flags = av_get_cpu_flags(); > > > > > >> + ptrdiff_t bw_aligned = FFALIGN(bytewidth, 64); > > > > > >> + > > > > > >> + if (EXTERNAL_SSE4(cpu_flags) && > > > > > >> + bw_aligned <= dst_linesize && bw_aligned <= src_linesize) > > > > > >> + ff_image_copy_plane_uc_from_sse4(dst, dst_linesize, src, > > > > > >> src_linesize, > > > > > >> + bw_aligned, height); > > > > > >> + else > > > > > >> + return AVERROR(ENOSYS); > > > > > >> + > > > > > >> + return 0; > > > > > >> +} > > > > > >> --- a/libavutil/imgutils.c > > > > > >> +++ b/libavutil/imgutils.c > > > > > >> @@ -265,9 +266,33 @@ void av_image_copy_plane(uint8_t *dst, > > > > > >> int dst_linesize, > > > > > >> +static void image_copy_plane_uc_from(uint8_t *dst, > > > > > >> ptrdiff_t dst_linesize, > > > > > >> + const uint8_t *src, > > > > > >> ptrdiff_t src_linesize, > > > > > >> + ptrdiff_t bytewidth, int > > > > > >> height) > > > > > >> +{ > > > > > >> + int ret = -1; > > > > > >> + > > > > > >> +#if ARCH_X86 > > > > > >> + ret = ff_image_copy_plane_uc_from_x86(dst, dst_linesize, src, > > > > > >> src_linesize, > > > > > >> + bytewidth, height); > > > > > >> +#endif > > > > > >> + > > > > > >> + if (ret < 0) > > > > > >> + image_copy_plane(dst, dst_linesize, src, src_linesize, > > > > > >> bytewidth, height); > > > > > >> +} > > > > > > > > > > > > This feels awkward. Why not make it into some dsp-like structure > > > > > > with > > > > > > overloading function pointers? > > > > > > > > > > +1 > > > > > > > > How is this more awkward than doing almost the exact same thing in the > > > > dsp > > > > context init function? > > > > > > > > My first approach actually did add a new dsp context, but it turned out > > > > that it's just a lot of extra boilerplat code for no real reason. > > > > > > You need an extra ifdef, you need to check errors, it's not quite the > > > same solution applied to a pattern we have all over the place already. > > > > It's not an "extra" ifdef, your suggestion would just move it to the > > init function. But it would still be there. > > No, the init function just needs the EXTERNAL_SSE4 macro that is already > there.
The x86 init function needs that macro. But the generic init function still needs to call the x86 init function, which will not be present on non-x86 architectures. So you need either an ifdef, or a plain if that depends on DCE. Which is exactly the same as what is there now. > > > The error check would also still need to be present. > > No, you'd just call the function pointer w/o error check. That would require the context to be initialized for specific hardcoded widths/linesizes, which would make it even more annoying to work with. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
