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. Diego _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
