[Ffmpeg-devel] [PATCH] Fix for the lol-ffplay.ogm testcase

Luca Abeni lucabe72
Wed Jan 17 13:03:08 CET 2007


Hi,

On Wed, 2007-01-17 at 11:35 +0100, Panagiotis Issaris wrote:
[...]
> > But I'd prefer to fix the calling code anyway. I'll check ffmpeg.c
> > during the lunch break.
> Excellent! :)
Ok, here is the problem:
- before calling sws_getContext(), ffmpeg tries to allocate a temporary
picture for the rescaled image, by calling avpicture_alloc() at line
1622. Since the picture size is 0x0, avpicture_alloc() fails, and ffmpeg
jumps to the fail: label... Then, it tries to cleanup everything, and
sws_freeContext(NULL) is called. The problem here is that "goto fail" is
used instead of "exit()". The attached fix1.diff patch fixes the
problem.
If people do not like calling exit(), and prefer "goto fail", then
fix2.diff can be applied... Or we can just leave things as they are,
since sws_freeContext() now likes NULL as a parameter (but I still do
not like having ost->video_resample != 0 and ost->img_resample_ctx ==
NULL).

Anyway, looking at ffmpeg.c I realized that there are a lot of
inconsistencies in error handling:
- sometimes, "goto fail" is used
- some other times, "exit()" is directly called
- finally, there are cases in which av_abort() is called! (I think this
is wrong, because av_abort() should be an internal function, no?).

So, should we try to always use the same construct in case of error?
If yes, which one?
In my opinion, for errors happening during the initialization phase
there is not much point in using "goto fail", and I'd be for using
"exit()" (of course, for errors happening during the decoding /
encoding, "goto exit" is the correct thing to do).

			Thanks,
				Luca
-- 
_____________________________________________________________________________
Copy this in your signature, if you think it is important:
                               N O    W A R ! ! !
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix1.diff
Type: text/x-patch
Size: 602 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070117/e0b1217b/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix2.diff
Type: text/x-patch
Size: 762 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070117/e0b1217b/attachment-0001.bin>



More information about the ffmpeg-devel mailing list