From patchwork Fri Mar 2 17:30:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 7796 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.181.170 with SMTP id m39csp6473244jaj; Fri, 2 Mar 2018 09:30:40 -0800 (PST) X-Google-Smtp-Source: AG47ELtLPmC1R+d7PTyiQCRmU6vw0wHvAcqPMEjRGXy1LQjMz/XDTyijGiJEYHeJogAZCORN8eqy X-Received: by 10.28.180.132 with SMTP id d126mr2317073wmf.93.1520011840778; Fri, 02 Mar 2018 09:30:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1520011840; cv=none; d=google.com; s=arc-20160816; b=SMojZchfXa79GTJFx5z7/WhlC6edG32YOAU/aoIitTU41wI2jz6BV8v/AobPA6yOlZ jyj0nag92JOq2eTljbQZNA/RMDd3GsWnD8m9cdRTMTdWPSloSkNIcRsr6bOoAYbbUyr8 VwYauyWKtwFYtbsariR1wye29GirmNMRvmJ1qovISMbVewfvwfZ2/GabaVsDvzFaLDI7 Jb29GYKbFGL8Rj81QP7JLA3G6SaQQyMxZzZIHrRl4HT/lFEpXqpo0/QxujzrWD0bVDjE ttoUnV4fY3FwyXljX7zPvjd9k7j0rqrq8Er3o521mgKpgsv6y2Y1KAs1PHgH5mNLmJGN 4KzA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:to:dkim-signature:delivered-to :arc-authentication-results; bh=qi7Mu7qM56DzXZVLkd7w0oj+c4lj6kslaABGmFXY2e8=; b=LGBpAyQxD0UEZ8Z3iVZ4gWHbiIL3I1kygje8YQ6YPLxXJiZrC0eWqrdMsA0BLM3DLX 2dKjdq90VeiwKbEkaB6QHSyeZtHmSJ09ZZa2ErT+emASEa76+1C47c6a5VnK7uzOj3X7 0tAmuq+1NJrO/ICjO1nA8tTHRTEQY7J0pCSr6NTZAUR+bmC8pFJtm1Oy8rNO1jSNBGgN wbTHBP7hUCGJRD2hsyMtTr0/SnarSWizFcSrltsJAQ3vzkR8LiqsA0EbWEHj2H8SXgi2 yYq2QEIgp6HlwrpSWHAa5SKeECxmzQgjNTePaH1T1QvkOPNRZ2a6XbbSDO6GYz40J6eN VeVQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=LRf7DpkH; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id i12si1132145wmd.215.2018.03.02.09.30.39; Fri, 02 Mar 2018 09:30:40 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=LRf7DpkH; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 80E5C68A67D; Fri, 2 Mar 2018 19:30:32 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qt0-f196.google.com (mail-qt0-f196.google.com [209.85.216.196]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7A02968A65E for ; Fri, 2 Mar 2018 19:30:26 +0200 (EET) Received: by mail-qt0-f196.google.com with SMTP id t6so12781395qtn.9 for ; Fri, 02 Mar 2018 09:30:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language; bh=nck4ZSo4ASpWQYPxTLIoxDjri2+/5b1Xn+dicqN8tqE=; b=LRf7DpkHnEMXIOO/rfIP3RCiUZ7zzMOpe6nDjTFTAHehFdsQR+cAbREftDrMWwEnE6 sUipKLBpGoGg0hPULKgjgol6+pKuiKYX19X+44Z7A4RMOA5N4TUbj2kta9e/tToAwAJo B+7GzdFMdSN4OQhxINsSaA9U1sLt0xNYS88QiH0uMm3ezjNrcBXSgGkQ2M3T7tuaX6Tu WjWm7E/qb7BnN8g4evAlgvdMEtFoKgg7XG/mIGfG4fq0m0P/Jr9PcVOCD3WQ8nYplxRN o5NlJl61ul3BrnAuGfsL+yB+LFnakSXmUmFYmDs5KgTXBO7UNvEfhRLq+I47+SmzyXZE ZdNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=nck4ZSo4ASpWQYPxTLIoxDjri2+/5b1Xn+dicqN8tqE=; b=oe2VwdRALwP22gFTaM3mP0+1J5o/mn/sFyScMyVzRbsJ5xwVSCfiAml/dUsKS4GGOO GIhI16MyQwmKkTxW/Uotogosx2ceg+U3Sn/KqL6trVU2YfOdOoFo6Me7cAhb5X6xqOM2 84nwmpTFnD4YiwX/I1P3M6xrgU41WxSEHS/D8W3Yet2//srubuK717Iy81Nki9oUaqVm K2XA/eJ+el4ko/OC3bfT6z3urY/anOFnwy5iKq4vel7xMvff6ocMEphJJE//dgnt/KaV 6ZK4rYyDd8zinliZNLu/xNPybZ89tDleBgZz7xzVrva7DnU38Ll6MkkUSADpjaDRSaMr ukhA== X-Gm-Message-State: AElRT7FIvDRat3WUtPgL+B3dNk1nd3qibMN0B+u5+4RA7sqqz4dOGe6H tSCuNd+AFc+3H6tjU2MIk+K7kkbv X-Received: by 10.200.52.41 with SMTP id u38mr9548105qtb.172.1520011831193; Fri, 02 Mar 2018 09:30:31 -0800 (PST) Received: from [192.168.0.4] ([181.229.225.176]) by smtp.gmail.com with ESMTPSA id m8sm5054627qkm.26.2018.03.02.09.30.30 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Mar 2018 09:30:30 -0800 (PST) To: ffmpeg-devel@ffmpeg.org References: <20180302011018.GD23018@michaelspb> <20180302111637.27597-1-nfxjfg@googlemail.com> <20180302174710.351ef8f6@debian> From: James Almer Message-ID: <8eda0a9a-b4ea-18c6-231c-668509b17e9b@gmail.com> Date: Fri, 2 Mar 2018 14:30:28 -0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180302174710.351ef8f6@debian> Content-Language: en-US Subject: Re: [FFmpeg-devel] [PATCH v2] lavu/frame: add QP side 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 3/2/2018 1:47 PM, wm4 wrote: > On Fri, 2 Mar 2018 13:11:35 -0300 > James Almer wrote: > >> On 3/2/2018 8:16 AM, wm4 wrote: >>> This adds a way for an API user to transfer QP data and metadata without >>> having to keep the reference to AVFrame, and without having to >>> explicitly care about QP APIs. It might also provide a way to finally >>> remove the deprecated QP related fields. In the end, the QP table should >>> be handled in a very similar way to e.g. AV_FRAME_DATA_MOTION_VECTORS. >>> >>> There are two side data types, because I didn't care about having to >>> repack the QP data so the table and the metadata are in a single >>> AVBufferRef. Otherwise it would have either required a copy on decoding >>> (extra slowdown for something as obscure as the QP data), or would have >>> required making intrusive changes to the codecs which support export of >>> this data. >> >> Why not add an AVBufferRef pointer to the qp_properties struct instead? >> You don't need to merge the properties fields into the buffer data. > > Not sure what you mean. The whole purpose of this is _not_ to add new > pointers because it'd require an API user to deal with extra fields > just for QP. I want to pretend that QP doesn't exist. I mean keep the buffer and the int fields all in a single opaque (for the user) struct handled by a single side data type. The user still only needs to worry about using the get/set functions and nothing else. See the attached, untested PoC to get an idea of what i mean. If I'm really missing the entire point of this patch (Which i don't discard may be the case), then ignore this. diff --git a/libavutil/frame.c b/libavutil/frame.c index 0db2a2d57b..b349ff84fe 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -46,8 +46,28 @@ MAKE_ACCESSORS(AVFrame, frame, enum AVColorRange, color_range) av_get_channel_layout_nb_channels((frame)->channel_layout)) #if FF_API_FRAME_QP +struct qp_properties { + AVBufferRef *buf; + int stride; + int type; +}; + +// Free the qp_properties struct and the QP data buffer +// To be used as the free() function for the side data buffer. +static void frame_qp_free(void *opaque, uint8_t *data) +{ + struct qp_properties *p = (struct qp_properties *)data; + + av_buffer_unref(&p->buf); + av_free(data); +} + int av_frame_set_qp_table(AVFrame *f, AVBufferRef *buf, int stride, int qp_type) { + struct qp_properties *p = NULL; + AVFrameSideData *sd; + AVBufferRef *sd_buf = NULL, *ref = NULL; + FF_DISABLE_DEPRECATION_WARNINGS av_buffer_unref(&f->qp_table_buf); @@ -57,20 +77,69 @@ FF_DISABLE_DEPRECATION_WARNINGS f->qscale_type = qp_type; FF_ENABLE_DEPRECATION_WARNINGS + av_frame_remove_side_data(f, AV_FRAME_DATA_QP_TABLE); + + // Create a new QP data table ref for the side data + ref = av_buffer_ref(buf); + if (!ref) + return AVERROR(ENOMEM); + + // Alloc the qp_properties struct + p = av_malloc(sizeof(struct qp_properties)); + if (!p) + goto fail; + + // Create a buffer to be used in side data, containing the qp_properties struct. + // Use a custom free() function to properly unref the QP table buffer when the side data + // is removed. + sd_buf = av_buffer_create((uint8_t *)p, sizeof(struct qp_properties), frame_qp_free, NULL, 0); + if (!sd_buf) + goto fail; + + // Add the buffer containing the qp_properties struct as side data + sd = av_frame_new_side_data_from_buf(f, AV_FRAME_DATA_QP_TABLE, sd_buf); + if (!sd) + goto fail; + + // Fill the prop fields and the QP table buffer. + p->stride = stride; + p->type = qp_type; + p->buf = ref; + return 0; +fail: + av_buffer_unref(&ref); + av_buffer_unref(&sd_buf); + av_free(p); + return AVERROR(ENOMEM); } int8_t *av_frame_get_qp_table(AVFrame *f, int *stride, int *type) { -FF_DISABLE_DEPRECATION_WARNINGS - *stride = f->qstride; - *type = f->qscale_type; + AVBufferRef *buf = NULL; - if (!f->qp_table_buf) - return NULL; + *stride = 0; + *type = 0; - return f->qp_table_buf->data; +FF_DISABLE_DEPRECATION_WARNINGS + if (f->qp_table_buf) { + *stride = f->qstride; + *type = f->qscale_type; + buf = f->qp_table_buf; FF_ENABLE_DEPRECATION_WARNINGS + } else { + AVFrameSideData *sd; + struct qp_properties *p; + sd = av_frame_get_side_data(f, AV_FRAME_DATA_QP_TABLE); + if (!sd) + return NULL; + p = (struct qp_properties *)sd->data; + *stride = p->stride; + *type = p->type; + buf = p->buf; + } + + return buf ? buf->data : NULL; } #endif @@ -787,6 +856,7 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type) case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL: return "Content light level metadata"; case AV_FRAME_DATA_GOP_TIMECODE: return "GOP timecode"; case AV_FRAME_DATA_ICC_PROFILE: return "ICC profile"; + case AV_FRAME_DATA_QP_TABLE: return "QP table"; } return NULL; } diff --git a/libavutil/frame.h b/libavutil/frame.h index ddbac3156d..dd1917882b 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -141,6 +141,16 @@ enum AVFrameSideDataType { * metadata key entry "name". */ AV_FRAME_DATA_ICC_PROFILE, + +#if FF_API_FRAME_QP + /** + * QP table data. + * The contents of this side data are undocumented and internal; use + * av_frame_set_qp_table() and av_frame_get_qp_table() to access this in a + * meaningful way instead. + */ + AV_FRAME_DATA_QP_TABLE, +#endif }; enum AVActiveFormatDescription {