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 9E1F4464D4 for ; Mon, 18 Sep 2023 11:59:38 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CA1CF68C776; Mon, 18 Sep 2023 14:59:33 +0300 (EEST) Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01olkn2062.outbound.protection.outlook.com [40.92.64.62]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E855E68C261 for ; Mon, 18 Sep 2023 14:59:25 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BS+LjrUwC0aRd7s1vqpn3Iml+ccKl0DTF2CpdI+Vw5uInHJf/Pbus/2kFejdU7UPPe4RQTqNLKqxwz6gVtx8uOcDcBPcHhGnErnKv6QGbjWZQ7PMAHUlocC7tSZQXOQxzjUl5M3frxf3KMZliyVz+j6FMWRC36ZRmtNXzy8xoGV7fRmrmEv/nZt5ua2Yad3LzrensBMo+hcRWqiPf/YjLdjoFBnwFG3fXPRKzC1tLda7rUKlBpum7y9ux4qvaNRXUJSgNsYfoJW/02yWs3TNEGgY8I2kMid/TwnWEkhpBHudwJdRGhAoAi4oY5XLUghgOZrJmt5izfuzA88qP1zs4Q== 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=/anSIRntVUEtjlLrE+FuyNGuUCRWk+kq7EuhM68156Q=; b=R+dTYn0OD+vlDAS41hEj1ni741ztj8YwxtE3DUUg/3P+5tF3K6WaziGBt/kCQ11v/eal79So7S1YNxZwaqrF5aqYk99aKRNEewGwEMMgd+i0QYhFwVfc3T/kH97mKh6q8FJUBX9HLkoOhRtUBZenQv929OfsTNgRe49emG9vZslsxtPIgPk9xBS8Owcs1f5zyJ3JfznmvM9KziOdyT3xDZwFYA3xY3I9OkhTJ3ZuOJ5wBymdHaYE5UIc305auyqzA5nIS4LrfDprcgkfp8v9O5F3xYsyJEp1+uKgF/EVt8JrVnT7+JCzldq+ujCd+2DX6e6AuhMTTJu1a1BUyP5X7w== 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=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=/anSIRntVUEtjlLrE+FuyNGuUCRWk+kq7EuhM68156Q=; b=MsV4K6QwGa+Q3uVegyorZAmdfDyg80i0oV1+uzWYarkIRy8Kb3GD5br4mDqNqgA0Wyx8yqrKpGSW7I/hV26zxMK5XRpKHP2rdhqlRxnZQMsdpDLhZzDQ8eUnXN7lnAWnuYjjWmUgQQqtdAFbMqzKIX/4kmhTcggp6d2YtaBSEjgHYsSUVKexwx7AonEo/pofHl8xChGgbuXjPHUC7niNguANYyx4fm/kweWusy8wTy7o1u2LFlQorwxST+3spKFsI4xNofwVDQBdtQiJ+zpBJTBPVkpYVPus4knaS8lRgV/H+rqc8a0Eoz18014uU7l4yr3tupnFKSsHTNv6tg1QoQ== Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) by AS8P250MB0086.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:371::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6792.24; Mon, 18 Sep 2023 11:59:23 +0000 Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::5e01:aea5:d3a8:cafa]) by AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::5e01:aea5:d3a8:cafa%3]) with mapi id 15.20.6792.026; Mon, 18 Sep 2023 11:59:23 +0000 Message-ID: Date: Mon, 18 Sep 2023 14:00:37 +0200 To: ffmpeg-devel@ffmpeg.org References: <20230918063728.198377-1-haihao.xiang@intel.com> Content-Language: en-US From: Andreas Rheinhardt In-Reply-To: <20230918063728.198377-1-haihao.xiang@intel.com> X-TMN: [xWIPWil0EJ50diXQ9AUNRuNUZ7wZgNJ7] X-ClientProxiedBy: ZR0P278CA0046.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:1d::15) To AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) X-Microsoft-Original-Message-ID: <95a74e91-4b40-3fb1-7906-633deb51be6b@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P250MB0744:EE_|AS8P250MB0086:EE_ X-MS-Office365-Filtering-Correlation-Id: 11fc603e-2a79-4f56-1175-08dbb83eb304 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: kbcED76hA/V64SMDnjrkOzrzCZi33FwWlmi4VZalo72VeLOC5UwjuSYCH5eqRqdAiZm4CktLKhkKESWW7UNzuYTaBfx0k/meqTFsOz0V5trToRFs8kVdEwrlDMyXpSVvqWo+l9RzLt38MGQoFjTcUUiIlkb0xWWcW3aLT7hKH+OnYfyG4nh5y/NCdd5A2iPR9Zh5Y4qVi4kPmjlXVCixhSfBC0CFckkr0w/bgRnWOX2mRF+ouR5iR0xXeLrD10m3w9NXn14dBrJl85DPaGiFBWQv3J1SRzdBDlfqd7UGpv36KIJ0mUEAaHK0f8a2mE1LnNw+lrh7T1noa9NCSnfPpfaHStjzaWh/RgrfeSIMMmKt/HrSFw7JJadazM62eiLAlqLy8XtFUcs1n8r/TbYBitY4uGKnZ7PAJMhNayFdn9vlzT2E+YybZJc7SFOFUwd/YlVxl3k4/oCLBlBax4gtGgRI/8LJVqWgtNdu6SyAGjlvjgQ3m6CA/xbd+w2FjGcpx53E5vZ470sQRMsjn6RK/dLfPp+MlA4R0kzyhUIp82vgQ7xsPQceQIQJ+j14Ukv5 X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Wk1CdDYrWGo1UEZmNHJMNmFWbC9XSnIyS2VRNTZsZWc4MWlxRTBaNElHWkVU?= =?utf-8?B?WktUTk5KWkZmRjdWWUxEQ2xING0rbE9BQUtnU3dud1dJYW1XVEFiZUJMOXhT?= =?utf-8?B?Tys4eE5Qb1NLbUtEMEd0QmxxdCtzQ1Z3Znh6SmdFR1FQaVB5M1hQZ3JZc1RS?= =?utf-8?B?M3ljbVBnVzliTlUyelUwWUdZaUpqSGNJKzc3N2ZUVFFvdzk3NWRsaTBZUXZx?= =?utf-8?B?c1dxY1FGVitoV09ycTJnM1A0ZUQwaXdnTmZjNjNBTnlJeGxkdXFwS2IvbGRS?= =?utf-8?B?NXlCMkdMR0VvZnkvM01iUkNKanhhUjE2bW51R1hFZG5CRThWT3YvSFVzSnZO?= =?utf-8?B?dE50T2JaODRUWWFUbk5TQkpNSDZnbm8yVzFKcFp4Q1IzbVM3amVFR2lwN1c1?= =?utf-8?B?U2JaWEtsVVViQ0RIM0oxUytudVh4Q2J3WlNFM2FVWkxZQlMyajhWaXNsOTVl?= =?utf-8?B?WDFBd09ONWRET1ZSaXNYcUdXQ2NsSDAyMlNFMU54NUlvTThBZTNpODVRQXd3?= =?utf-8?B?UWFmV1FzS2NycGxVcldHT0UxRHJPYUdDUG9WdW0yOC9zMk5oNXdzSDV3cWxX?= =?utf-8?B?MWx6cWRjdEg0RUk5ZWJsSmJMUmVUR1kxWStYbGRvV0RmQTJERFVqcWFYMWRW?= =?utf-8?B?VjVyMUpMVW85Q1ZFa28rdzJIS2NubEZicWl1T2VUVzY5alhwaG1McUs0dXRF?= =?utf-8?B?aGhIYlFrdWJZZnNhUlNWbVZidkMwdVN4WlRVdW9EMHRhVE9UWFVUT21zb2tw?= =?utf-8?B?QUlyMU9HcUczbGZDeTdXakx5K2dLbVJHSGF6bkZtNFBkYWUyamFHVGZHYkF5?= =?utf-8?B?a2ZIWkNIRmdEL2VsU2VLQ1NOdUxvNUhhME9ZMDZrLzh5MHFiaGw4TER4Yk5k?= =?utf-8?B?b2l5M2cxVHR0eWdMeGlsRWZVOTBheTFJT1ByRmZXRUN2T2RxdmpDTytBdUxZ?= =?utf-8?B?bTBUK0FZeDZ2Y2tDRlFFaUFlb0syNEcrQmx1OWdNc3ROOU13cHp6cG5LN0ZQ?= =?utf-8?B?STJlZTYzU1FIQVJNekk0dFZWV3lUQ1RIREE1UzEwUFl5SEFzZ1p0dGhSeS9m?= =?utf-8?B?cGx4d1hka293OVIzSXlzU1FINitmS2pUZ0pLbFRTZEtyQnpSZGYrc3VvaFQv?= =?utf-8?B?dGVvVUFndUVjMlRRcTVQTkdaNGVLdmhwdHVNcGd2WEl1U24zdmJWRUpQN1Vh?= =?utf-8?B?ZkJzQnFmWlk0cVdpV1JnWDZRWnpBMEd6NkM0YmhlZTNIaTY5TStOZG9iVGZZ?= =?utf-8?B?ZXhFT3E3VEdacU1VVmJZRy9aL3NFcmJ6RDhQOHNHSlZqZytwVGFOV0x3MVd1?= =?utf-8?B?TmRTVmNDS3FvdFEwRVpsT3ZWYjJCT2xUaHpnaGFHZWVCaGZlVWFFMi9CR213?= =?utf-8?B?RjRnSWNNRTBLWkxXMFFKblcvNTNoRElWSWVXMERjeEt0ZUpoRndnRnZKWWR3?= =?utf-8?B?UDRGTGVUMUhSeC9kNmtqU3QwTFpwMHNPMnNQMVBJVVRINjMvbkV1ZklRKzFi?= =?utf-8?B?UVFGdE5NN3RKelVWdlo3YVJ0aGorOXhWQVpDZC91dVI2ZUlnWWF1bmpWN0do?= =?utf-8?B?VW9uRFVmalVhUE55UjVXUnJTaWx4Z0s5ZnZZWEU0NGorVWcxZ0pIUC9sN3B4?= =?utf-8?B?Z0Y0aUVkZHExRGxtK3pNTWxKQkFOSHdiaXE2S1pKNG9Ic0dwa25peEIrNnZR?= =?utf-8?Q?lWzslgeSQonThnn01uog?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 11fc603e-2a79-4f56-1175-08dbb83eb304 X-MS-Exchange-CrossTenant-AuthSource: AS8P250MB0744.EURP250.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Sep 2023 11:59:23.6428 (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: AS8P250MB0086 Subject: Re: [FFmpeg-devel] [PATCH] lavd/sdl2: postpone sdl2 window creation 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: Xiang, Haihao: > From: Haihao Xiang > > Since 2d924b3, sdl2_write_header() and sdl2_write_packet() are called > in two different threads. However SDL2 requires window creation and > rendering should be done in the same thread, otherwise it shows nothing > when specifying SDL2 output device. Modifying a library to fix behaviour in an application (even the FFmpeg tool) is very fishy. This patch makes things worse for people who want their call to avformat_write_header() to actually check whether sdl2 works. > > $ ffmpeg -f lavfi -i yuvtestsrc -pix_fmt yuv420p -f sdl2 "sdl2" > > Signed-off-by: Haihao Xiang > --- > libavdevice/sdl2.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/libavdevice/sdl2.c b/libavdevice/sdl2.c > index 342a253dc0..c9f7f03c28 100644 > --- a/libavdevice/sdl2.c > +++ b/libavdevice/sdl2.c > @@ -158,6 +158,11 @@ static int sdl2_write_trailer(AVFormatContext *s) > } > > static int sdl2_write_header(AVFormatContext *s) > +{ > + return 0; > +} There is no reason to keep an empty write-header function around. Just delete the function pointer in the FFOutputFormat. > + > +static int sdl2_init(AVFormatContext *s) > { > SDLContext *sdl = s->priv_data; > AVStream *st = s->streams[0]; > @@ -165,6 +170,9 @@ static int sdl2_write_header(AVFormatContext *s) > int i, ret = 0; > int flags = 0; > > + if (sdl->inited) I am not an sdl2-expert at all (in fact, your patch made me read their headers for the first time), but it seems to me that you misread the semantics of "inited": It is set if someone else already initialized the library or if we called sdl2_write_header() without entering the fail path. In particular, inited being set does not mean that we called sdl2_write_header() without entering the fail path. The reason I am writing "without entering the fail path" and not "returns an error" is that sdl2_write_header() actually never sets an error on any error path. This means that the sdl2_init() below will always succeed. Given that inited is currently only used for one thing (namely checking whether SDL_Quit() should be called), it would be equivalent to the current code to move the call to SDL_Quit() to the error path in sdl2_write_header() and to make inited a local variable in sdl2_write_header(). The reason for the way inited is handled seems to me that because SDL_Quit() quits everything (and discards reference counters etc.), it may only be called if no one else uses SDL2, hence not calling SDL_Quit() if it was already initialized or if someone else may have initialized it after we have have returned via the non-error path in sdl2_write_header(). I think that this is wrong even if all interactions with SDL2 happen only in one thread: The check for whether SDL2 was already initialized only checks for whether the video subsystem was initialized. But other subsystems might have already been initialized and these would also be killed by SDL_Quit(). Therefore I think that we should never call SDL_Quit(). Instead we should call SDL_InitSubSystem() and call SDL_QuitSubSystem() iff SDL_InitSubSystem() succeeded. This is refcounted. And then write_trailer should be made a deinit function. I just made a quick test and this reduces the still reachable amount of memory at the end (as reported by Valgrind) considerably (from 5,314,497 to 312,104). Sorry for this rant which does not affect your threading issue at all. > + return 0; > + > if (!sdl->window_title) > sdl->window_title = av_strdup(s->url); > > @@ -249,6 +257,11 @@ static int sdl2_write_packet(AVFormatContext *s, AVPacket *pkt) > int linesize[4]; > > SDL_Event event; > + > + ret = sdl2_init(s); > + if (ret) > + return ret; > + > if (SDL_PollEvent(&event)){ > switch (event.type) { > case SDL_KEYDOWN: _______________________________________________ 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".