On Sun, 14 Apr 2013, Diego Biurrun wrote:
LGTM in general, mostly minor comments below.
On Wed, Apr 10, 2013 at 07:24:31PM +0300, Martin Storsjö wrote:
--- a/libavcodec/hpeldsp.c
+++ b/libavcodec/hpeldsp.c
@@ -54,5 +54,6 @@ av_cold void ff_hpeldsp_init(HpelDSPContext *c, int flags)
hpel_funcs(avg, [3], 2);
hpel_funcs(avg_no_rnd,, 16);
+ if (ARCH_PPC) ff_hpeldsp_init_ppc (c, flags);
if (ARCH_X86) ff_hpeldsp_init_x86 (c, flags);
Break the line and eliminate the odd spacing please.
Fixed locally already (and should have been part of the branch on github I
showed you).
--- a/libavcodec/hpeldsp.h
+++ b/libavcodec/hpeldsp.h
@@ -94,6 +94,7 @@ typedef struct HpelDSPContext {
void ff_hpeldsp_init(HpelDSPContext *c, int flags);
+void ff_hpeldsp_init_ppc(HpelDSPContext* c, int flags);
void ff_hpeldsp_init_x86(HpelDSPContext* c, int flags);
*c
Also fixed locally already
--- a/libavcodec/ppc/dsputil_altivec.c
+++ b/libavcodec/ppc/dsputil_altivec.c
@@ -1367,16 +956,6 @@ av_cold void ff_dsputil_init_altivec(DSPContext *c,
AVCodecContext *avctx)
if (!high_bit_depth) {
c->get_pixels = get_pixels_altivec;
c->clear_block = clear_block_altivec;
- c->put_pixels_tab[0][0] = ff_put_pixels16_altivec;
- /* the two functions do the same thing, so use the same code */
- c->put_no_rnd_pixels_tab[0][0] = ff_put_pixels16_altivec;
- c->avg_pixels_tab[0][0] = ff_avg_pixels16_altivec;
- c->avg_pixels_tab[1][0] = avg_pixels8_altivec;
- c->avg_pixels_tab[1][3] = avg_pixels8_xy2_altivec;
- c->put_pixels_tab[1][3] = put_pixels8_xy2_altivec;
- c->put_no_rnd_pixels_tab[1][3] = put_no_rnd_pixels8_xy2_altivec;
- c->put_pixels_tab[0][3] = put_pixels16_xy2_altivec;
- c->put_no_rnd_pixels_tab[0][3] = put_no_rnd_pixels16_xy2_altivec;
The comment is dropped; not sure if we need to care.
Not if we reorder the lines as you suggested below
--- /dev/null
+++ b/libavcodec/ppc/hpeldsp_altivec.c
@@ -0,0 +1,464 @@
+
+void ff_hpeldsp_init_ppc(HpelDSPContext* c, int flags)
*c
Fixed locally already
+{
+#if HAVE_ALTIVEC
+ int mm_flags = av_get_cpu_flags();
+
+ if (mm_flags & AV_CPU_FLAG_ALTIVEC) {
+ c->put_pixels_tab[0][0] = ff_put_pixels16_altivec;
+ c->put_no_rnd_pixels_tab[0][0] = ff_put_pixels16_altivec;
+ c->avg_pixels_tab[0][0] = ff_avg_pixels16_altivec;
+ c->avg_pixels_tab[1][0] = avg_pixels8_altivec;
+ c->avg_pixels_tab[1][3] = avg_pixels8_xy2_altivec;
+ c->put_pixels_tab[1][3] = put_pixels8_xy2_altivec;
+ c->put_no_rnd_pixels_tab[1][3] = put_no_rnd_pixels8_xy2_altivec;
+ c->put_pixels_tab[0][3] = put_pixels16_xy2_altivec;
+ c->put_no_rnd_pixels_tab[0][3] = put_no_rnd_pixels16_xy2_altivec;
I think the following would be more readable, with or without the
empty lines:
c->avg_pixels_tab[0][0] = ff_avg_pixels16_altivec;
c->avg_pixels_tab[1][0] = avg_pixels8_altivec;
c->avg_pixels_tab[1][3] = avg_pixels8_xy2_altivec;
c->put_pixels_tab[0][0] = ff_put_pixels16_altivec;
c->put_pixels_tab[0][3] = put_pixels16_xy2_altivec;
c->put_pixels_tab[1][3] = put_pixels8_xy2_altivec;
c->put_no_rnd_pixels_tab[0][0] = ff_put_pixels16_altivec;
c->put_no_rnd_pixels_tab[0][3] = put_no_rnd_pixels16_xy2_altivec;
c->put_no_rnd_pixels_tab[1][3] = put_no_rnd_pixels8_xy2_altivec;
Indeed, that does look more straightforward - reordered.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
