[03/13] dsputil: Make square put/avg_pixels functions local to h264qpel

Message ID 1364995179-98359-3-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö April 3, 2013, 1:19 p.m.
From: "Ronald S. Bultje" <rsbultje@gmail.com>

Put a copy of the 8bit functions only in dsputil, where they are
used for some other things (e.g. mpeg4qpel, mspel, cavsqpel). We
could perhaps also try to share specifically the 8bit functions
from h264qpel between it and the others, but that will be slightly
more complicated. H264qpel already had these functions, so we can
simply remove the duplicates.
---
 libavcodec/dsputil.c           |   15 ++++++++++++++-
 libavcodec/dsputil.h           |   18 ++++--------------
 libavcodec/dsputil_template.c  |   19 ++-----------------
 libavcodec/h264qpel_template.c |   17 -----------------
 4 files changed, 20 insertions(+), 49 deletions(-)

Comments

Luca Barbato April 3, 2013, 1:33 p.m. | #1
On 03/04/13 15:19, Martin Storsjö wrote:
> From: "Ronald S. Bultje" <rsbultje@gmail.com>
> 
> Put a copy of the 8bit functions only in dsputil, where they are
> used for some other things (e.g. mpeg4qpel, mspel, cavsqpel). We
> could perhaps also try to share specifically the 8bit functions
> from h264qpel between it and the others, but that will be slightly
> more complicated. H264qpel already had these functions, so we can
> simply remove the duplicates.

At least a step in a good direction.

lu
Diego Biurrun April 4, 2013, 3:04 p.m. | #2
On Wed, Apr 03, 2013 at 04:19:29PM +0300, Martin Storsjö wrote:
> From: "Ronald S. Bultje" <rsbultje@gmail.com>
> 
> Put a copy of the 8bit functions only in dsputil, where they are
> used for some other things (e.g. mpeg4qpel, mspel, cavsqpel). We
> could perhaps also try to share specifically the 8bit functions
> from h264qpel between it and the others, but that will be slightly
> more complicated. H264qpel already had these functions, so we can
> simply remove the duplicates.

Most of this should be an annotation not part of the log msg.

> --- a/libavcodec/dsputil.c
> +++ b/libavcodec/dsputil.c
> @@ -1281,12 +1281,25 @@ QPEL_MC(0, avg_       , _       , op_avg)
>  #undef op_put
>  #undef op_put_no_rnd
>  
> +void ff_put_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
> +    put_pixels8_8_c(dst, src, stride, 8);
> +}
> +void ff_avg_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
> +    avg_pixels8_8_c(dst, src, stride, 8);
> +}
> +void ff_put_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
> +    put_pixels16_8_c(dst, src, stride, 16);
> +}
> +void ff_avg_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
> +    avg_pixels16_8_c(dst, src, stride, 16);
> +}

Please fix { placement.

> --- a/libavcodec/dsputil_template.c
> +++ b/libavcodec/dsputil_template.c
> @@ -107,7 +107,9 @@ DCTELEM_FUNCS(int16_t, _16)
>  
> +#if BIT_DEPTH == 8
>  #include "hpel_template.c"
> +#endif

I'd rather remove the include from here and move it to dsputil.c, in the
BIT_DEPTH == 8 section.

Diego
Martin Storsjö April 4, 2013, 3:49 p.m. | #3
On Thu, 4 Apr 2013, Diego Biurrun wrote:

> On Wed, Apr 03, 2013 at 04:19:29PM +0300, Martin Storsjö wrote:
>> From: "Ronald S. Bultje" <rsbultje@gmail.com>
>>
>> Put a copy of the 8bit functions only in dsputil, where they are
>> used for some other things (e.g. mpeg4qpel, mspel, cavsqpel). We
>> could perhaps also try to share specifically the 8bit functions
>> from h264qpel between it and the others, but that will be slightly
>> more complicated. H264qpel already had these functions, so we can
>> simply remove the duplicates.
>
> Most of this should be an annotation not part of the log msg.

Fixed

>
>> --- a/libavcodec/dsputil.c
>> +++ b/libavcodec/dsputil.c
>> @@ -1281,12 +1281,25 @@ QPEL_MC(0, avg_       , _       , op_avg)
>>  #undef op_put
>>  #undef op_put_no_rnd
>>
>> +void ff_put_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
>> +    put_pixels8_8_c(dst, src, stride, 8);
>> +}
>> +void ff_avg_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
>> +    avg_pixels8_8_c(dst, src, stride, 8);
>> +}
>> +void ff_put_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
>> +    put_pixels16_8_c(dst, src, stride, 16);
>> +}
>> +void ff_avg_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
>> +    avg_pixels16_8_c(dst, src, stride, 16);
>> +}
>
> Please fix { placement.

Done

>
>> --- a/libavcodec/dsputil_template.c
>> +++ b/libavcodec/dsputil_template.c
>> @@ -107,7 +107,9 @@ DCTELEM_FUNCS(int16_t, _16)
>>
>> +#if BIT_DEPTH == 8
>>  #include "hpel_template.c"
>> +#endif
>
> I'd rather remove the include from here and move it to dsputil.c, in the
> BIT_DEPTH == 8 section.

Done

// Martin
Martin Storsjö April 8, 2013, 10:33 a.m. | #4
On Thu, 4 Apr 2013, Martin Storsjö wrote:

> On Thu, 4 Apr 2013, Diego Biurrun wrote:
>
>> On Wed, Apr 03, 2013 at 04:19:29PM +0300, Martin Storsjö wrote:
>> 
>>> --- a/libavcodec/dsputil_template.c
>>> +++ b/libavcodec/dsputil_template.c
>>> @@ -107,7 +107,9 @@ DCTELEM_FUNCS(int16_t, _16)
>>> 
>>> +#if BIT_DEPTH == 8
>>>  #include "hpel_template.c"
>>> +#endif
>> 
>> I'd rather remove the include from here and move it to dsputil.c, in the
>> BIT_DEPTH == 8 section.
>
> Done

Actually, the way I moved the include to dsputil.c breaks some tests - 
bit_depth_template.c needs to be included again before hpel_template.c. 
I'd rather not start reshuffling those things in this patch. So reverted 
this patch back to the form posted here.

// Martin
Luca Barbato April 8, 2013, 11:29 a.m. | #5
On 08/04/13 12:33, Martin Storsjö wrote:
> On Thu, 4 Apr 2013, Martin Storsjö wrote:
> 
>> On Thu, 4 Apr 2013, Diego Biurrun wrote:
>>
>>> On Wed, Apr 03, 2013 at 04:19:29PM +0300, Martin Storsjö wrote:
>>>
>>>> --- a/libavcodec/dsputil_template.c
>>>> +++ b/libavcodec/dsputil_template.c
>>>> @@ -107,7 +107,9 @@ DCTELEM_FUNCS(int16_t, _16)
>>>>
>>>> +#if BIT_DEPTH == 8
>>>>  #include "hpel_template.c"
>>>> +#endif
>>>
>>> I'd rather remove the include from here and move it to dsputil.c, in the
>>> BIT_DEPTH == 8 section.
>>
>> Done
> 
> Actually, the way I moved the include to dsputil.c breaks some tests -
> bit_depth_template.c needs to be included again before hpel_template.c.
> I'd rather not start reshuffling those things in this patch. So reverted
> this patch back to the form posted here.

We'll reshuffle later. as is it is still a step in a good direction.

lu
Diego Biurrun April 8, 2013, 12:33 p.m. | #6
On 04/08/2013 12:33 PM, Martin Storsjö wrote:
> On Thu, 4 Apr 2013, Martin Storsjö wrote:
>
>> On Thu, 4 Apr 2013, Diego Biurrun wrote:
>>
>>> On Wed, Apr 03, 2013 at 04:19:29PM +0300, Martin Storsjö wrote:
>>>
>>>> --- a/libavcodec/dsputil_template.c
>>>> +++ b/libavcodec/dsputil_template.c
>>>> @@ -107,7 +107,9 @@ DCTELEM_FUNCS(int16_t, _16)
>>>>
>>>> +#if BIT_DEPTH == 8
>>>>  #include "hpel_template.c"
>>>> +#endif
>>>
>>> I'd rather remove the include from here and move it to dsputil.c, in the
>>> BIT_DEPTH == 8 section.
>>
>> Done
>
> Actually, the way I moved the include to dsputil.c breaks some tests -
> bit_depth_template.c needs to be included again before hpel_template.c.
> I'd rather not start reshuffling those things in this patch. So reverted
> this patch back to the form posted here.

I think all you need to do is add a bit_depth_template #include to 
hpel_template.c.

Diego
Martin Storsjö April 8, 2013, 12:45 p.m. | #7
On Mon, 8 Apr 2013, Diego Biurrun wrote:

> On 04/08/2013 12:33 PM, Martin Storsjö wrote:
>> On Thu, 4 Apr 2013, Martin Storsjö wrote:
>> 
>>> On Thu, 4 Apr 2013, Diego Biurrun wrote:
>>> 
>>>> On Wed, Apr 03, 2013 at 04:19:29PM +0300, Martin Storsjö wrote:
>>>> 
>>>>> --- a/libavcodec/dsputil_template.c
>>>>> +++ b/libavcodec/dsputil_template.c
>>>>> @@ -107,7 +107,9 @@ DCTELEM_FUNCS(int16_t, _16)
>>>>> 
>>>>> +#if BIT_DEPTH == 8
>>>>>  #include "hpel_template.c"
>>>>> +#endif
>>>> 
>>>> I'd rather remove the include from here and move it to dsputil.c, in the
>>>> BIT_DEPTH == 8 section.
>>> 
>>> Done
>> 
>> Actually, the way I moved the include to dsputil.c breaks some tests -
>> bit_depth_template.c needs to be included again before hpel_template.c.
>> I'd rather not start reshuffling those things in this patch. So reverted
>> this patch back to the form posted here.
>
> I think all you need to do is add a bit_depth_template #include to 
> hpel_template.c.

Yes, that ought to do it. Nevertheless, I don't want to do that in this 
patch.

// Martin

Patch

diff --git a/libavcodec/dsputil.c b/libavcodec/dsputil.c
index 10d775c..49f0610 100644
--- a/libavcodec/dsputil.c
+++ b/libavcodec/dsputil.c
@@ -1281,12 +1281,25 @@  QPEL_MC(0, avg_       , _       , op_avg)
 #undef op_put
 #undef op_put_no_rnd
 
+void ff_put_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
+    put_pixels8_8_c(dst, src, stride, 8);
+}
+void ff_avg_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
+    avg_pixels8_8_c(dst, src, stride, 8);
+}
+void ff_put_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
+    put_pixels16_8_c(dst, src, stride, 16);
+}
+void ff_avg_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride) {
+    avg_pixels16_8_c(dst, src, stride, 16);
+}
+
 #define put_qpel8_mc00_c  ff_put_pixels8x8_c
 #define avg_qpel8_mc00_c  ff_avg_pixels8x8_c
 #define put_qpel16_mc00_c ff_put_pixels16x16_c
 #define avg_qpel16_mc00_c ff_avg_pixels16x16_c
 #define put_no_rnd_qpel8_mc00_c  ff_put_pixels8x8_c
-#define put_no_rnd_qpel16_mc00_c ff_put_pixels16x16_8_c
+#define put_no_rnd_qpel16_mc00_c ff_put_pixels16x16_c
 
 static void wmv2_mspel8_h_lowpass(uint8_t *dst, uint8_t *src, int dstStride, int srcStride, int h){
     uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;
diff --git a/libavcodec/dsputil.h b/libavcodec/dsputil.h
index ce4469d..aa34488 100644
--- a/libavcodec/dsputil.h
+++ b/libavcodec/dsputil.h
@@ -49,20 +49,10 @@  extern const uint8_t ff_zigzag248_direct[64];
 extern uint32_t ff_squareTbl[512];
 extern uint8_t ff_cropTbl[256 + 2 * MAX_NEG_CROP];
 
-#define PUTAVG_PIXELS(depth)\
-void ff_put_pixels8x8_   ## depth ## _c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);\
-void ff_avg_pixels8x8_   ## depth ## _c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);\
-void ff_put_pixels16x16_ ## depth ## _c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);\
-void ff_avg_pixels16x16_ ## depth ## _c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);
-
-PUTAVG_PIXELS( 8)
-PUTAVG_PIXELS( 9)
-PUTAVG_PIXELS(10)
-
-#define ff_put_pixels8x8_c ff_put_pixels8x8_8_c
-#define ff_avg_pixels8x8_c ff_avg_pixels8x8_8_c
-#define ff_put_pixels16x16_c ff_put_pixels16x16_8_c
-#define ff_avg_pixels16x16_c ff_avg_pixels16x16_8_c
+void ff_put_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);
+void ff_avg_pixels8x8_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);
+void ff_put_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);
+void ff_avg_pixels16x16_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);
 
 /* RV40 functions */
 void ff_put_rv40_qpel16_mc33_c(uint8_t *dst, uint8_t *src, ptrdiff_t stride);
diff --git a/libavcodec/dsputil_template.c b/libavcodec/dsputil_template.c
index 619404c..d232d51 100644
--- a/libavcodec/dsputil_template.c
+++ b/libavcodec/dsputil_template.c
@@ -107,7 +107,9 @@  DCTELEM_FUNCS(int16_t, _16)
 DCTELEM_FUNCS(dctcoef, _32)
 #endif
 
+#if BIT_DEPTH == 8
 #include "hpel_template.c"
+#endif
 
 #define PIXOP2(OPNAME, OP) \
 static inline void FUNC(OPNAME ## _no_rnd_pixels8_l2)(uint8_t *dst, const uint8_t *src1, const uint8_t *src2, int dst_stride, \
@@ -420,20 +422,3 @@  PIXOP2(put, op_put)
 #endif
 #undef op_avg
 #undef op_put
-
-void FUNCC(ff_put_pixels8x8)(uint8_t *dst, uint8_t *src, ptrdiff_t stride)
-{
-    FUNCC(put_pixels8)(dst, src, stride, 8);
-}
-void FUNCC(ff_avg_pixels8x8)(uint8_t *dst, uint8_t *src, ptrdiff_t stride)
-{
-    FUNCC(avg_pixels8)(dst, src, stride, 8);
-}
-void FUNCC(ff_put_pixels16x16)(uint8_t *dst, uint8_t *src, ptrdiff_t stride)
-{
-    FUNCC(put_pixels16)(dst, src, stride, 16);
-}
-void FUNCC(ff_avg_pixels16x16)(uint8_t *dst, uint8_t *src, ptrdiff_t stride)
-{
-    FUNCC(avg_pixels16)(dst, src, stride, 16);
-}
diff --git a/libavcodec/h264qpel_template.c b/libavcodec/h264qpel_template.c
index 4732b29..027edf5 100644
--- a/libavcodec/h264qpel_template.c
+++ b/libavcodec/h264qpel_template.c
@@ -547,20 +547,3 @@  H264_MC(avg_, 16)
 #undef op_put
 #undef op2_avg
 #undef op2_put
-
-#if BIT_DEPTH == 8
-#   define put_h264_qpel8_mc00_8_c  ff_put_pixels8x8_8_c
-#   define avg_h264_qpel8_mc00_8_c  ff_avg_pixels8x8_8_c
-#   define put_h264_qpel16_mc00_8_c ff_put_pixels16x16_8_c
-#   define avg_h264_qpel16_mc00_8_c ff_avg_pixels16x16_8_c
-#elif BIT_DEPTH == 9
-#   define put_h264_qpel8_mc00_9_c  ff_put_pixels8x8_9_c
-#   define avg_h264_qpel8_mc00_9_c  ff_avg_pixels8x8_9_c
-#   define put_h264_qpel16_mc00_9_c ff_put_pixels16x16_9_c
-#   define avg_h264_qpel16_mc00_9_c ff_avg_pixels16x16_9_c
-#elif BIT_DEPTH == 10
-#   define put_h264_qpel8_mc00_10_c  ff_put_pixels8x8_10_c
-#   define avg_h264_qpel8_mc00_10_c  ff_avg_pixels8x8_10_c
-#   define put_h264_qpel16_mc00_10_c ff_put_pixels16x16_10_c
-#   define avg_h264_qpel16_mc00_10_c ff_avg_pixels16x16_10_c
-#endif