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 <di...@biurrun.de> 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 error check would also still need to be present.
No, you'd just call the function pointer w/o error check.
Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
