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 8EC2F42D28 for ; Tue, 3 May 2022 21:04:44 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9955668B26C; Wed, 4 May 2022 00:04:41 +0300 (EEST) Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10olkn2031.outbound.protection.outlook.com [40.92.41.31]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3F0F868008A for ; Wed, 4 May 2022 00:04:34 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IlSYjAlVztQM6276fdEgOENW86fyHNJtSfZqSALRa+tKM33KuYhrhf0e46GpY+lRhBHLrcVwwMz7PF69NMk5QIRiH+8fK5r3BvtZ00ZrgdVq1dOb+4ASEt7TbFQUuJJA3DNFljFVgObIcmMjOQJlyhhTch6+GMsaefc8x97HeXFUQJMQEeqVhoarfYK2qcVZkx04wnFg0sdr6dCb86+ywCREbUMgxegqtCGR2uS8Gye+yRIjdladoIKzK2CJnj6tvMbUz7BPUIfdOsqAN+zSYr5TvBs8HN0x1u/KT+w5ZAzFilzDKnIF27IE42dlfCOdC66EsE8K9jjMEI2VBkqOmQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=GEyekGvNjacJ7Y+6nutVEd2nY8a0LEv5MVbPrQ5SqgQ=; b=DoBdK1o10WFPnlZPv5aXGdOlPjF2yGv6DV5Oa8hFGhBOyKw5ctLHIpr/r+7P8YyT81C7wiG8LeOsoZm0ODkVboXWQ8CO5iulk2hi+OeFY94HA48g3RAow2of+dEZyxuDomJepzooA2UR8YUNH0YOVMJKF+GIoEqvbDw6f327byxIpO/4tDwluSc7hSO8wKkE3ZajmEYPFLYVdzRJyiGZ6OS45rU+PFzbk2J8qTPtwbZm5bMB/t2kg4rUU/J9Q7i47KG46EFXhGmmW4tUc+0EiGBTJdVr2mgzPxRcYOVnsJdxOlyCyev8zV8NAjg+4bgyqFcUHQnAkHEA/YFp13Lwow== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GEyekGvNjacJ7Y+6nutVEd2nY8a0LEv5MVbPrQ5SqgQ=; b=TAOcls7ejEEAIS+UMVN+FJnUlHhG3OJX2MToF+rZbSw+qWi2gXRDQ4umGu6ctc0lSl5VOykfwXAYZ7tM/Vm4gnyvi5Iv55DpG6H9gPDQ4uIEU5H4z6MUuozdlaTKJQNRK3Zd0yP/s8DlzIGP4YPzr0TgND/kpB++1QCX8rhjcTiKcdxyrBHCMUuzD3BoKnatFhrJc1dDJnxu/drrmUcWGzI45HX5OGhKPVv3lSG0/fvwbGXO/G4GREQahAex+Ju8DLsXNIY3K0AJHjDwDOLNXNxz2jn4SlNIgVsUmdeDL7He+jp+aocpEPK0keym3YtcsBeDkbsojUnfCoaviarG3A== Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by IA1P223MB0401.NAMP223.PROD.OUTLOOK.COM (2603:10b6:208:3d7::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5186.14; Tue, 3 May 2022 21:04:31 +0000 Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::7472:6f83:eeb:45e3]) by DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::7472:6f83:eeb:45e3%9]) with mapi id 15.20.5206.013; Tue, 3 May 2022 21:04:29 +0000 From: Soft Works To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH 1/3] avutils/hwcontext: add derive-device function which searches for existing devices in both directions Thread-Index: AQHYXM31okUFSHUnd0mZd8hUGLzrfq0I+66AgAABwGCAAZbXgIAAlKpggAEArACAAAVXcIAAGBsAgAAAnpCAAVXggIAACrsw Date: Tue, 3 May 2022 21:04:29 +0000 Message-ID: References: <85ff784b6ff80d4f2e0f724d0b93472e50d2c256.1651349262.git.ffmpegagent@gmail.com> <994ee7a9-ab91-4d13-9538-ed4502c73bd6@jkqxz.net> <33e93072-cde1-437b-bf60-e78250f26692@jkqxz.net> <99c7fc4a-dcc3-91f7-615e-36218564a65b@jkqxz.net> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-tmn: [yxqU9ERbK3nhwNmQdqU+L0WDXbwIGtXV] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: ea4a7d49-d031-429a-ad33-08da2d4883b5 x-ms-traffictypediagnostic: IA1P223MB0401:EE_ x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: e+FHRvU+YZq+xkYy/weNkvUD5fD04KY5/RcHQEFaJkIlB5nUdfH8NJzOtvMiAQSY600X+uX4Kc5FJPdjBnGxYufqfLAPNekscwd1EP92eI/N9gB6OFfOp7zKkPz8Yj+7Xi84HZJQWKnWkSGFKnaUOApyQ5e3u9c308lRCL0VnnlEMHwkzDDgY7zIwpIDIGOhicmb3iomBRp1zX5FH9vbzOurfE5SNQna2xcSLTOXSoBEpzmVS4cmHzzr3DnPvOJY/TiNuy4tzkgFmup+tSQWEgmAX/P40/l+IYBjVIIGqPmY4LdtnJOHtMbECWXQCucQfJRo7Ly9VbDrWkr4DdFbc6cxai0/TK2T126YCv4gqtC4rvs/2msTuB0/WDyLgmTRTCCwd4QINuMli+LYQkxTMpKbZsm/IKesIqfaX/g0XCqM97zvfvG/fFGPWquoBx4EI4Kpb23I6lMtub5UWMsZxDauGGQbX+KidnIQYZuGkcctY+EnnJYfND6s7/E0z90j+EKQq/cMdR663/59Ec6hWCFveG1GrmEFIG0u4355VqYL3ym5nfms5iwW/ShYiEwdrcU4r7VIEaPilJNsagnH30DVixyOE753J2o78966OPNgUcwa2wUCr2gQV4EbufCsEHNqpL5DVvq99Zq9u427vFiiRowxfC0CCupZ2QOQRL+3OYaDZdT+JGcCMasSNBeZ x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?ZWxRTm9mZkRPL3k0SHpBbFNqVVB1MHlic0dyM1RYNmJrQWp0VFNDR04rVGZw?= =?utf-8?B?S0haZ0V2KzNZMkxGdzR0NndLYzNLbmpyNGdhT1JHOFM0Nk1GSWJaWUZkay9G?= =?utf-8?B?MXhFNkV5WE9TNkVJRkUzcXd6UEpHdWRrdWxMODNsUENKdzlqWGR1THVLeUFy?= =?utf-8?B?Q2VENDd4azlKU0FJcWloRHhzaXRhVFk0SmJDbmk5SmlTblhRTytUR2ZoeldN?= =?utf-8?B?TjRhaDJSM1E0RHc4dXowNE1SdG5sZWNHWlc0VW56T0ZId040dVp1RDBZYS9G?= =?utf-8?B?Z0dYZU5ZcjkvbHZMSjBnUzNLU1Y0TlhNVHJxZVNVVDZBNFpHTURLaUg5SEZX?= =?utf-8?B?UnIvODNSNzJTcVRFNURWUjJmeFBDMHAzQVk0RlY5YnlxRjNhUnJUWGRobm10?= =?utf-8?B?QUpnL25XRkVEN2VVa2FYa0hac01IakpjTmlMbWxXWXlRN1Bubk1Wd3Z3bnU1?= =?utf-8?B?K2ZVTGFTa1J4dUtkd2xuWS8xcU9wdjVHaXc0QzNWM2NjbGtWSllDZHUzUzFt?= =?utf-8?B?TThSMnFzNm5KNElCZnBUM2k3ZVBHeTQ3NTU4VnRRZUJ4Q29JOXBCbjhuclYv?= =?utf-8?B?L0hVQWFsK0lROS9VMEUvRUN4eHdmWE9qK25PbXhyNjU0QUROakk5RTBqb1B3?= =?utf-8?B?akt4ZDFBV3paM1R3QzVhQWk0OFVEQjRiOFlDM0VzL2NvN0thdjM2TWJKdlpO?= =?utf-8?B?Y0xobEtNV0ovR1FOeWNMa3pPZHo4cHRzWHl0elV0Zk9WVnNNYnlBM3UxNzBR?= =?utf-8?B?Kzh6bzdqQi9FZHpmUmlHTUk2WWJZaWtDbEgxMmplMWxwckU3aHhzbUlvbGVI?= =?utf-8?B?QWxEdUhnVktHQVJ6SWdXU1NEQm52d1ZJdjI0bEVRM3FGeGFuTUliU2xweUpF?= =?utf-8?B?Wjd4OEV3dEV2U0lRL21SaXB4VUVCa2pCc0JCRTh5dHl3YUw5WGp2bUVvMjhD?= =?utf-8?B?ejlXQmFVczQvdEFnek5ORGpVY0lubjl4R21IRWRjM2pKSnN4ZE9TMWtDQU5T?= =?utf-8?B?eElXclNCeWlCc0ZnbHJ1ZC9GMUJZU3lHcXY4SmdtVURqYndWMmhTMWpzdXBX?= =?utf-8?B?V2wrUXhNcDd6SVQyczZzTTQ2MEpYMUNXVCtJRkIrdCt2N1FKTGZyNWlhUUw3?= =?utf-8?B?L1JSL1BjYkgvaFNKdnBQaUNTWWhVQ0dZenNneE9vbFZkVGdWRjIrOEhaWmNC?= =?utf-8?B?NUFBNW9TdlpWZ0NUQm1acCt5MnkwL3MvRHBHbXhWOFUxM2V1WFEwbmtaSU14?= =?utf-8?B?QXNSTlpvOTB2RTM0ZUIzMDE0cVRkSU1wc2pGV2RXWmtVK0ZwWG1DZ0dCQTZV?= =?utf-8?B?Zkh1MFZWUXpEVFpWN1hmVmtUQXJPanVic01MbmJMVnp6Q0d3YXFrNnBUN1M3?= =?utf-8?B?YytjcElOVkFFYmQwZDVaT0hlcG1JcG1FU2V6VmRUNWFOWUxldzZRQWNBSkh3?= =?utf-8?B?M0o4clcrTTBNMmVIOGg3R3EyVjVkYXFSV2hlbWJSU3pLNFF2QXRTOXNHVkhI?= =?utf-8?B?RHlwR0d1UzcyRGZnU3dIVkRCVWg1UTBYT296UXROQU5oa05mNkoxS0JLY1l5?= =?utf-8?B?NTdaeUZlTmVtUEFBQ0ZtNEx1enpFUjE5ckJGWjZ5bk1JN3FFWVREengxVHJP?= =?utf-8?B?YkRpUllyUnhhaEFXZEpCU05DaVFlSEVOejJMQ3NNcXIxZDdhR3VhQzFxOVNL?= =?utf-8?B?ejBGSUt6MktEWm1BeVl0STc5bU4zU09QOUhUb3JFcThVS2h6S2pPa2xhNEdl?= =?utf-8?B?eHV5L3NOUzI2RHN0TWd5WFI2d2lMczUyemlhUVlxcGwyNnRXQjQ3WnlqOFR5?= =?utf-8?B?VTF5MHJFRTF4WGNEbUYwRFFEMjN0NVAxZnU3NjVISzAxc3M5M2pFRlpTd2ZY?= =?utf-8?B?TGY2T1hlTmhJMWpZQ1RsUTd4bGZoeHFsem5LYjFzbTlPTzcyMm4xaTl6K2hG?= =?utf-8?Q?V65BRvaBfkw=3D?= MIME-Version: 1.0 X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-1ff67.templateTenant X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: ea4a7d49-d031-429a-ad33-08da2d4883b5 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 May 2022 21:04:29.6650 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1P223MB0401 Subject: Re: [FFmpeg-devel] [PATCH 1/3] avutils/hwcontext: add derive-device function which searches for existing devices in both directions 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > -----Original Message----- > From: ffmpeg-devel On Behalf Of Mark > Thompson > Sent: Tuesday, May 3, 2022 10:23 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 1/3] avutils/hwcontext: add derive- > device function which searches for existing devices in both directions > > On 03/05/2022 01:11, Soft Works wrote: > >> -----Original Message----- > >> From: ffmpeg-devel On Behalf Of > Mark > >> Thompson > >> Sent: Tuesday, May 3, 2022 1:57 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH 1/3] avutils/hwcontext: add > derive- > >> device function which searches for existing devices in both > directions > >> > >> On 02/05/2022 23:59, Soft Works wrote: > >>>> -----Original Message----- > >>>> From: ffmpeg-devel On Behalf Of > >> Mark > >>>> Thompson > >>>> Sent: Tuesday, May 3, 2022 12:12 AM > >>>> To: ffmpeg-devel@ffmpeg.org > >>>> Subject: Re: [FFmpeg-devel] [PATCH 1/3] avutils/hwcontext: add > >> derive- > >>>> device function which searches for existing devices in both > >> directions > >>>> > >>>> On 02/05/2022 09:14, Soft Works wrote: > >>>>>> -----Original Message----- > >>>>>> From: ffmpeg-devel On Behalf > Of > >>>> Mark > >>>>>> Thompson > >>>>>> Sent: Monday, May 2, 2022 12:01 AM > >>>>>> To: ffmpeg-devel@ffmpeg.org > >>>>>> Subject: Re: [FFmpeg-devel] [PATCH 1/3] avutils/hwcontext: add > >>>> derive- > >>>>>> device function which searches for existing devices in both > >>>> directions > >>>>> > >>>>> [..] > >>>>> > >>>>>>>> * The thread-safety properties of the hwcontext API have been > >>>> lost > >>>>>> - > >>>>>>>> you can no longer operate on devices independently across > >> threads > >>>>>>>> (insofar as the underlying API allows that). > >>>>>>>> Maybe there is an argument that derivation is > something > >>>> which > >>>>>>>> should happen early on and therefore documenting it as > thread- > >>>>>> unsafe > >>>>>>>> is ok, but when hwupload/hwmap can use it inside filtergraphs > >>>> that > >>>>>>>> just isn't going to happen (and will be violated in the > FFmpeg > >>>>>> utility > >>>>>>>> if filters get threaded, as is being worked on). > >>>>>>> > >>>>>>> From my understanding there will be a single separate > thread > >>>> which > >>>>>>> handles all filtergraph operations. > >>>>>>> I don't think it would even be possible (without massive > >> changes) > >>>>>>> to arbitrate filter processing in parallel. > >>>>>>> But even if this would be implemented: the filtergraph setup > >>>> (init, > >>>>>>> uninit, query_formats, etc.) would surely happen on a single > >>>> thread. > >>>>>> > >>>>>> The ffmpeg utility creates filtergraphs dynamically when the > >> first > >>>>>> frame is available from their inputs, so I don't see why you > >>>> wouldn't > >>>>>> allow multiple of them to be created in parallel in that case. > >>>>>> > >>>>>> If you create all devices at the beginning and then give > >> references > >>>> to > >>>>>> them to the various filters which need them (so never > manipulate > >>>>>> devices dynamically within the graph) then it would be ok, but > I > >>>> think > >>>>>> you've already rejected that approach. > >>>>> > >>>>> Please let's not break out of the scope of this patchset. > >>>>> This patchset is not about re-doing device derivation. The only > >>>>> small change that it does is that it returns an existing device > >>>>> instead of creating a new one when such device already exists > >>>>> in the same(!) chain. > >>>>> > >>>>> The change it makes has really nothing to do with threading or > >>>>> anything around it. > >>>> > >>>> The change makes existing thread-safe hwcontext APIs unsafe. > That > >> is > >>>> definitely not "nothing to do with threading", and has to be > >> resolved > >>>> since they can currently be called from contexts which expect > >> thread- > >>>> safety (such as making filtergraphs). > >>> > >>> As I said, I'm not aware that filtergraph creation would be a > >> context > >>> which requires thread-safety, even when considering frames which > get > >>> pushed into a filtergraph (like from a decoder). > >> > >> User programs can do whatever they like within the API constraints. > >> > >>> Could you please show a command line which causes a situation > where > >>> device_derive is being called from multiple threads? > >> > >> Given that the ffmpeg utility is currently entirely single- > threaded, > >> this will only happen once the intended plans for adding threading > to > >> it are complete. > > > > As mentioned, I don't think that would be possible easily, and from > > what I have understood, the plan is to have separate threads for > decoding, > > encoding and filtering but not multiple threads for filtering. > > As the summary in devel/2022-April/294940.html> states, the intent is a separate thread > for each instance of those things, including filtergraphs. > > >> With that, it will be true for something which makes two > filtergraphs > >> and uses derivation in both of them. For example:, this currently > >> makes two independent VAAPI devices, but equally could reuse the > same > >> device: > >> > >> # ffmpeg -y -f kmsgrab -i /dev/dri/card0 -vf > >> hwmap=derive_device=vaapi,scale_vaapi=format=nv12 -c:v h264_vaapi > >> out1.mp4 -vf > >> hwmap=derive_device=vaapi,scale_vaapi=w=1280:h=720:format=nv12 -c:v > >> h264_vaapi out2.mp4 > > > > Well, multi-threading is not an issue right now, and I don't expect > it > > to be then. > > > > But on a more practical take: what do you want me to do? > > > > Guard that function with a lock? I can do this, no problem. > > I'd like it to be designed in a way that avoids this sort of problem, > as all the existing functions are. > > > But > > none of any of the device control function does any synchronization, > > so why would exactly this - and only this function need > synchronization? > > The derived_devices field is modified post-creation, which gives you > data races if there is no other synchronisation. > > (Consider simultaneous derivation calls: currently that's completely > fine - it makes no changes to the parent device and the derived > devices are independent. With your proposed patch there is undefined > behaviour because of the simultaneous reading and writing of the > derived_devices field.) > > The only other mutable field is the buffer pool in a frames context, > which has its own internal synchronisation. > > Having thought about this a bit further, I suspect you are going to > need an additional shared-devices object of some kind in order to get > around the circular references and fix the memory leak problem. > > That additional object will have to be mutable and accessible from > multiple threads, so will probably need an internal lock (or careful > lock-free design). Yup, that sounds quite reasonable to me. Very helpful comment, thanks! _______________________________________________ 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".