From patchwork Mon Sep 7 02:49:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22142 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 308F74484C1 for ; Mon, 7 Sep 2020 05:55:48 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0974768A370; Mon, 7 Sep 2020 05:55:48 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-lj1-f194.google.com (mail-lj1-f194.google.com [209.85.208.194]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B5D4368084B for ; Mon, 7 Sep 2020 05:55:41 +0300 (EEST) Received: by mail-lj1-f194.google.com with SMTP id u21so4047334ljl.6 for ; Sun, 06 Sep 2020 19:55:41 -0700 (PDT) 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=+4+Xx/0E2P3lJ43nOxOvSYvtr2E2kt8696NPgNY/l4c=; b=cowSktDQOJkoE0q32brluaOSGYajBkIudysGr9KMZx9S+UQDWBQJwksgF2d/4HDV4e SgU26gIktXO2K6FhW77KZEUj9LUQzWfL14U/i6XxhTQGZqOQ9b9oT8PBbYuw78V/69cp fzNDd9mPBdhVCSQ3FahF9dMf4V+IkOHkGNTg27m0GqZRbFo64STXVyOnl9MjCZa3BTVG vUfhQldE51poRxS9fVH82OC9WgbnYlJ5mEpy3KI5SmMspPKjsH6obDK9XIYSPHXwQWxh Yps62caQae6lvQ4ALb3wujwNP79ncEAkXpzUDfmJUYp00Bx0LAvIvcv1xsc9+tHg+XYo bXpA== 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=+4+Xx/0E2P3lJ43nOxOvSYvtr2E2kt8696NPgNY/l4c=; b=Pt+t0MiA2VK7VrhOwxLR+OMv8CfLDq/jogha1ArC24Uq+70NTvdlKHTU5OsJqDZLMW vqQefpRgOpSjYyfqZTxp14UjOfEH3AXsYpH9/YT9A7g7RFMwZxMBNmjUPwmTLe9umqs/ u2P2OM/hVCt3MdHvbVqFn17OWbv7/eeXQhSJuad/2Z1kKw6QhDaZnclNbdXK/36L9jLQ uCIZ1MH5oZllqaC4Ul2Bquji4XgTfSb1urBdLyANTA7C5m/0Wo04IV0IwpVR09cMWZO/ QafI/rqiJBN9v1ybENDSp4m11zuJxPRhmlJI+n/DPCLd4H4P7zicf876/zXVlJgx9tLC 45OA== X-Gm-Message-State: AOAM533HrWlbeHpJJKpdtg+QiUwQ3Bvg0xQCfKzJWp7ylHDzFxKVZKMB EU18Vm4AxaKgEzkTyehNzcsld4Ee8TI= X-Google-Smtp-Source: ABdhPJx8aIOtOUa52qutVDzhufBoAtT5FxMUJUhmfi2tL/DXymA98kCyFI1QGnNpBjdtREAPEIevxw== X-Received: by 2002:a17:906:8c8:: with SMTP id o8mr18780271eje.91.1599447002083; Sun, 06 Sep 2020 19:50:02 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id a15sm13802048eje.16.2020.09.06.19.50.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 06 Sep 2020 19:50:01 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 7 Sep 2020 04:49:43 +0200 Message-Id: <20200907024952.11697-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 01/10] avformat/segment: Don't overwrite AVCodecParameters after init 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" The segment muxer copies the user-provided AVCodecParameters to the newly created child streams in its init function before initializing the child muxer; and since commit 8e6478b723affe4d44f94d34b98e0c47f6a0b411, it does this again before calling avformat_write_header() if that is called from seg_write_header(). The reason for this is complicated: At that time writing the header was delayed, i.e. it was not triggered by avformat_write_header() (unless the AVFMT_FLAG_AUTO_BSF was unset), but instead by writing the very first packet. The rationale behind this was to allow to run bitstream filters on the packets in the interleavement queue in order to generate missing extradata from them before the muxer's write_header function is actually called. The segment muxer went even further: It initialized the child muxer and ran the child muxer's check_bitstream functions on the packets in its own muxing queue and stole any bitstream filters that got inserted. The reason for this is that the segment muxer has an option to write the header to a separate file and for this it is needed to write the child muxer's header without delay, but with correct extradata. Unsetting AVFMT_FLAG_AUTO_BSF for the child muxer accomplished the first goal and stealing the bitstream filters the second; and in order for the child muxer to actually use the updated extradata, the old AVCodecParameters (set before avformat_init_output()) were overwritten with the new ones. Updating the extradata proceeded as follows: The bitstream filter itself simply updated the AVBSFContext's par_out when processing a packet, in violation of the new BSF API (where par_out may only be set in the init function); the muxing code then simply forwarded the updated extradata, overwriting the par_in of the next BSF in the BSF chain with the fresh par_out of the last one and the AVStream's par with the par_out of the last BSF. This was an API violation, too, of course, but it made remuxing ADTS AAC into mp4/matroska work. But this no longer serves a useful purpose since the aac_adtstoasc BSF was updated to propagate new extradata via packet side data in commit f63c3516577d605e51cf16358cbdfa0bc97565d8; the next commit then removed the code in mux.c passing new extradata along the filter chain. This alone justifies removing the code for setting the AVCodecParameters a second time. But there is even another reason to do so: It is harmful. The ogg muxer parses the extradata of Theora and Vorbis in its init function and keeps pointers to parts of it. Said pointers become dangling when the extradata is overwritten by the segment muxer, leading to use-after-frees as has happened in ticket #8881 which this commit fixes. Ticket #8517 is about another issue caused by this: Immediately after having overwritten the old AVCodecParameters the segment muxer checks whether the codec_tag is ok (the codec_tag is set generically when initializing the child muxer based upon muxer-specific lists). The check used is: If the child output format has such a list and if the codec tag of the non-child stream does not match the codec id given the list of codec tags and if there is a match for the codec id in the codec tag list, then set the codec tag to zero (and not to the existing match), otherwise set the codec tag of the child stream to the codec tag of the corresponding stream of the main AVFormatContext (which is btw redundant given that the child AVCodecParameters have just been overwritten with the AVCodecParameters of the corresponding stream of the main AVFormatContext). Signed-off-by: Andreas Rheinhardt --- libavformat/segment.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/libavformat/segment.c b/libavformat/segment.c index f67456fa57..0c9b93725d 100644 --- a/libavformat/segment.c +++ b/libavformat/segment.c @@ -817,26 +817,9 @@ static int seg_write_header(AVFormatContext *s) { SegmentContext *seg = s->priv_data; AVFormatContext *oc = seg->avf; - int ret, i; + int ret; if (!seg->header_written) { - for (i = 0; i < s->nb_streams; i++) { - AVStream *st = oc->streams[i]; - AVCodecParameters *ipar, *opar; - - ipar = s->streams[i]->codecpar; - opar = oc->streams[i]->codecpar; - avcodec_parameters_copy(opar, ipar); - if (!oc->oformat->codec_tag || - av_codec_get_id (oc->oformat->codec_tag, ipar->codec_tag) == opar->codec_id || - av_codec_get_tag(oc->oformat->codec_tag, ipar->codec_id) <= 0) { - opar->codec_tag = ipar->codec_tag; - } else { - opar->codec_tag = 0; - } - st->sample_aspect_ratio = s->streams[i]->sample_aspect_ratio; - st->time_base = s->streams[i]->time_base; - } ret = avformat_write_header(oc, NULL); if (ret < 0) return ret;