From patchwork Wed Mar 13 00:17:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Thompson X-Patchwork-Id: 12296 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 A732B447D88 for ; Wed, 13 Mar 2019 02:18:01 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8B2B268A7E2; Wed, 13 Mar 2019 02:18:01 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1C80068A43E for ; Wed, 13 Mar 2019 02:17:55 +0200 (EET) Received: by mail-wr1-f51.google.com with SMTP id p8so49184wrq.6 for ; Tue, 12 Mar 2019 17:17:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=Dn0Bw1jjx5JvtM2khDKlKtKk6/K1M+bohOTG3qaFPCM=; b=veHKCpFDsM6Wxq0cjNOkhjHr5sytOLhKcxKzc/2yDjnGteMxUi7rNRVxBTNOg7HHhk 7rfB01Pz3sLbql2SqwV1FmuclyKYHH12IDNq7mWhbVqCxVAIHrt+ceIQd3BMQ/9Wui8x FsOB9b0rGb5peXy01jtNzILHl3eJMzUHwjWrsgaslgxymMFylG1/VNhJEjTJ64Rusaeu uRsY6hHfbFpvBZqu4MKGHMHXdGFUW8AhIrXzsz2TEB2FiZySj3lquRcF9lz1ei0VuiUD WuDwoEltcxsuunWTgJvfYFhUQIeGGdWc3oPOSr+VY4YAlLna3myPKVeyUgfgERXMCiwg 8pWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=Dn0Bw1jjx5JvtM2khDKlKtKk6/K1M+bohOTG3qaFPCM=; b=Z99mZSghHyGfajdqe3f5cHTLLbKz6mJLo1wAl6M4BGvmMxbtnBj6jf3wwYkpVbLB6C LqMkwQullAs85J4NK8WP5o7z/ncMYrsbp+kwCd+MEh2kHupvUpjO6aNkHr3dQ2EWCbXI RkW8Xnc6mUVf9sQuMLQAcpBUV2mSTRVOlG6XEy6eSWp3b6tcYDYL3SsDQjDgoU7zyW2Q rbLoHzDUU4jaOHJ0a9SpTSuL2qFTlE8UaNKyL9LJ2tXenuXlF+DPJGEudt1fJEcuiEMl Ak2RQR3xRtshDwM42hNksX362Yj/o5/IAg619ZfOJk4+aKi6Ne+FOT95jX9r6lB+mYM7 6QnQ== X-Gm-Message-State: APjAAAXfqbH7RoAXR5YAq+N/Gi0wgqRu1Gk9Gs07sxTeL40eYHIhz+G8 7x3H1tMNKIlZfUV1/WgVscaQI0aV/XE= X-Google-Smtp-Source: APXvYqwBjU1i9EjDXE66AABUi9KjjrYkcDn2fT+3Q/5BtkdFBaXZCd+mle+/+W+w2kx6laGdjkOn8Q== X-Received: by 2002:adf:cd87:: with SMTP id q7mr27257961wrj.92.1552436273891; Tue, 12 Mar 2019 17:17:53 -0700 (PDT) Received: from rywe.jkqxz.net (cpc91242-cmbg18-2-0-cust650.5-4.cable.virginm.net. [82.8.130.139]) by smtp.gmail.com with ESMTPSA id u17sm32505710wrg.71.2019.03.12.17.17.52 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Mar 2019 17:17:53 -0700 (PDT) From: Mark Thompson To: ffmpeg-devel@ffmpeg.org Date: Wed, 13 Mar 2019 00:17:43 +0000 Message-Id: <20190313001748.11781-1-sw@jkqxz.net> X-Mailer: git-send-email 2.19.2 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 1/6] lavu/frame: Change region of interest rectangle definition 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" The definition here previously matched the way cropping worked, but the actual uses in libavcodec instead treated the values as the coordinates of the top-left and bottom-right corners. The most common way of defining rectangles in FFmpeg appears to be as coordinates of the top-left corner combined with a width and height; so, change to using that instead. This is technically both an API and an ABI break, but since the structure was added recently and the previous definition was never actually used correctly I believe that this should not cause any problems. --- This seems to be by far the most common choice of rectangle definition, so I think the most consistent one to use. If you think the break is a step too far (if there are active users then maybe it's left a bit too long since the API was added) then please do say and I'll revert back to the version keeping the currently-documented cropping behaviour. (Actual version bump to add.) - Mark doc/APIchanges | 3 +++ libavcodec/libx264.c | 8 ++++---- libavcodec/libx265.c | 8 ++++---- libavutil/frame.h | 20 ++++++++++++-------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index 784a5e5bd2..4e3b5d6b5a 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2019-03-XX - XXXXXXXXXX - lavc 56.XX.100 - frame.h + Change definition of AVRegionOfInterest. + 2019-01-27 - XXXXXXXXXX - lavc 58.46.100 - avcodec.h Add discard_damaged_percentage diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index a3493f393d..87e6fe7c94 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -369,10 +369,10 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame, nb_rois = sd->size / sizeof(AVRegionOfInterest); roi = (AVRegionOfInterest*)sd->data; for (int count = 0; count < nb_rois; count++) { - int starty = FFMIN(mby, roi->top / MB_SIZE); - int endy = FFMIN(mby, (roi->bottom + MB_SIZE - 1)/ MB_SIZE); - int startx = FFMIN(mbx, roi->left / MB_SIZE); - int endx = FFMIN(mbx, (roi->right + MB_SIZE - 1)/ MB_SIZE); + int starty = av_clip(roi->y / MB_SIZE, 0, mby); + int endy = av_clip((roi->y + roi->height + MB_SIZE - 1) / MB_SIZE, 0, mby); + int startx = av_clip(roi->x / MB_SIZE, 0, mbx); + int endx = av_clip((roi->x + roi->width + MB_SIZE - 1) / MB_SIZE, 0, mbx); float qoffset; if (roi->qoffset.den == 0) { diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c index fe39f45241..c2c1f8b9bc 100644 --- a/libavcodec/libx265.c +++ b/libavcodec/libx265.c @@ -314,10 +314,10 @@ static av_cold int libx265_encode_set_roi(libx265Context *ctx, const AVFrame *fr nb_rois = sd->size / sizeof(AVRegionOfInterest); roi = (AVRegionOfInterest*)sd->data; for (int count = 0; count < nb_rois; count++) { - int starty = FFMIN(mby, roi->top / mb_size); - int endy = FFMIN(mby, (roi->bottom + mb_size - 1)/ mb_size); - int startx = FFMIN(mbx, roi->left / mb_size); - int endx = FFMIN(mbx, (roi->right + mb_size - 1)/ mb_size); + int starty = av_clip(roi->y / mb_size, 0, mby); + int endy = av_clip((roi->y + roi->height + mb_size - 1) / mb_size, 0, mby); + int startx = av_clip(roi->x / mb_size, 0, mbx); + int endx = av_clip((roi->x + roi->width + mb_size - 1) / mb_size, 0, mbx); float qoffset; if (roi->self_size == 0) { diff --git a/libavutil/frame.h b/libavutil/frame.h index 8aa3e88367..cc3b78d8b6 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -212,10 +212,6 @@ typedef struct AVFrameSideData { * self_size specifies the size of this data structure. This value * should be set to sizeof(AVRegionOfInterest). EINVAL is returned if self_size is zero. * - * Number of pixels to discard from the top/bottom/left/right border of - * the frame to obtain the region of interest of the frame. - * They are encoder dependent and will be extended internally - * if the codec requires an alignment. * If the regions overlap, the last value in the list will be used. * * qoffset is quant offset, and base rule here: @@ -228,10 +224,18 @@ typedef struct AVFrameSideData { */ typedef struct AVRegionOfInterest { uint32_t self_size; - int top; - int bottom; - int left; - int right; + /** + * x/y coordinates of the top-left corner and width/height of the + * rectangle defining this region of interest. + * + * The constraints on a region are encoder dependent, so the region + * actually affected may be slightly larger for alignment or other + * reasons. + */ + int x; + int y; + int width; + int height; AVRational qoffset; } AVRegionOfInterest;