[FFmpeg-cvslog] r12759?-?in?trunk/libavcodec:?aac_ac3_parser.c?aac_ac3_parser.h?aac_ parser.?c?ac3_parser.c

[Michael Niedermayer]

On Fri, Apr 18, 2008 at 12:14:54PM +0200, Bartlomiej Wolowiec wrote:
> On pi?tek, 18 kwietnia 2008, Michael Niedermayer wrote:
> > On Fri, Apr 18, 2008 at 12:10:35AM +0200, Bartlomiej Wolowiec wrote:
> > > Hi, I think that I got lost somewhere, because I can't see the main goal
> > > of this...
> >
> > The main goal is to pass values which represent the truth and check the
> > return value of ff_combine_frame()
> > what you do is a little like malloc(7) and use 8 bytes because malloc
> > happens to round it up internally.
> >
> > > Generally, I hope that you thought about a? solution like the solution in
> > > attachment (or something in this direction).
> >
> > i wanted you to add checks for the return value of ff_combine_frame() and
> > to pass a truthfull parameter which says if you found a frame boundary.
> > You still do not do this, you still explicitly check for EOF and then
> > provide ff_combine_frame() with a fake "i found a startcode here at 0".
> > If that was true it would also be if buf_size>0 but in that case its not
> > so it cannot be true if buf_size==0.
> >

> > So please remove the buf_size==0 check.
> 
> Ok, there is no check of buf_size==0 now, when buf_size==0 END_NOT_FOUND  is 
> sent.
> 
> > Also the code was working without such checks already, it just failed at
> > EOF when ff_combine_frame() told you that it had a frame but your code
> > continued as if ff_combine_frame() had none.
> 
> After calling ff_combine_frame I added check of buf_size==0, to check whether 
> there is  anything in the buffer.

contradictionary, you say there is no check (ohh happyness) and then you say
you added it back. So obviously its no longer "no check" its just moved
around again.

also the buf_size check now in addition to being wrong now also really
breaks EOF handling by skiping random parts of the code.


> 
> > besides this, the code now is completely reordered and its somewhat hard
> > to check the correctness of the new code. at least the frame_in_buffer
> > code is buggy now though.
> 
> after rebuilding it more readable for me.

Yes, i agree, the while is somewhat more readable


> I moved the condition with 
> frame_in_buffer few lines up, so that it should work now.

you set frame_in_buffer=0 whatever this is trying to achive it is not what
the code was originally used for. Which was discarding initial junk after
seeking. Now it looks like its going to discard things in the middle in
some cases.

Also ive removed AACAC3FrameFlag in svn so i dont need to see this
unrelaed change in every of your patches anymore.

[...]
-- 
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: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20080418/28ecb230/attachment.pgp>