From patchwork Mon Jul 29 19:56:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 14128 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 4219644800E for ; Mon, 29 Jul 2019 23:03:13 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 20FF368A582; Mon, 29 Jul 2019 23:03:13 +0300 (EEST) 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 7A9A1689992 for ; Mon, 29 Jul 2019 23:03:06 +0300 (EEST) Received: by mail-wm1-f65.google.com with SMTP id h19so43755442wme.0 for ; Mon, 29 Jul 2019 13:03:06 -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=9jnZRgr6fen8mJ8b5uC1aMPXicj7hDGNTDoHXm5L814=; b=IOP+2fQZOmLGvcaX7cj6s9BTauK7XbGpbZVayzmvLpR3+wh3NdCMe31tZ+W/9/lmlK rybQ41NUFDnvlx6/WIizDwfuMeWzvjCEusE/KxO44YSaAh64EW/na6QECXU3MPf1Zr/I mTP2DAejgBbpJqaLV0076zglM1nw/QjfV9D6yYqUvFyDyAGKXF8KO/NUjBiJC/sYkB2G 6CMNR9YC/25mB3JNqGhFpZXDzq7/jPKH/aFWf3agpPJ6ZrgKGgMRTuwRqdTS9gDg9Dv1 Hj2+Wpb3m1HFC7IHhFR8IVF3IJ0RsofFAPnOAh4z8kEiKuruTQqa2/U7P8QN2xRSnOHM Nptw== 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=9jnZRgr6fen8mJ8b5uC1aMPXicj7hDGNTDoHXm5L814=; b=m/wYH60XdEV3iICCYWWoyxJjq4LG7INBtdhiJElkAnbi4xyGK6xp9sIS4QLENhtJ/O ZLdPYqxXNgwWOg2w3ZlI/jc5YWgVr1udUbaNphXjRqPG4oMKWwgDj6bYMBfZ4dA+xQ0R wJs69QRXx/9a5pFUVNyPRyzmPWVf1b8ePKgKDbAoCmpNNHWvEJMpI8i8dzoPH9Imd4HM hmz449P25hl4+zSHiN+SUPgbTvpp77zRy0m0DrhH5DVNC9C/wjihLdGcNi0aGGHrKncE bfB5aXYIZR1ITiT+QiZ9BqNVjLSBFu/MJ3BvfTZPmCStpUFVZLBuRgEJ9n545ZUQq5Nn /Tjw== X-Gm-Message-State: APjAAAUHoMH22/F+heBTpJOl412Xi/0DhhUKTyF8Mqn6gRrcbOEGMvLw tYSUc81esqc0+iE98+MFNZLObc+b X-Google-Smtp-Source: APXvYqy3cCe7uj9xaK+Y42DrGeAHgcpY9QqgMHKJXeytWSqaVxhkiutOShhNC+wteT//+lmM6oew8Q== X-Received: by 2002:a1c:e715:: with SMTP id e21mr103761457wmh.16.1564430247422; Mon, 29 Jul 2019 12:57:27 -0700 (PDT) Received: from localhost.localdomain (ipbcc08b8f.dynamic.kabel-deutschland.de. [188.192.139.143]) by smtp.gmail.com with ESMTPSA id t24sm56602211wmj.14.2019.07.29.12.57.26 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 29 Jul 2019 12:57:26 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 29 Jul 2019 21:56:52 +0200 Message-Id: <20190729195658.56078-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/7] cbs: Don't set AVBuffer's opaque 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" cbs is currently inconsistent regarding the opaque field that can be used as a special argument to av_buffer_create in order to be used during freeing the buffer: ff_cbs_alloc_unit_content and all the free functions used name this parameter as if it should contain a pointer to the unit whose content is about to be created; but both ff_cbs_alloc_unit_content as well as ff_cbs_h264_add_sei_message actually use a pointer to the CodedBitstreamContext as opaque. It should actually be neither, because it is unneeded (as is evidenced by the fact that none of the free functions use this pointer at all) and because it ties the unit's content to the lifetime of other objects, although a refcounted buffer is supposed to have its own lifetime that only ends when its reference count reaches zero. This problem manifests itself in the pointer becoming dangling. The pointer to the unit can become dangling if another unit is added to the fragment later as happens in the bitstream filters; in this case, the pointer can point to the wrong unit (if the fragment's unit array needn't be relocated) or it can point to where the array was earlier. It can also become dangling if the unit's content is meant to survive the resetting of the fragment it was originally read with. This applies to the extradata of H.264 and HEVC. The pointer to the context can become dangling if the context is closed before the content is freed. Although this doesn't seem to happen right now, it could happen, in particular if one uses different CodedBitstreamContexts for in- and output. Signed-off-by: Andreas Rheinhardt --- libavcodec/cbs.c | 2 +- libavcodec/cbs.h | 2 +- libavcodec/cbs_av1.c | 2 +- libavcodec/cbs_h2645.c | 18 +++++++++--------- libavcodec/cbs_jpeg.c | 6 +++--- libavcodec/cbs_mpeg2.c | 6 +++--- libavcodec/cbs_vp9.c | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index 2350416501..1a43cd2694 100644 --- a/libavcodec/cbs.c +++ b/libavcodec/cbs.c @@ -597,7 +597,7 @@ int ff_cbs_alloc_unit_content(CodedBitstreamContext *ctx, return AVERROR(ENOMEM); unit->content_ref = av_buffer_create(unit->content, size, - free, ctx, 0); + free, NULL, 0); if (!unit->content_ref) { av_freep(&unit->content); return AVERROR(ENOMEM); diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h index fe57e7b2a5..7c341bffe7 100644 --- a/libavcodec/cbs.h +++ b/libavcodec/cbs.h @@ -341,7 +341,7 @@ void ff_cbs_fragment_free(CodedBitstreamContext *ctx, int ff_cbs_alloc_unit_content(CodedBitstreamContext *ctx, CodedBitstreamUnit *unit, size_t size, - void (*free)(void *unit, uint8_t *content)); + void (*free)(void *opaque, uint8_t *content)); /** * Allocate a new internal data buffer of the given size in the unit. diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c index 288ef8e14b..98cd37ef74 100644 --- a/libavcodec/cbs_av1.c +++ b/libavcodec/cbs_av1.c @@ -829,7 +829,7 @@ static void cbs_av1_free_metadata(AV1RawMetadata *md) } } -static void cbs_av1_free_obu(void *unit, uint8_t *content) +static void cbs_av1_free_obu(void *opaque, uint8_t *content) { AV1RawOBU *obu = (AV1RawOBU*)content; diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index 0f697434e5..1c35be51e7 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -443,7 +443,7 @@ static int cbs_h2645_read_more_rbsp_data(GetBitContext *gbc) #undef allocate -static void cbs_h264_free_pps(void *unit, uint8_t *content) +static void cbs_h264_free_pps(void *opaque, uint8_t *content) { H264RawPPS *pps = (H264RawPPS*)content; av_buffer_unref(&pps->slice_group_id_ref); @@ -473,7 +473,7 @@ static void cbs_h264_free_sei_payload(H264RawSEIPayload *payload) } } -static void cbs_h264_free_sei(void *unit, uint8_t *content) +static void cbs_h264_free_sei(void *opaque, uint8_t *content) { H264RawSEI *sei = (H264RawSEI*)content; int i; @@ -482,35 +482,35 @@ static void cbs_h264_free_sei(void *unit, uint8_t *content) av_freep(&content); } -static void cbs_h264_free_slice(void *unit, uint8_t *content) +static void cbs_h264_free_slice(void *opaque, uint8_t *content) { H264RawSlice *slice = (H264RawSlice*)content; av_buffer_unref(&slice->data_ref); av_freep(&content); } -static void cbs_h265_free_vps(void *unit, uint8_t *content) +static void cbs_h265_free_vps(void *opaque, uint8_t *content) { H265RawVPS *vps = (H265RawVPS*)content; av_buffer_unref(&vps->extension_data.data_ref); av_freep(&content); } -static void cbs_h265_free_sps(void *unit, uint8_t *content) +static void cbs_h265_free_sps(void *opaque, uint8_t *content) { H265RawSPS *sps = (H265RawSPS*)content; av_buffer_unref(&sps->extension_data.data_ref); av_freep(&content); } -static void cbs_h265_free_pps(void *unit, uint8_t *content) +static void cbs_h265_free_pps(void *opaque, uint8_t *content) { H265RawPPS *pps = (H265RawPPS*)content; av_buffer_unref(&pps->extension_data.data_ref); av_freep(&content); } -static void cbs_h265_free_slice(void *unit, uint8_t *content) +static void cbs_h265_free_slice(void *opaque, uint8_t *content) { H265RawSlice *slice = (H265RawSlice*)content; av_buffer_unref(&slice->data_ref); @@ -545,7 +545,7 @@ static void cbs_h265_free_sei_payload(H265RawSEIPayload *payload) } } -static void cbs_h265_free_sei(void *unit, uint8_t *content) +static void cbs_h265_free_sei(void *opaque, uint8_t *content) { H265RawSEI *sei = (H265RawSEI*)content; int i; @@ -1615,7 +1615,7 @@ int ff_cbs_h264_add_sei_message(CodedBitstreamContext *ctx, sei->nal_unit_header.nal_ref_idc = 0; sei_ref = av_buffer_create((uint8_t*)sei, sizeof(*sei), - &cbs_h264_free_sei, ctx, 0); + &cbs_h264_free_sei, NULL, 0); if (!sei_ref) { av_freep(&sei); return AVERROR(ENOMEM); diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c index c46abd0a45..a20f062f1b 100644 --- a/libavcodec/cbs_jpeg.c +++ b/libavcodec/cbs_jpeg.c @@ -82,21 +82,21 @@ #undef xu -static void cbs_jpeg_free_application_data(void *unit, uint8_t *content) +static void cbs_jpeg_free_application_data(void *opaque, uint8_t *content) { JPEGRawApplicationData *ad = (JPEGRawApplicationData*)content; av_buffer_unref(&ad->Ap_ref); av_freep(&content); } -static void cbs_jpeg_free_comment(void *unit, uint8_t *content) +static void cbs_jpeg_free_comment(void *opaque, uint8_t *content) { JPEGRawComment *comment = (JPEGRawComment*)content; av_buffer_unref(&comment->Cm_ref); av_freep(&content); } -static void cbs_jpeg_free_scan(void *unit, uint8_t *content) +static void cbs_jpeg_free_scan(void *opaque, uint8_t *content) { JPEGRawScan *scan = (JPEGRawScan*)content; av_buffer_unref(&scan->data_ref); diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c index 516b728a64..3e6797bd42 100644 --- a/libavcodec/cbs_mpeg2.c +++ b/libavcodec/cbs_mpeg2.c @@ -140,21 +140,21 @@ #undef infer -static void cbs_mpeg2_free_picture_header(void *unit, uint8_t *content) +static void cbs_mpeg2_free_picture_header(void *opaque, uint8_t *content) { MPEG2RawPictureHeader *picture = (MPEG2RawPictureHeader*)content; av_buffer_unref(&picture->extra_information_picture.extra_information_ref); av_freep(&content); } -static void cbs_mpeg2_free_user_data(void *unit, uint8_t *content) +static void cbs_mpeg2_free_user_data(void *opaque, uint8_t *content) { MPEG2RawUserData *user = (MPEG2RawUserData*)content; av_buffer_unref(&user->user_data_ref); av_freep(&content); } -static void cbs_mpeg2_free_slice(void *unit, uint8_t *content) +static void cbs_mpeg2_free_slice(void *opaque, uint8_t *content) { MPEG2RawSlice *slice = (MPEG2RawSlice*)content; av_buffer_unref(&slice->header.extra_information_slice.extra_information_ref); diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c index 7102bb87d3..bd172100fc 100644 --- a/libavcodec/cbs_vp9.c +++ b/libavcodec/cbs_vp9.c @@ -474,7 +474,7 @@ static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx, return 0; } -static void cbs_vp9_free_frame(void *unit, uint8_t *content) +static void cbs_vp9_free_frame(void *opaque, uint8_t *content) { VP9RawFrame *frame = (VP9RawFrame*)content; av_buffer_unref(&frame->data_ref);