From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 2BF2942D1E for ; Sun, 5 Jun 2022 15:52:42 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3032168B62C; Sun, 5 Jun 2022 18:52:40 +0300 (EEST) Received: from mail-oi1-f173.google.com (mail-oi1-f173.google.com [209.85.167.173]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id AD77C68B58E for ; Sun, 5 Jun 2022 18:52:33 +0300 (EEST) Received: by mail-oi1-f173.google.com with SMTP id r206so16777868oib.8 for ; Sun, 05 Jun 2022 08:52:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to:content-transfer-encoding; bh=j5Gn1zp3tFHIvw6mEKxjRN2KiX8YoVHhbRp1SQ518go=; b=egY+CdtXuoZASYFVM/pDByDwLBdj7LZqArkmQSnli1J2GPNZDGr5/EkKkbWaPxPFbG VfmahE0BTOxdxuCQdRWC6UEhhw14xveim3HvMPXvC2qRMNfWKDQvMMMCf8qSPu6rZyHO TTJCgTBByNL5fid/G+HEYzgqoaA5OqG1P82clIvxse5v0Y3Gr8rZcXZgPfmMM/cE/LvP amUU/E5/z2Ly/nLopF2jXwPy0vnpR3p/sc9prsaVcn8oAOcAHxeB3gDypS4gPvqv6UOd ocKCUC1R+eKFx409NHjOUsjw0ix4mlAglnbb7aa3rU3o+t4JyOZpAb+cBNm80lQvVi0L yrLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=j5Gn1zp3tFHIvw6mEKxjRN2KiX8YoVHhbRp1SQ518go=; b=t66yE8BEO1pzlARaL3/OjoDC1okXk/IIxDP1fdO8EoFj7IwqRVfUsrPlpoV+8IDeCM 44rt93P1gr0NR23Qf7t7UmEa3BXp1S7XxLCcI+Je9/CXqa/j2XWslciP/Sm2wj/KeXNP qX7W02BpPd0kbZ+UgAy+ZEqoK++TKdPfVuTdxKAMbuXRX4R5O63D+m+7Los6Fz35vada c/bdAQVDNoP+CnNxTR92e8vtaTxxcGKarbxqtPAgpYFconSpgAPbCVyPuCT/i0JEUCZd W9yS3LGgyaTaEvB9aCiqRKErhncOhoR/E2gNa173ke22nV25nGwZjCL0T3M29RbKoMEY jhWA== X-Gm-Message-State: AOAM530PEOYa5YuyIqQiX6TsQlTWKgR2hWVDfRUXYFGmjZ9Yj7o4sYA6 FZNlvS4z/BODnmuX9ciLAud6T0a4C+k= X-Google-Smtp-Source: ABdhPJw+LQ57HStHfMNj20+molCnQPjh9eSFx2Zh3lvp1kzmp5y8xkyYiT62C64BQMNrD/m2EkCeCQ== X-Received: by 2002:a05:6808:179c:b0:32b:7493:8e22 with SMTP id bg28-20020a056808179c00b0032b74938e22mr11137291oib.248.1654444351803; Sun, 05 Jun 2022 08:52:31 -0700 (PDT) Received: from [192.168.0.13] ([186.136.131.204]) by smtp.gmail.com with ESMTPSA id g24-20020a544f98000000b00325cda1ffb4sm6924576oiy.51.2022.06.05.08.52.30 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 05 Jun 2022 08:52:31 -0700 (PDT) Message-ID: Date: Sun, 5 Jun 2022 12:52:30 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220605152813.1888-1-jamrial@gmail.com> From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] avutil/frame: add av_frame_replace X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 6/5/2022 12:42 PM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer >> --- >> Now taking into account the new channel layout API fields, supporting non >> refcouted src, plus changing the documentation to state that the dst frame >> properties will be replaced by those from src even if the data described >> by both frames is the same. >> > > Apparently, you didn't get my point last time: What happens if dst and > src coincide did not mean "what happpens if the data both frames refer > to already coincides"; it meant "what happens if src == dst?" (so that > there is only one AVFrame involved). You either disallow this altogether > or you support it. This patch (and the last one) does neither. Ok, will disallow this with if (dst == src) return AVERROR(EINVAL); How does av_frame_ref() handle this scenario, for that matter? The checks i see to ensure dst is clean (and thus one could assume is different than src) are non strict with av_assert1(). > >> Still missing APIChanges entry and version bump. >> >> libavutil/frame.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++ >> libavutil/frame.h | 17 ++++++ >> 2 files changed, 161 insertions(+) >> >> diff --git a/libavutil/frame.c b/libavutil/frame.c >> index 4c16488c66..2a4a2c27ad 100644 >> --- a/libavutil/frame.c >> +++ b/libavutil/frame.c >> @@ -461,6 +461,150 @@ fail: >> return ret; >> } >> >> +int av_frame_replace(AVFrame *dst, const AVFrame *src) >> +{ >> + int i, ret = 0; > > Why don't you use "for (int i = 0;" loop variables? Because i copied this from av_frame_ref(). Will change it. > >> + >> + dst->format = src->format; >> + dst->width = src->width; >> + dst->height = src->height; >> + dst->nb_samples = src->nb_samples; >> +#if FF_API_OLD_CHANNEL_LAYOUT >> +FF_DISABLE_DEPRECATION_WARNINGS >> + dst->channels = src->channels; >> + dst->channel_layout = src->channel_layout; >> + if (!av_channel_layout_check(&src->ch_layout)) { >> + av_channel_layout_uninit(&dst->ch_layout); >> + if (src->channel_layout) >> + av_channel_layout_from_mask(&dst->ch_layout, src->channel_layout); >> + else { >> + dst->ch_layout.nb_channels = src->channels; >> + dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; >> + } >> + } else { >> +#endif >> + ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout); >> + if (ret < 0) >> + goto fail; >> +#if FF_API_OLD_CHANNEL_LAYOUT >> + } >> +FF_ENABLE_DEPRECATION_WARNINGS >> +#endif >> + >> + wipe_side_data(dst); >> + av_dict_free(&dst->metadata); >> + ret = frame_copy_props(dst, src, 0); >> + if (ret < 0) >> + goto fail; >> + >> + /* duplicate the frame data if it's not refcounted */ >> + if (!src->buf[0]) { >> + for (i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++) >> + av_buffer_unref(&dst->buf[i]); >> + for (i = 0; i < dst->nb_extended_buf; i++) >> + av_buffer_unref(&dst->extended_buf[i]); >> + av_freep(&dst->extended_buf); >> + >> + memset(dst->data, 0, sizeof(dst->data)); >> + if (dst->extended_data != dst->data) >> + av_freep(&dst->extended_data); >> + >> + ret = av_frame_get_buffer(dst, 0); >> + if (ret < 0) >> + goto fail; >> + >> + ret = av_frame_copy(dst, src); >> + if (ret < 0) >> + goto fail; >> + >> + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); >> + if (ret < 0) >> + goto fail; >> + >> + return 0; >> + } >> + >> + /* replace the buffers */ >> + for (i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) { >> + ret = av_buffer_replace(&dst->buf[i], src->buf[i]); >> + if (ret < 0) >> + goto fail; >> + } >> + >> + if (src->extended_buf) { >> + if (dst->nb_extended_buf != src->nb_extended_buf) { >> + int nb_extended_buf = FFMIN(dst->nb_extended_buf, src->nb_extended_buf); >> + void *tmp; >> + >> + for (i = nb_extended_buf; i < dst->nb_extended_buf; i++) >> + av_buffer_unref(&dst->extended_buf[i]); >> + >> + tmp = av_realloc_array(dst->extended_buf, sizeof(*dst->extended_buf), >> + src->nb_extended_buf); >> + if (!tmp) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } >> + dst->extended_buf = tmp; >> + dst->nb_extended_buf = src->nb_extended_buf; >> + >> + memset(&dst->extended_buf[nb_extended_buf], 0, >> + (src->nb_extended_buf - nb_extended_buf) * sizeof(*dst->extended_buf)); >> + } >> + >> + for (i = 0; i < src->nb_extended_buf; i++) { >> + ret = av_buffer_replace(&dst->extended_buf[i], src->extended_buf[i]); >> + if (ret < 0) >> + goto fail; >> + } >> + } else if (dst->extended_buf) { >> + for (i = 0; i < dst->nb_extended_buf; i++) >> + av_buffer_unref(&dst->extended_buf[i]); >> + av_freep(&dst->extended_buf); >> + } >> + >> + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); >> + if (ret < 0) >> + goto fail; >> + >> + if (dst->extended_data != dst->data) >> + av_freep(&dst->extended_data); >> + >> + if (src->extended_data != src->data) { >> + int ch = src->ch_layout.nb_channels; >> + >> +#if FF_API_OLD_CHANNEL_LAYOUT >> +FF_DISABLE_DEPRECATION_WARNINGS >> + if (!ch) { >> + ch = src->channels; >> + CHECK_CHANNELS_CONSISTENCY(src); >> + } >> +FF_ENABLE_DEPRECATION_WARNINGS >> +#endif >> + if (!ch) { >> + ret = AVERROR(EINVAL); >> + goto fail; >> + } >> + >> + dst->extended_data = av_malloc_array(sizeof(*dst->extended_data), ch); >> + if (!dst->extended_data) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } >> + memcpy(dst->extended_data, src->extended_data, sizeof(*src->extended_data) * ch); >> + } else >> + dst->extended_data = dst->data; >> + >> + memcpy(dst->data, src->data, sizeof(src->data)); >> + memcpy(dst->linesize, src->linesize, sizeof(src->linesize)); >> + >> + return 0; >> + >> +fail: >> + av_frame_unref(dst); >> + return ret; >> +} >> + >> AVFrame *av_frame_clone(const AVFrame *src) >> { >> AVFrame *ret = av_frame_alloc(); >> diff --git a/libavutil/frame.h b/libavutil/frame.h >> index 33fac2054c..a971b1655e 100644 >> --- a/libavutil/frame.h >> +++ b/libavutil/frame.h >> @@ -752,6 +752,23 @@ void av_frame_free(AVFrame **frame); >> */ >> int av_frame_ref(AVFrame *dst, const AVFrame *src); >> >> +/** >> + * Ensure the destination frame refers to the same data described by the source >> + * frame by creating a new reference for each AVBufferRef from src if they >> + * differ from those in dst, or if src is not reference counted, by allocating >> + * new buffers and copying data. >> + * >> + * Frame properties on dst will be replaced by those from src. >> + * >> + * @param src The source frame. If there's data described in it, it must be >> + * reference counted. > > The text above says "if src is not reference counted, by allocating new > buffers and copying data". Remnant from the previous version. Will fix. > >> + * @param dst The destination frame. >> + * >> + * @return 0 on success, a negative AVERROR on error. On error, dst is >> + * unreferenced. >> + */ >> +int av_frame_replace(AVFrame *dst, const AVFrame *src); >> + >> /** >> * Create a new frame that references the same data as src. >> * > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".