[FFmpeg-devel] [PATCH 3/7] lavc/utils: check av_frame_alloc() failure.
Nicolas George
george at nsup.org
Mon Dec 30 10:56:51 CET 2013
Le decadi 10 nivôse, an CCXXII, Michael Niedermayer a écrit :
> i test almost all my comits but its quite possible that what i tested
> (fate probably) did never test the code i changed, thats my mistake
> no question about that
I never doubted you did test, it was intended as a joke. The code is indeed
certainly not covered by FATE, or we would probably have noticed the
breakage I am reporting sooner, but your commit was perfectly correct
(except for the error check part, and this is of dubious use due to
overcommit).
> I think we should add some fate tests for old APIs
> maybe the examples could be adapted to do that, i could look into that
> but as you tested the code, i assume you already have a test for them?
I had the same idea.
> I possibly worded this unclearly
> what i meant was that if setting
> release_buffer= avcodec_default_release_buffer
> causes some problem then an application that calls
> avcodec_default_release_buffer from its release_buffer implementation
> would also cause the problem, and not setting release_buffer would
> likely leak and i saw one case where its called without a null
> pointer check so it could crash
>
> but maybe iam missing something, can you elaborate about the issue
> you found ?
I suppose you may be forgetting something: since avcodec_decode_audio4() was
introduced (2011-09-06 / 0eea21294), using a custom get_buffer with compat
avcodec_decode_audio3() does not work and is forbidden. The original check
was:
+ if (avctx->get_buffer != avcodec_default_get_buffer) {
+ av_log(avctx, AV_LOG_ERROR, "A custom get_buffer() cannot be used with "
+ "avcodec_decode_audio3()\n");
+ return AVERROR(EINVAL);
+ }
And it was later amended to make the message clearer and override the custom
callback instead of failing (2012-01-15 / e2ff436ef):
+ av_log(avctx, AV_LOG_ERROR, "Custom get_buffer() for use with"
+ "avcodec_decode_audio3() detected. Overriding with
avcodec_default_get_buffer\n");
+ av_log(avctx, AV_LOG_ERROR, "Please port your application to "
+ "avcodec_decode_audio4()\n");
+ avctx->get_buffer = avcodec_default_get_buffer;
Unfortunately, since the refcounted frames, i.e. "the evil plan"
(2012-11-21, 759001c), the codec context initialization no longer sets the
get_buffer field (nor release_buffer, of course).
- s->get_buffer = avcodec_default_get_buffer;
- s->release_buffer = avcodec_default_release_buffer;
+ s->get_buffer2 = avcodec_default_get_buffer2;
Note that the fork ignores the problem since they dropped
FF_API_OLD_DECODE_AUDIO at the 54->55 version bump (refcounted frames),
while the test in FFmpeg is currently for the 56 bump.
I realize it was a bit exaggerated to state that avcodec_decode_audio3() is
broken: it only prints an obnoxious warning. The patch I posted removes that
warning by initing the get_buffer, but only when needed (no point in leaving
an obsolete field loaded unless necessary). I realize this is strange it did
work since I did not address the release_buffer part.
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131230/51f7ad57/attachment.asc>
More information about the ffmpeg-devel
mailing list