On Tue, Apr 10, 2012 at 02:18:45PM -0700, Ronald S. Bultje wrote: > On Tue, Apr 10, 2012 at 1:49 PM, Diego Biurrun <[email protected]> wrote: > > On Tue, Apr 10, 2012 at 01:27:59PM -0700, Ronald S. Bultje wrote: > >> On Tue, Apr 10, 2012 at 1:23 PM, Diego Biurrun <[email protected]> wrote: > >> > On Tue, Apr 10, 2012 at 12:49:20PM -0700, Ronald S. Bultje wrote: > >> >> On Tue, Apr 10, 2012 at 12:40 PM, Diego Biurrun <[email protected]> > >> >> wrote: > >> >> > On Tue, Apr 10, 2012 at 11:37:30AM -0700, Luca Barbato wrote: > >> >> >> On 10/04/12 09:24, Diego Biurrun wrote: > >> >> >> > On Tue, Apr 10, 2012 at 07:30:30AM -0700, Ronald S. Bultje wrote: > >> >> >> >> On Tue, Apr 10, 2012 at 7:12 AM, Diego Biurrun <[email protected]> > >> >> >> >> wrote: > >> >> >> >>> On Fri, Mar 30, 2012 at 02:08:22AM +0200, Diego Biurrun wrote: > >> >> >> >>>> --- > >> >> >> >>>> libavcodec/Makefile | 2 +- > >> >> >> >>>> libavcodec/h264.c | 358 > >> >> >> >>>> +++----------------------------------------------- > >> >> >> >>>> libavcodec/h264.h | 5 + > >> >> >> >>>> libavcodec/h264dec.c | 349 > >> >> >> >>>> ++++++++++++++++++++++++++++++++++++++++++++++++ > >> >> >> >>>> 4 files changed, 375 insertions(+), 339 deletions(-) > >> >> >> >>>> create mode 100644 libavcodec/h264dec.c > >> >> >> >>> > >> >> >> >>> ping > >> >> >> >> > >> >> >> >> You told me on IRC it did not significantly decrease compile > >> >> >> >> time, so > >> >> >> >> I don't see the point of the patch. > >> >> >> > > >> >> >> > That's not the only reason I went for implementing this patch. It > >> >> >> > also > >> >> >> > reduces the size of h264.c by 350 lines, which is not huge, but > >> >> >> > more > >> >> >> > than nothing, and it reduces the size of libavcodec if the H.264 > >> >> >> > decoder > >> >> >> > is disabled but some other part that requires h264.c is enabled. > >> >> >> > >> >> >> Good point, if it doesn't break anything and doesn't hinder people > >> >> >> working on h264.c seems a good patch. > >> >> > > >> >> > Size difference before and after while configuring with > >> >> > "--disable-everything --enable-decoder=svq3": > >> >> > > >> >> > before: > >> >> > biurrun@passion:~/src/priv/libav $ ls -l libavcodec/libavcodec.a > >> >> > -rw-rw-r-- 1 biurrun src-pisa 33880296 2012-04-10 20:47 > >> >> > libavcodec/libavcodec.a > >> >> > > >> >> > after: > >> >> > -rw-rw-r-- 1 biurrun src-pisa 33852434 2012-04-10 21:36 > >> >> > libavcodec/libavcodec.a > >> >> > > >> >> > This amounts to a saving of 32kB. > >> >> > >> >> I still don't think the split makes sense. Please re-read my earlier > >> >> comment. > >> > > >> > There is no earlier comment to re-read, you only mentioned the effect of > >> > this patch on compile time. Since some miscommunication seems to be > >> > going on, let me quickly reiterate the goals of this patch: > >> > > >> > 1) Reduce line count of h264.c, > >> > 2) reduce compile time of h264.c and > >> > 3) reduce libavcodec size in certain configurations. > >> > > >> > Unfortunately the split did not quite have the effect on 2) that I > >> > had hoped for, but there are modest improvements in 1) and 3). > >> > > >> > Could you please quickly state why you think this patch is not worth > >> > the improvements in 1) and 3)? > >> > >> I wonder how you think the patch manages to accomplish (3), given that > >> it doesn't add or remove code? Sorry to sound like an asshole, but I'm > >> a bit sceptical over this claim. > > > > Read what I write :) > > > > Let me repeat: > > > > Size difference before and after while configuring with > > "--disable-everything --enable-decoder=svq3": > > > > before: > > biurrun@passion:~/src/priv/libav $ ls -l libavcodec/libavcodec.a > > -rw-rw-r-- 1 biurrun src-pisa 33880296 2012-04-10 20:47 > > libavcodec/libavcodec.a > > > > after: > > -rw-rw-r-- 1 biurrun src-pisa 33852434 2012-04-10 21:36 > > libavcodec/libavcodec.a > > > > This amounts to a saving of 32kB. > > > > h264.c is currently not only required by the H.264 decoder, but also by > > the SVQ3 decoder and the H.264 parser. After my patch the common code > > remains in h264.c and the decoder-only code is in h264dec.c. Configurations > > that disable the H.264 decoder while enabling parts that require the common > > code in h264.c thus end up smaller because h264dec.c is no longer compiled. > > So by removing H264 specific code from a binary that only contains a > SVQ3 decoder, you manage to save 32kB out of 34MB? Isn't that a little > ... Ehm, how shall I put this kindly... Lame? Wouldn't you expect > more?
I was hoping for more of course, but that's not something you can know beforehand. before: -rwxrwxr-x 1 biurrun src-pisa 17237595 2012-04-10 23:51 avconv -rwxrwxr-x 1 biurrun src-pisa 3125464 2012-04-10 23:53 avconv.stripped -rw-rw-r-- 1 biurrun src-pisa 33880296 2012-04-10 23:51 libavcodec/libavcodec.a -rw-rw-r-- 1 biurrun src-pisa 2647890 2012-04-10 23:53 libavcodec/libavcodec.a.stripped after: -rwxrwxr-x 1 biurrun src-pisa 17225151 2012-04-10 21:36 avconv -rwxrwxr-x 1 biurrun src-pisa 3121304 2012-04-10 23:47 avconv.stripped -rw-rw-r-- 1 biurrun src-pisa 33852434 2012-04-10 21:36 libavcodec/libavcodec.a -rw-rw-r-- 1 biurrun src-pisa 2643658 2012-04-10 23:48 libavcodec/libavcodec.a.stripped > How's speed and size of the binary containing both decoders? Does it > change after this patch? before: -rwxrwxr-x 1 biurrun src-pisa 53714897 2012-04-11 00:04 avconv -rwxrwxr-x 1 biurrun src-pisa 8829336 2012-04-11 00:26 avconv.stripped -rw-rw-r-- 1 biurrun src-pisa 101500476 2012-04-11 00:04 libavcodec/libavcodec.a -rw-rw-r-- 1 biurrun src-pisa 7765212 2012-04-11 00:22 libavcodec/libavcodec.a.stripped after: -rwxrwxr-x 1 biurrun src-pisa 53747112 2012-04-11 00:47 avconv -rwxrwxr-x 1 biurrun src-pisa 8829336 2012-04-11 00:50 avconv.stripped -rw-rw-r-- 1 biurrun src-pisa 101607350 2012-04-11 00:47 libavcodec/libavcodec.a -rw-rw-r-- 1 biurrun src-pisa 7766776 2012-04-11 00:50 libavcodec/libavcodec.a.stripped So size increases for the default case. I would suspect that some tables are getting duplicated somewhere. I'll not go to the trouble of investigating issue now, so patch dropped. Diego _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
