[FFmpeg-devel] [PATCH 9/9] lavf/ffm: use AVOption API to store/restore stream properties

[Michael Niedermayer]

On Thu, Nov 13, 2014 at 01:39:16AM +0100, Lukasz Marek wrote:
> W dniu czwartek, 13 listopada 2014 Michael Niedermayer <michaelni at gmx.at>
> napisał(a):
> 
> > On Thu, Nov 13, 2014 at 12:50:38AM +0100, Lukasz Marek wrote:
> > > On 12.11.2014 03:19, Michael Niedermayer wrote:
> > > >On Wed, Nov 12, 2014 at 12:02:07AM +0100, Lukasz Marek wrote:
> > > >>On 11.11.2014 08:31, Lukasz Marek wrote:
> > > >>>This is a generic solution that will not reqiore modifications when
> > new options are added.
> > > >>>This also fixes problem with current implementation when qmin or
> > qmax=-1.
> > > >>>Only 8 bits was sent and read back as 255.
> > > >>>
> > > >>>Fixes #1275
> > > >>>Fixes #1461
> > > >>>
> > > >>>Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com <javascript:;>>
> > > >>>---
> > > >>>  libavformat/ffmdec.c | 20 +++++++++++++++
> > > >>>  libavformat/ffmenc.c | 70
> > +++++++++++++++-------------------------------------
> > > >>>  2 files changed, 40 insertions(+), 50 deletions(-)
> > > >>
> > > >>Updated version attached.
> > > >
> > > >not related to this patch specifically but please test
> > > >fate-opt under valgrind
> > > >it seems one of the patches adds a memleak
> > > >
> > > >==16696== 4 bytes in 1 blocks are definitely lost in loss record 1 of 6
> > > >==16696==    at 0x4C2A6C5: memalign (vg_replace_malloc.c:727)
> > > >==16696==    by 0x4C2A760: posix_memalign (vg_replace_malloc.c:876)
> > > >==16696==    by 0x40FC2D: av_malloc (mem.c:95)
> > > >==16696==    by 0x402022: set_string_binary (opt.c:142)
> > > >==16696==    by 0x40617F: av_opt_set_defaults2 (opt.c:1235)
> > > >==16696==    by 0x405ED1: av_opt_set_defaults (opt.c:1175)
> > > >==16696==    by 0x40809C: main (opt.c:1986)
> > >
> > > This is leak in the test itself :P
> > > Fix was trivial so I pushed it directly.
> > >
> > > I uploaded updated patch, that removes necessity of the patch I dropped
> > > "[PATCH] lavc/options: set all codec options to defaults"
> > >
> > > Note: it extends opt serialize with new flag, so before a sumbit I
> > > will move that chunk to commit that introduce it.
> > >
> >
> > > I pushed to github current patches too.
> >
> > it seems your github tree doesnt pass fate:
> >
> > git checkout  lukaszmluki/master
> > make distclean ; ./configure && make -j12 fate
> >
> > --- ./tests/ref/lavf/ffm        2014-11-13 00:12:31.840566628 +0100
> > +++ tests/data/fate/lavf-ffm    2014-11-13 01:07:06.060635609 +0100
> > @@ -1,3 +1,3 @@
> > -16cc0ee04b036c210b2fc85182d748e1 *./tests/data/lavf/lavf.ffm
> > +2dc0c4555adb5c54fab55e9c9b3fc522 *./tests/data/lavf/lavf.ffm
> >  376832 ./tests/data/lavf/lavf.ffm
> >  ./tests/data/lavf/lavf.ffm CRC=0x000e23ae
> >
> >
> have you tested previous versions too? yesterday for example?

yes i had tested a previous one too and it failed the same way
dont remember when 


> I ran fate, but not for whole patchet as i remember.
> 
> > I hope it is side effect the patches fixed something, not broke, but the
> answer will have to wait until tommorow. if fate pass without top commit,
> then it is very possibe.

seems passing with lukaszmluki/master^

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141113/1acb9a54/attachment.asc>