From patchwork Sat May 2 23:27:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19447 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 499D544B2DD for ; Sun, 3 May 2020 02:27:56 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2623C68C253; Sun, 3 May 2020 02:27:56 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B4C9D68BC07 for ; Sun, 3 May 2020 02:27:49 +0300 (EEST) Received: by mail-wm1-f66.google.com with SMTP id e26so4206346wmk.5 for ; Sat, 02 May 2020 16:27:49 -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=N5NfeAKplRn4M178xITusxnh31qH+o/WDOCB/jysvJM=; b=WN4bHF7Zxt7F0mBKQhTNGJuYtCYVwG8xcFA42SPVcMisY8AkoeCIYEVe2dkpI2fgyU viQjkS0JeW9ck50Rylptd9yC3LWkZ70+yhPEeHMkdboFCDajHcBjXkhPEUd89kP9LIdt DniOUv0UrTJ92m1BLVuOCsnMvBW04jxL+d9kYy/YaNs6NbbD/iQIImBDsfdXzlO4wGUR CDIR/M8J+Tg1qqo+2apsdbE9/SU+/1tGgdHQG1XHFE69NxVr96Jcs5qsY7FzHTIkI5SW jue70LmVb67YQ+ljzpjdWuRasJNKCliLHhie7Dt/TRJu2c12SqfLP2Q+AK7ERlT7VCgK mSOw== 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=N5NfeAKplRn4M178xITusxnh31qH+o/WDOCB/jysvJM=; b=YmPMsu9vgUDWjKQ652ZY6JbErd7/pfKwnbwL4l9D2BsMCJ3R5bfuo2viPx+j95Bejw Q1d7tpKSqKyBPMXyCzA27PyizTXJW+dpOimNcPR4prQer4AeulxV71Tpl8bU2Yp60KxJ 887aEDUFsJ6zuVIyj4ZPkyXMktPKl6xqRyzOWMKia9L0P5aVFwYveQQBUW6xmnZFAXzx veP7b225BwzjYsDWzfYlbeU2/wdOzlbhOouDpxqBabxtTQTK9M1qBLeVxhpoCTi/G519 oplupF1/PmFg3Kb2EE4YUDAwT6w+aNSgitKfdhPCbTXc6PkJc2JeVjcxd4eWTl5JuLXL VRTg== X-Gm-Message-State: AGi0PubA0mH8RvpuxkygFBCmwjEmvyNZzz83c/uv9kG8bJoWdfO6VtF1 Bz6rcKComLMhZ5Ad9jqiIlkV3cHV X-Google-Smtp-Source: APiQypJbrUTbfduhFZPwwgwLYhIpvO7CKMr5Hb/YqHHBKEkA2l2FvclNyfGpC7aARvDt9+adAk0Mmw== X-Received: by 2002:a1c:4ca:: with SMTP id 193mr6744236wme.18.1588462068818; Sat, 02 May 2020 16:27:48 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id v16sm6357039wml.30.2020.05.02.16.27.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 May 2020 16:27:48 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 3 May 2020 01:27:41 +0200 Message-Id: <20200502232741.19205-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avformat/oggenc: Avoid allocations and copying when writing page data 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" When the Ogg muxer writes a page, it has to do three things: It needs to write a page header, then it has to actually copy the page data and then it has to calculate and write a CRC checksum of both header as well as data at a certain position in the page header. To do this, the muxer used a dynamic buffer for both writing as well as calculating the checksum via an AVIOContext's feature to automatically calculate checksums on the data it writes. This entails an allocation of an AVIOContext, of the opaque specific to dynamic buffers and of the buffer itself (which may be reallocated multiple times) as well as memcopying the data (first into the AVIOContext's small write buffer, then into the dynamic buffer's big buffer). This commit changes this: The page header is no longer written into a dynamic buffer any more; instead the (small) page header is written into a small buffer on the stack. The CRC is then calculated directly via av_crc() on both the page header as well as the page data. Then both the page header and the page data are written. Finally, ogg_write_page() can now no longer fail, so it has been modified to return nothing; this also fixed a bug in the only caller of this function: It didn't check the return value. Signed-off-by: Andreas Rheinhardt --- libavformat/oggenc.c | 63 ++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 41 deletions(-) diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c index fbd14fedf9..f81907117e 100644 --- a/libavformat/oggenc.c +++ b/libavformat/oggenc.c @@ -29,7 +29,6 @@ #include "libavcodec/bytestream.h" #include "libavcodec/flac.h" #include "avformat.h" -#include "avio_internal.h" #include "internal.h" #include "vorbiscomment.h" @@ -99,50 +98,32 @@ static const AVClass flavor ## _muxer_class = {\ .version = LIBAVUTIL_VERSION_INT,\ }; -static void ogg_update_checksum(AVFormatContext *s, AVIOContext *pb, int64_t crc_offset) -{ - int64_t pos = avio_tell(pb); - uint32_t checksum = ffio_get_checksum(pb); - avio_seek(pb, crc_offset, SEEK_SET); - avio_wb32(pb, checksum); - avio_seek(pb, pos, SEEK_SET); -} - -static int ogg_write_page(AVFormatContext *s, OGGPage *page, int extra_flags) +static void ogg_write_page(AVFormatContext *s, OGGPage *page, int extra_flags) { OGGStreamContext *oggstream = s->streams[page->stream_index]->priv_data; - AVIOContext *pb; - int64_t crc_offset; - int ret, size; - uint8_t *buf; - - ret = avio_open_dyn_buf(&pb); - if (ret < 0) - return ret; - ffio_init_checksum(pb, ff_crc04C11DB7_update, 0); - ffio_wfourcc(pb, "OggS"); - avio_w8(pb, 0); - avio_w8(pb, page->flags | extra_flags); - avio_wl64(pb, page->granule); - avio_wl32(pb, oggstream->serial_num); - avio_wl32(pb, oggstream->page_counter++); - crc_offset = avio_tell(pb); - avio_wl32(pb, 0); // crc - avio_w8(pb, page->segments_count); - avio_write(pb, page->segments, page->segments_count); - avio_write(pb, page->data, page->size); - - ogg_update_checksum(s, pb, crc_offset); - - size = avio_close_dyn_buf(pb, &buf); - if (size < 0) - return size; - - avio_write(s->pb, buf, size); + uint8_t buf[4 + 1 + 1 + 8 + 4 + 4 + 4 + 1 + 255], *ptr = buf, *crc_pos; + const AVCRC *crc_table = av_crc_get_table(AV_CRC_32_IEEE); + uint32_t crc; + + bytestream_put_le32(&ptr, MKTAG('O', 'g', 'g', 'S')); + bytestream_put_byte(&ptr, 0); + bytestream_put_byte(&ptr, page->flags | extra_flags); + bytestream_put_le64(&ptr, page->granule); + bytestream_put_le32(&ptr, oggstream->serial_num); + bytestream_put_le32(&ptr, oggstream->page_counter++); + crc_pos = ptr; + bytestream_put_le32(&ptr, 0); + bytestream_put_byte(&ptr, page->segments_count); + bytestream_put_buffer(&ptr, page->segments, page->segments_count); + + crc = av_crc(crc_table, 0, buf, ptr - buf); + crc = av_crc(crc_table, crc, page->data, page->size); + bytestream_put_be32(&crc_pos, crc); + + avio_write(s->pb, buf, ptr - buf); + avio_write(s->pb, page->data, page->size); avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_FLUSH_POINT); - av_free(buf); oggstream->page_count--; - return 0; } static int ogg_key_granule(OGGStreamContext *oggstream, int64_t granule)