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 9F7FA435C2 for ; Fri, 17 Jun 2022 15:10:38 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4AA8E68B90D; Fri, 17 Jun 2022 18:10:35 +0300 (EEST) Received: from btbn.de (btbn.de [136.243.74.85]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D4FEB68B8F0 for ; Fri, 17 Jun 2022 18:10:28 +0300 (EEST) Received: from [authenticated] by btbn.de (Postfix) with ESMTPSA id 7E2D5342B15 for ; Fri, 17 Jun 2022 17:10:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rothenpieler.org; s=mail; t=1655478628; bh=FZG8XqfEO2On9SIh1ohlWRzCKm8JWAOF969NoWzF76M=; h=Date:Subject:To:References:From:In-Reply-To; b=VBXM0A1R5ZG1pirnGN1NZMLBQg6rQgEqelwnEGqbZGM3eShI435F2sTmMl5WsqTtN aluYPz0L2fQPap4S67EyzwXT8P8CgKG8zjyOlFzvqoXtC3VPcTYElkt/8cRjQjfK7w vSaHI3BHzFL2RxxeT23Jt6SiKr9yKYefsk9+vOROyIPBjdhzAi7/Dvqggfz6g3xCXZ k3Dsi/nWOoxBYNMPWyZ421cWcsFdMy268zvDVPKQPu//TMjFPBLOVr1lTJ2/mPex5g deXyMB4AmJPnRV4b8WhC5g8KSv94knUVZerrn3228yWNyYaIg/x5+dcza5/0WjxlGO zB66BMlbT15HA== Message-ID: <4cce0209-f45c-fa9f-8672-91bb6da7c8f0@rothenpieler.org> Date: Fri, 17 Jun 2022 17:10:25 +0200 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: <9642852e9da11714c8643833562b6e86133ce1a1.camel@amazon.it> <0215838ca87728a7e2742ab19af7fe1d5c3f9abc.camel@amazon.it> <2d163f70-25e5-27d6-fc6a-a5a8084a03d7@rothenpieler.org> <31d55286-6118-0438-62d8-c04fcb6d183d@rothenpieler.org> <197e819b43e1c8d203fc035b6fd2ca7f46502ef6.camel@amazon.it> From: Timo Rothenpieler In-Reply-To: <197e819b43e1c8d203fc035b6fd2ca7f46502ef6.camel@amazon.it> Subject: Re: [FFmpeg-devel] [PATCH] Added support for MB_INFO 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 17.06.2022 12:59, Carotti, Elias wrote: >> >> Yes, exactly. It relies on x264 to free it. > > Not really. It can rely on x264 if you explicitly want that behavior. > If you do not set a deallocator, it remains the caller responsibility. > >> What happens if x264 is not involved, but some other encoder, which >> does >> not check for that kind of side data? >> >> How does the caller know that the data was consumed, and the >> ownership >> passed on? > > Again, you don't have to pass the ownership, and in fact in my use case > I do not pass it since I actually recycle and update the same buffer > for subsequent frames. If you do intentionally pass the ownership you > need to be aware of what you are doing. As I said, I see a leak only in > case of a programming error. > Maybe we could explicitly state it in the comment section in mb_info.h: > if you set the deallocator you're relinquishing ownership of the > buffer. I'm not sure if that's something you can sensibly do with side data? What if it gets buffered, copied, and so on? > Plus, there's one more flag (b_mb_info_update) in libx264 which allows > to pass back information to the caller about which macroblocks among > the flagged ones were actually encoded as P_SKIP. I did not add support > for that though but in the future somebody may want to. Yes, it's very x264 specific. But the side data is generic. If for some reason x264 does not process a frame, or any other encoder ends up getting used, the data will leak if it relied on x264 to free it. > In principle I could've put the buffer into the side information and > not just pass a pointer to it but I thought that it would require > adding more semantics which would imply a stronger dependency on > libx264. > Right now, mb_info is a vector of uint8_t flags and the only possible > value - to date - is X264_MBINFO_CONSTANT. What if, say, one day > libx264 decides the buffer has to be a vector of uint16_t or still > uint8_t but the flags are packed? On the other hand, this, AFAIK, is > only supported by libx264. Other codecs may want to choose a different > semantic for a similar field and the only possibility to make it > generic is letting the caller handling the low level details. I'm not aware of any other side data with a similar semantic. And I'm really not sure if it's sane or even valid to do it like that. Can't the data be entirely contained within the side data? _______________________________________________ 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".