On Thu, Jan 21, 2016 at 1:48 PM, Diego Biurrun <[email protected]> wrote:
> On Thu, Jan 21, 2016 at 12:47:29PM -0500, Vittorio Giovara wrote:
>> On Thu, Jan 21, 2016 at 12:23 PM, Diego Biurrun <[email protected]> wrote:
>> > On Thu, Jan 21, 2016 at 11:38:20AM -0500, Vittorio Giovara wrote:
>> >> On Thu, Jan 21, 2016 at 3:17 AM, Diego Biurrun <[email protected]> wrote:
>> >> > On Wed, Jan 20, 2016 at 10:48:12AM -0500, Vittorio Giovara wrote:
>> >> >> On Wed, Jan 20, 2016 at 5:10 AM, Diego Biurrun <[email protected]> 
>> >> >> wrote:
>> >> >> > On Tue, Jan 19, 2016 at 07:45:25PM -0500, Vittorio Giovara wrote:
>> >> >> >> --- a/libavcodec/eatqi.c
>> >> >> >> +++ b/libavcodec/eatqi.c
>> >> >> >> @@ -35,69 +35,177 @@
>> >> >> >>
>> >> >> >> +/* Based off mpeg1_decode_block_intra from mpeg12dec.c */
>> >> >> >
>> >> >> > s/Based off/Duplicated from/
>> >> >>
>> >> >> it's not strictly duplicated as it has some simplifications, but ok
>> >> >
>> >> > Well, one "simplification" I noticed was mixing declarations and 
>> >> > statements,
>> >> > so ...
>> >> >
>> >> >> > I don't see this as an improvement.
>> >> >>
>> >> >> why?
>> >> >
>> >> > Because it's code duplication, pure and simple.
>> >>
>> >> you can't be seriously comparing the mpegvideo detangling and a
>> >> complete decoder deduplication (mpeg1) to the addition of a single
>> >> relatively small function
>> >
>> > Yes, I can.  You are turning an increase in size on the library level
>> > for one obscure config (eatqi w/o mpeg1) to actual code duplication.
>>
>> It's not completely clear why this build config is less or more
>> important than the celp ones tbh.
>
> Let's stay on topic.
>
>> Regardless, I consider the addition for a 50 line function (which is
>> going to optimized anyway, leading to a negligible size increase) a
>> small price for having one less decoder depend on mpegvideo, one less
>> file that needs to be recompiled whenever mpegvideo.h changes, and one
>> less weird private function.
>>
>> Heck, I went ahead and measured this size increase
>> before:  36784144 libavcodec/libavcodec.a
>> after:  36775064 libavcodec/libavcodec.a
>> so this patch created a SMALLER library, **with optimizations disabled**.
>
> So find out why, because it certainly is not eatqi.o getting smaller, on
> the contrary:
>
> ~/src/build/debug $ size libavcodec/libavcodec* | grep eatqi
>    text    data     bss     dec     hex filename
>    3325     104       0    3429     d65 eatqi.o (ex libavcodec/libavcodec.a)
>    1683     104       0    1787     6fb eatqi.o (ex 
> libavcodec/libavcodec_before.a)

you should really update your compilers ;)

before: 57808 Jan 21 13:54 libavcodec/eatqi.o
after:  49092 Jan 21 12:44 libavcodec/eatqi.o

clang-700.1.81
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to