From patchwork Fri Feb 28 04:14:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 17951 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 5F2AB44988B for ; Fri, 28 Feb 2020 06:22:03 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3C9B768B112; Fri, 28 Feb 2020 06:22:03 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 306B168B08C for ; Fri, 28 Feb 2020 06:21:57 +0200 (EET) Received: by mail-wr1-f66.google.com with SMTP id y17so1419827wrn.6 for ; Thu, 27 Feb 2020 20:21:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=G4CzzTN3IiOSLHIZdZoD2g5vRByDIBFLmLrZbtx3heg=; b=DTbK1tVVZRMPre8qmABNFH/kHVZEZU01nd2E8fwTe6nibZuJNhTbW4ipwi29+KJ2aj 7jQlWAmr07lV6opoxhJ8wqAX6oGmVsNHwMIWWMyvMQote1Ht75xu07drcnF+iDHOSGwV EcBeQ/yncutgJPZ72cwKvn/aVqGRO/VFkRFZhLGeoiLU1e7EWC7tsN5DiW16mjWRTh6e LxmEZsaSbL7gh1Keo55YjHqQ/p4z0ceYbS2DNpGb5pcqKdx/KoRg6A7paa/ldz7ZCsPA /4GVjvGFQ/jFk5exR1driBJC1VDKUDNEWIRMK9YA2cb9dGkrxi22M7Rr7QVYN55aO8KO ixfQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=G4CzzTN3IiOSLHIZdZoD2g5vRByDIBFLmLrZbtx3heg=; b=smu7hrKVUWnp2blfd51864GQd6UdfqGSGhnkIdb32tvkCoTKKs88lKeGYSw6nq2cpZ YZYLY0lh+GBQm9CwmSHpZCBs3X5fbulTXL59VOYlNmMFwrYapTLJmOa9t/0ErlSVpJb3 tJsqSz6FOJAMA0oBTQ1lPmGk3gV2ZGf2dYPq1CoF8I6hImXDPNoxX8dyzL+KOuttQeh3 5utq5llCD08q7poKCxkqQ29x2qEL26dNA0CicS/P5YxlZ4XZ5VKvJCBheOKuw5veDkPX zDQfOyKDR1hZSQVXBL04dzIihVF2qvVz46sAq/2ObYt5nh32o9cqJYC8EivunUdajCVx w01Q== X-Gm-Message-State: APjAAAXRa3EOKtL5fjlD2ohMqEEy+1r9wiPgMat/iUbpaFy8VxlFbKew VVbaENMzT4bl9XwidFAFWDkOZoRP X-Google-Smtp-Source: APXvYqz5Cc8uSYT73mZVEwin6PdcrkZH3tZBkCK5LRhBMGF0IqSljOr1ML3L1tj7Sq/5rKWp4owEGg== X-Received: by 2002:adf:fc12:: with SMTP id i18mr2504599wrr.354.1582863279358; Thu, 27 Feb 2020 20:14:39 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc1ab4b.dynamic.kabel-deutschland.de. [188.193.171.75]) by smtp.gmail.com with ESMTPSA id e22sm344059wme.45.2020.02.27.20.14.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Feb 2020 20:14:38 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 28 Feb 2020 05:14:32 +0100 Message-Id: <20200228041432.4454-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avformat/mxfenc: Never set codec_ul UID to NULL 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 Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" mxf distinguishes codec profiles by different UIDs and therefore needs to check that the input is actually compatible with mxf (i.e. if there is a defined UID for it). If not, then sometimes the UID would be set to NULL and writing the (video) packet would fail. Yet the following audio packet would trigger writing the header (which has been postponed because the UID is not known at the start) and if the UID is NULL, this can lead to segfaults. This commit therefore stops setting the UID to NULL if the input is incompatible with mxf (it has initially been set to a generic value in mxf_write_header()). Fixes #7993. Signed-off-by: Andreas Rheinhardt --- a) #7993 is actually a segfault even if its description suggests otherwise. b) My actually preferred way to fix this was to error out if an audio packet is encountered and the header has not been written yet, thereby ensuring that only a video packet can trigger writing the header and said code is only reached when the video is actually compatible with mxf (or with this muxer?). Yet I was surprised to find out that the FATE-test mxf-d10-user-comments would fail with this, because right now writing the header in this test is triggered by an audio packet (the video packets aren't written because they are not of the right size) and if these aren't written either, there will be zero bytes of output. c) It seems that mxf_write_header() doesn't actually write anything and could be converted into an init function. libavformat/mxfenc.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 7ea47d7311..c00ab9d1c3 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -1952,7 +1952,6 @@ static int mxf_parse_prores_frame(AVFormatContext *s, AVStream *st, AVPacket *pk if (mxf->header_written) return 1; - sc->codec_ul = NULL; profile = st->codecpar->profile; for (i = 0; i < FF_ARRAY_ELEMS(mxf_prores_codec_uls); i++) { if (profile == mxf_prores_codec_uls[i].profile) { @@ -1960,7 +1959,7 @@ static int mxf_parse_prores_frame(AVFormatContext *s, AVStream *st, AVPacket *pk break; } } - if (!sc->codec_ul) + if (i == FF_ARRAY_ELEMS(mxf_prores_codec_uls)) return 0; sc->frame_size = pkt->size; @@ -2006,7 +2005,6 @@ static int mxf_parse_dnxhd_frame(AVFormatContext *s, AVStream *st, AVPacket *pkt if (pkt->size < 43) return 0; - sc->codec_ul = NULL; cid = AV_RB32(pkt->data + 0x28); for (i = 0; i < FF_ARRAY_ELEMS(mxf_dnxhd_codec_uls); i++) { if (cid == mxf_dnxhd_codec_uls[i].cid) { @@ -2014,7 +2012,7 @@ static int mxf_parse_dnxhd_frame(AVFormatContext *s, AVStream *st, AVPacket *pkt break; } } - if (!sc->codec_ul) + if (i == FF_ARRAY_ELEMS(mxf_dnxhd_codec_uls)) return 0; sc->component_depth = 0; @@ -2177,6 +2175,7 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st, const uint8_t *buf = pkt->data; const uint8_t *buf_end = pkt->data + pkt->size; const uint8_t *nal_end; + const UID *codec_ul = NULL; uint32_t state = -1; int extra_size = 512; // support AVC Intra files without SPS/PPS header int i, frame_size, slice_type, intra_only = 0; @@ -2246,12 +2245,11 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st, if (!sps) sc->interlaced = st->codecpar->field_order != AV_FIELD_PROGRESSIVE ? 1 : 0; - sc->codec_ul = NULL; frame_size = pkt->size + extra_size; for (i = 0; i < FF_ARRAY_ELEMS(mxf_h264_codec_uls); i++) { if (frame_size == mxf_h264_codec_uls[i].frame_size && sc->interlaced == mxf_h264_codec_uls[i].interlaced) { - sc->codec_ul = &mxf_h264_codec_uls[i].uid; + codec_ul = &mxf_h264_codec_uls[i].uid; sc->component_depth = 10; // AVC Intra is always 10 Bit sc->aspect_ratio = (AVRational){ 16, 9 }; // 16:9 is mandatory for broadcast HD st->codecpar->profile = mxf_h264_codec_uls[i].profile; @@ -2265,7 +2263,7 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st, mxf_h264_codec_uls[i].profile == sps->profile_idc && (mxf_h264_codec_uls[i].intra_only < 0 || mxf_h264_codec_uls[i].intra_only == intra_only)) { - sc->codec_ul = &mxf_h264_codec_uls[i].uid; + codec_ul = &mxf_h264_codec_uls[i].uid; st->codecpar->profile = sps->profile_idc; st->codecpar->level = sps->level_idc; // continue to check for avc intra @@ -2274,10 +2272,11 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st, av_free(sps); - if (!sc->codec_ul) { + if (!codec_ul) { av_log(s, AV_LOG_ERROR, "h264 profile not supported\n"); return 0; } + sc->codec_ul = codec_ul; return 1; } @@ -2374,9 +2373,13 @@ static int mxf_parse_mpeg2_frame(AVFormatContext *s, AVStream *st, } } } - if (s->oformat != &ff_mxf_d10_muxer) - sc->codec_ul = mxf_get_mpeg2_codec_ul(st->codecpar); - return !!sc->codec_ul; + if (s->oformat != &ff_mxf_d10_muxer) { + const UID *codec_ul = mxf_get_mpeg2_codec_ul(st->codecpar); + if (!codec_ul) + return 0; + sc->codec_ul = codec_ul; + } + return 1; } static uint64_t mxf_parse_timestamp(int64_t timestamp64)