[FFmpeg-devel] [RFC] Abort in av_find_stream_info if decoding is needed and the decoder isn't found

[Michael Niedermayer]

On Wed, Jan 27, 2010 at 10:39:39AM +0200, Martin Storsj? wrote:
> Hi,
> 
> If trying to open a realtime network stream which hasn't got any matching 
> decoder available, the av_find_stream_info call may block for a very long 
> time. (max_analyze_duration doesn't come into effect if pkt->duration 
> isn't set.)
> 
> If has_codec_parameters indicates that the stream needs decoding, but 
> try_decode_frame fails due to not being able to open the decoder, we could 
> abort immediately, since we will never be able to open that decoder. (See 
> the attached patch.)
> 
> Is this a sensible thing to add? Or are there cases where codec_id could 
> change during av_find_stream_info, so we shouldn't bail out immediately 
> even if we can't find a decoder for the current codec_id?
> 
> // Martin
>  utils.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 8cf8ba80ac6932f9ebb5dbd050bc9aa1f762d8d3  try_decode_failure.patch
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 000ab90..98ea9e1 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2204,8 +2204,20 @@ int av_find_stream_info(AVFormatContext *ic)
>             decompress the frame. We try to avoid that in most cases as
>             it takes longer and uses more memory. For MPEG-4, we need to
>             decompress for QuickTime. */
> -        if (!has_codec_parameters(st->codec))
> -            try_decode_frame(st, pkt);
> +        if (!has_codec_parameters(st->codec)) {
> +            if (try_decode_frame(st, pkt)) {
> +                if (st->codec->codec_id != CODEC_ID_NONE &&
> +                    !st->codec->codec) {
> +                    /* Codec ID known, but unable to open the codec.
> +                     * Abort instead of trying infinitely. */
> +                    av_log(ic, AV_LOG_ERROR,
> +                           "Unable to open codec %d in try_decode_frame\n",
> +                           st->codec->codec_id);
> +                    ret = AVERROR_INVALIDDATA;
> +                    break;
> +                }
> +            }
> +        }
>  
>          count++;
>      }

this is definitly not ok
there may be other streams that are decodeable, a single stream causing
a problem must not cause an abort earlier than what would have been
with all parameters for that stream being available at the start.


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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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-devel/attachments/20100127/0d5622b1/attachment.pgp>