From patchwork Mon Dec 7 03:56:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 24377 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 DF76D44B9AC for ; Mon, 7 Dec 2020 05:56:28 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BD92168AACC; Mon, 7 Dec 2020 05:56:28 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C968168A9D5 for ; Mon, 7 Dec 2020 05:56:22 +0200 (EET) Received: by mail-wm1-f65.google.com with SMTP id g25so8698182wmh.1 for ; Sun, 06 Dec 2020 19:56:22 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=TpKxpAB27/iKkhJKnkeNEFm6vhxHd/hdVTtJqTljOuw=; b=BbveCVyl8vTdQA4JKt7SwiYIb/j1nXOdP+i5KouHaNpcIOGTGCNWSNoL4MNH7W1nr4 4PNrIa71T4NYf/+ofTzXINYA3NWpOp7NBhFuIhv06EizTt6ZK+qjXqWAf2p+eB/XiIAS nqdhzQqINWPaoXnxpm+2GspVJH+e9Gh/umbSdu6J5P+1ycIiI0Lz2T+rsQR/gSJvZAxi ceFWIuCply8NamYSsD206EeoDpGa8vUG7yEw6gwhdNelyJa00iGD5jbxlETU/77tHLcC 49c56duXc8YwB84nKJvVliBOs+rS843TweQXKPfhgLqtJoELQuY1TdVCcz7vBCXdVg/9 rq5g== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=TpKxpAB27/iKkhJKnkeNEFm6vhxHd/hdVTtJqTljOuw=; b=AeppobBe9H0RcxYtMqPNnOULfL80u+PJ2lUtX+v5he+t5rDN2cTrHnWHBZQBxMNLfM Sx86Z1P/FNNVFeg/JdJXHUAPDh8UfFAsCyi/YqsV3nWJ4D4fM4Jleeab6KN/z8e6GkVB PuGK1CXuQQkWvkPVOm8fbMEmVepDJA5FFb+9b6I7+M5+/0I+qtIeLb5SZ1lzKs74kRYH MJFWhY9h1B6uPZfxU4Mf57ROs+HkoJiF3hhMOiqh6IrT1dhiRpzX+9uMDNBNr5/01gZl 2iCJoBQVxDDWE/vOBx1E9+1VjEroISPM5wd8E/azlCa4vRo9k8AgPsh0eVv6+EmvMMeJ lSAA== X-Gm-Message-State: AOAM531XROOspR84mHIX9MvC8k3ihHuNvcLjib3WRAtCNAPf5UyFjDci /Whhm72QeLBeyHlXavCRvebZ/h+nMSlpZQ== X-Google-Smtp-Source: ABdhPJyk6rGuM3iudk8rp/gC/w0sJiZgUmfA3LC5GtwXHwKE/X/jbFsxCA/p10+i1vRXqClkBpOTag== X-Received: by 2002:a1c:1f54:: with SMTP id f81mr16271249wmf.44.1607313381768; Sun, 06 Dec 2020 19:56:21 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id k6sm12262309wmf.25.2020.12.06.19.56.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 06 Dec 2020 19:56:20 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 7 Dec 2020 04:56:09 +0100 Message-Id: <20201207035609.546350-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201206030934.395352-2-andreas.rheinhardt@gmail.com> References: <20201206030934.395352-2-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v3] avformat/framecrcenc: Make side-data checksums endian-independent 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" Do this by converting big-endian side data to little endian for checksumming. Reviewed-by: Andriy Gelman Reviewed-by: Michael Niedermayer Signed-off-by: Andreas Rheinhardt --- I have incorporated Michael's resp. Andriy's suggestion regarding av_unused resp. renaming the buffer for the actual output. But I have left hashenc.c untouched. Unfortunately one can't simply merge framecrc into hashenc.c without changing every checksum, because the av_hash_* API initializes its adler-32 checksums to 1, whereas framecrc initializes it to 0. The latter seems to be wrong; if one intends to switch to the former, then I suggest to use the framehash muxer in FATE instead of framecrc and deprecate framecrc without modifying it and make framehash endian-independent (essentially using the patch here); after all, other people may use it, too. The diff would be huge (more than 70000 lines of removal), but if that is preferred, I can do so. libavformat/framecrcenc.c | 59 +++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/libavformat/framecrcenc.c b/libavformat/framecrcenc.c index f7c48779a0..1fbe4aa4ee 100644 --- a/libavformat/framecrcenc.c +++ b/libavformat/framecrcenc.c @@ -21,9 +21,11 @@ #include +#include "config.h" #include "libavutil/adler32.h" #include "libavutil/avstring.h" #include "libavutil/intreadwrite.h" +#include "libavcodec/avcodec.h" #include "avformat.h" #include "internal.h" @@ -43,6 +45,17 @@ static int framecrc_write_header(struct AVFormatContext *s) return ff_framehash_write_header(s); } +static av_unused void inline bswap(char *buf, int offset, int size) +{ + if (size == 8) { + uint64_t val = AV_RN64(buf + offset); + AV_WN64(buf + offset, av_bswap64(val)); + } else if (size == 4) { + uint32_t val = AV_RN32(buf + offset); + AV_WN32(buf + offset, av_bswap32(val)); + } +} + static int framecrc_write_packet(struct AVFormatContext *s, AVPacket *pkt) { uint32_t crc = av_adler32_update(0, pkt->data, pkt->size); @@ -58,17 +71,53 @@ static int framecrc_write_packet(struct AVFormatContext *s, AVPacket *pkt) for (i=0; iside_data_elems; i++) { const AVPacketSideData *const sd = &pkt->side_data[i]; + const uint8_t *data = sd->data; uint32_t side_data_crc = 0; - if (HAVE_BIGENDIAN && AV_PKT_DATA_PALETTE == pkt->side_data[i].type) { + + switch (sd->type) { +#if HAVE_BIGENDIAN + uint8_t bswap_buf[FFMAX(sizeof(AVCPBProperties), + sizeof(AVProducerReferenceTime))]; + case AV_PKT_DATA_PALETTE: + case AV_PKT_DATA_REPLAYGAIN: + case AV_PKT_DATA_DISPLAYMATRIX: + case AV_PKT_DATA_STEREO3D: + case AV_PKT_DATA_AUDIO_SERVICE_TYPE: + case AV_PKT_DATA_FALLBACK_TRACK: + case AV_PKT_DATA_MASTERING_DISPLAY_METADATA: + case AV_PKT_DATA_SPHERICAL: + case AV_PKT_DATA_CONTENT_LIGHT_LEVEL: + case AV_PKT_DATA_S12M_TIMECODE: for (int j = 0; j < sd->size / 4; j++) { uint8_t buf[4]; AV_WL32(buf, AV_RB32(sd->data + 4 * j)); side_data_crc = av_adler32_update(side_data_crc, buf, 4); } - } else { - side_data_crc = av_adler32_update(0, - pkt->side_data[i].data, - pkt->side_data[i].size); + break; + case AV_PKT_DATA_CPB_PROPERTIES: +#define BSWAP(struct, field) bswap(bswap_buf, offsetof(struct, field), sizeof(((struct){0}).field)) + if (sd->size == sizeof(AVCPBProperties)) { + memcpy(bswap_buf, sd->data, sizeof(AVCPBProperties)); + data = bswap_buf; + BSWAP(AVCPBProperties, max_bitrate); + BSWAP(AVCPBProperties, min_bitrate); + BSWAP(AVCPBProperties, avg_bitrate); + BSWAP(AVCPBProperties, buffer_size); + BSWAP(AVCPBProperties, vbv_delay); + } + goto pod; + case AV_PKT_DATA_PRFT: + if (sd->size == sizeof(AVProducerReferenceTime)) { + memcpy(bswap_buf, sd->data, sizeof(AVProducerReferenceTime)); + data = bswap_buf; + BSWAP(AVProducerReferenceTime, wallclock); + BSWAP(AVProducerReferenceTime, flags); + } + goto pod; + pod: +#endif + default: + side_data_crc = av_adler32_update(0, data, sd->size); } av_strlcatf(buf, sizeof(buf), ", %8d, 0x%08"PRIx32, pkt->side_data[i].size, side_data_crc); }