From patchwork Thu Nov 24 00:37:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1542 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.90.1 with SMTP id o1csp3036107vsb; Wed, 23 Nov 2016 16:37:29 -0800 (PST) X-Received: by 10.28.208.203 with SMTP id h194mr9371691wmg.45.1479947849737; Wed, 23 Nov 2016 16:37:29 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id l2si19379667wjk.20.2016.11.23.16.37.29; Wed, 23 Nov 2016 16:37:29 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@googlemail.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=QUARANTINE dis=NONE) header.from=googlemail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 42539689756; Thu, 24 Nov 2016 02:37:23 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 64B64680596 for ; Thu, 24 Nov 2016 02:37:16 +0200 (EET) Received: by mail-wm0-f65.google.com with SMTP id a20so3526858wme.2 for ; Wed, 23 Nov 2016 16:37:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:subject:to:references:message-id:date:user-agent:mime-version :in-reply-to; bh=CZY8SszYj0CNvoJRjTWjHca+DvIsgQtUwoq/nlflu5k=; b=CXexe1S42+jsRDIimwWqTuh53V5FAeJC7+QRgyL1XzKnPUqvgO+VIdijf/Yk8ApV61 92iwiTiznL+Ki+W4XZtooGO3yJqxnSuWArDPsRLbYpNr+nimSV2x+0E5tysjNk0p5HUr 3URj980KMNF9OwlwhDm1JRZSRCwMUyXagRfn4g2cXIxYkAV4RZjPx+Y/o2dD6dSN+MoQ vkS6wTLQQdOE6AKzj6aO8f+6ceT1E6APJ+Dktvud2IrDSzjKpGXnFO0qKuphd1grI5lJ nhrwyuVaxZ0ef0XFNn08oAsefHH9aiZPIMXzM27DmaPPI46zIDWOAD2i6pX7UFbqr0kV 4xxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:references:message-id:date :user-agent:mime-version:in-reply-to; bh=CZY8SszYj0CNvoJRjTWjHca+DvIsgQtUwoq/nlflu5k=; b=VYW7fgB7Ll4+rtvFrSd+uA8Tqea0DH2YBpaqvIqRfzswSHZfFQd9QcWR8Zl5zJRf4g i+SPRpHvuqpc7x+Z5tMs+VmV2aT4CvIyimTKjDxHdS8Kg4/SxrT0oa3mmZ+32yF4ey2v 06SwwDtSDbR9iLLxXLf1UFspOmjmqL3yWahy+kFKmIuAAgAXqXP8xKdiAIFHfXO2+M2P nTprbp56FsJCutwjcEtQ1xC1wGVbq/HGsLOqaiKFoqXO+KBbYKpiExkHYc/BT9JKOsbZ Sam1VJTPF4lleBAiHV6fOtjlaQ1yOUq7VqhsjFNfNhe8lHCBWv4jW8/B1zVw0vXnfl+U smyQ== X-Gm-Message-State: AKaTC022Be1+s6sC1er71lGYmCQ+U+IIDUCSSB0gHNe7OTbaSJmxPswAVQPa9M2H9L6T5w== X-Received: by 10.28.8.202 with SMTP id 193mr5423117wmi.101.1479947839398; Wed, 23 Nov 2016 16:37:19 -0800 (PST) Received: from [192.168.2.21] (p5B0729FA.dip0.t-ipconnect.de. [91.7.41.250]) by smtp.googlemail.com with ESMTPSA id jm6sm38942889wjb.27.2016.11.23.16.37.18 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 23 Nov 2016 16:37:18 -0800 (PST) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: ffmpeg-devel@ffmpeg.org References: <407bf1b7-2b73-4998-d8a7-660f01f8a30e@googlemail.com> <20161123022656.GX4824@nb4> Message-ID: <6ebf692b-439a-ef0e-37b7-56a1390228b9@googlemail.com> Date: Thu, 24 Nov 2016 01:37:17 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161123022656.GX4824@nb4> Subject: Re: [FFmpeg-devel] [PATCH] flvdec: set need_context_update when changing codec id X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 23.11.2016 03:26, Michael Niedermayer wrote: > On Fri, Nov 04, 2016 at 10:28:20PM +0100, Andreas Cadhalpun wrote: >> Otherwise the codec context and codecpar might disagree on the codec id, >> triggering asserts in av_parser_parse2. >> >> Signed-off-by: Andreas Cadhalpun >> --- >> libavformat/flvdec.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c >> index e53c345..4ba7fc8 100644 >> --- a/libavformat/flvdec.c >> +++ b/libavformat/flvdec.c >> @@ -289,7 +289,9 @@ static int flv_same_video_codec(AVCodecParameters *vpar, int flags) >> static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream, >> int flv_codecid, int read) >> { >> + int ret = 0; >> AVCodecParameters *par = vstream->codecpar; >> + enum AVCodecID old_codec_id = vstream->codecpar->codec_id; >> switch (flv_codecid) { >> case FLV_CODECID_H263: >> par->codec_id = AV_CODEC_ID_FLV1; >> @@ -317,20 +319,26 @@ static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream, >> else >> avio_skip(s->pb, 1); >> } >> - return 1; // 1 byte body size adjustment for flv_read_packet() >> + ret = 1; // 1 byte body size adjustment for flv_read_packet() >> + break; >> case FLV_CODECID_H264: >> par->codec_id = AV_CODEC_ID_H264; >> vstream->need_parsing = AVSTREAM_PARSE_HEADERS; >> - return 3; // not 4, reading packet type will consume one byte >> + ret = 3; // not 4, reading packet type will consume one byte >> + break; >> case FLV_CODECID_MPEG4: >> par->codec_id = AV_CODEC_ID_MPEG4; >> - return 3; >> + ret = 3; >> + break; >> default: >> avpriv_request_sample(s, "Video codec (%x)", flv_codecid); >> par->codec_tag = flv_codecid; >> } >> >> - return 0; >> + if (par->codec_id != old_codec_id) >> + vstream->internal->need_context_update = 1; > > If this occurs only for fuzzed samples then rejecting the change > with a request for a sample seems better > > changing teh codec_id midstream like this could, seems problematic > changing at at header time might be ok if that works better than not > changing it for some real sample This happens for almost every file, because at the beginning the codec_id is AV_CODEC_ID_NONE. However, usually need_context_update is already set from avformat_new_stream, so a change can be rejected, if this is not the case. Patch doing that attached. Best regards, Andreas From 2561ebee3efd4e9b4b9ade04caa8ce7b79e0ac03 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Fri, 4 Nov 2016 21:37:13 +0100 Subject: [PATCH] flvdec: require need_context_update when changing codec id Otherwise the codec context and codecpar might disagree on the codec id, triggering asserts in av_parser_parse2. Signed-off-by: Andreas Cadhalpun --- libavformat/flvdec.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c index 3812994..39e2142 100644 --- a/libavformat/flvdec.c +++ b/libavformat/flvdec.c @@ -291,7 +291,9 @@ static int flv_same_video_codec(AVCodecParameters *vpar, int flags) static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream, int flv_codecid, int read) { + int ret = 0; AVCodecParameters *par = vstream->codecpar; + enum AVCodecID old_codec_id = vstream->codecpar->codec_id; switch (flv_codecid) { case FLV_CODECID_H263: par->codec_id = AV_CODEC_ID_FLV1; @@ -319,20 +321,28 @@ static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream, else avio_skip(s->pb, 1); } - return 1; // 1 byte body size adjustment for flv_read_packet() + ret = 1; // 1 byte body size adjustment for flv_read_packet() + break; case FLV_CODECID_H264: par->codec_id = AV_CODEC_ID_H264; vstream->need_parsing = AVSTREAM_PARSE_HEADERS; - return 3; // not 4, reading packet type will consume one byte + ret = 3; // not 4, reading packet type will consume one byte + break; case FLV_CODECID_MPEG4: par->codec_id = AV_CODEC_ID_MPEG4; - return 3; + ret = 3; + break; default: avpriv_request_sample(s, "Video codec (%x)", flv_codecid); par->codec_tag = flv_codecid; } - return 0; + if (!vstream->internal->need_context_update && par->codec_id != old_codec_id) { + avpriv_request_sample(s, "Changing the codec id midstream"); + return AVERROR_PATCHWELCOME; + } + + return ret; } static int amf_get_string(AVIOContext *ioc, char *buffer, int buffsize) @@ -547,7 +557,9 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream, st->codecpar->codec_id = AV_CODEC_ID_TEXT; } else if (flv->trust_metadata) { if (!strcmp(key, "videocodecid") && vpar) { - flv_set_video_codec(s, vstream, num_val, 0); + int ret = flv_set_video_codec(s, vstream, num_val, 0); + if (ret < 0) + return ret; } else if (!strcmp(key, "audiocodecid") && apar) { int id = ((int)num_val) << FLV_AUDIO_CODECID_OFFSET; flv_set_audio_codec(s, astream, apar, id); @@ -1098,7 +1110,10 @@ retry_duration: avcodec_parameters_free(&par); } } else if (stream_type == FLV_STREAM_TYPE_VIDEO) { - size -= flv_set_video_codec(s, st, flags & FLV_VIDEO_CODECID_MASK, 1); + int ret = flv_set_video_codec(s, st, flags & FLV_VIDEO_CODECID_MASK, 1); + if (ret < 0) + return ret; + size -= ret; } else if (stream_type == FLV_STREAM_TYPE_DATA) { st->codecpar->codec_id = AV_CODEC_ID_TEXT; } -- 2.10.2