[{"id":28126,"web_url":"https://patchwork.libcamera.org/comment/28126/","msgid":"<a2d7c004997ba01661c21bdf3c29f93790aa586b.camel@ndufresne.ca>","date":"2023-11-22T15:25:30","subject":"Re: [libcamera-devel] [PATCH 2/2] gstreamer: Implement renegotiation","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le mercredi 22 novembre 2023 à 14:43 +0100, Jaslo Ziska via libcamera-devel a\nécrit :\n> This commit implements renegotiation of the camera configuration and\n> source pad caps. A renegotiation can happen when a downstream element\n> decides to change caps or the pipeline is dynamically changed.\n> \n> To handle the renegotiation the GST_FLOW_NOT_NEGOTIATED return value has\n> to be handled in GstLibcameraSrcState::processRequest. Otherwise the\n> default would be to print an error and stop streaming.\n> To archive this in a clean way the if-statement is altered into a switch\n> statement which now also has a case for GST_FLOW_NOT_NEGOTIATED. Just\n> like the case for GST_FLOW_OK, the function will return without an error\n> because the renegotiation will happen later in the running task.\n\nThat's an interesting comment, I don't think we expect GST_FLOW_NOT_NEGOTIATED\nin the renegotiation process. Downstream should keep handling the current\nformat, emit an upstream reconfigure event and wait (may drop) but returning\nthat flow is not really expected. How did you get that ?\n\n> \n> In gst_libcamera_src_task_run all the source pads are checked for the\n> reconfigure flag by calling gst_pad_check_reconfigure. It is important\n> to iterate all source pads and not break after one pad requested a\n> reconfiguration because gst_pad_check_reconfigure clears the reconfigure\n> flag and if two pads request a reconfiguration at the same time the\n> renegotiation would happen twice.\n\nnit: We may want to break on first, and then just systematically clear all the\nreconfigure flags in the negotiation() function. This would then become handy if\nwe need to renegotiation due to some \"signal\" behind sent by libcamera itself.\n\n> If a source pad requested a reconfiguration it is first checked whether\n> the old caps are still sufficient. If they are not the renegotiation\n> will happen.\n> If any pad requested a reconfiguration the following will happen:\n> 1. The camera is stopped because changing the configuration may not\n>    happen while running.\n> 2. The completedRequests queue will be cleared because the completed\n>    buffers have the wrong configuration (see below).\n> 3. The new caps are negotiated by calling gst_libcamera_src_negotiate.\n>    When the negotiation fails streaming will stop.\n> 4. The camera is started again.\n> \n> Clearing the completedRequests queue is archived with a new method in\n> GstLibcameraSrcState called clearRequests. This function is now also\n> used in gst_libcamera_src_task_leave as the code there did the same\n> thing.\n\nMight want to slim down the patch by doing this refactoring in its own patch ?\n\n> \n> In gst_libcamera_src_task_enter, after the initial negotiation, a call\n> to gst_pad_check_reconfigure was added to clear the reconfigure flag to\n> avoid triggering a renegotiation when running the task for the first\n> time.\n> \n> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n> ---\n>  src/gstreamer/gstlibcamerasrc.cpp | 72 +++++++++++++++++++++++--------\n>  1 file changed, 55 insertions(+), 17 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index e7a49fef..868fa20a 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -11,7 +11,6 @@\n>   *  - Implement GstElement::send_event\n>   *    + Allowing application to use FLUSH/FLUSH_STOP\n>   *    + Prevent the main thread from accessing streaming thread\n> - *  - Implement renegotiation (even if slow)\n>   *  - Implement GstElement::request-new-pad (multi stream)\n>   *    + Evaluate if a single streaming thread is fine\n>   *  - Add application driven request (snapshot)\n> @@ -133,6 +132,7 @@ struct GstLibcameraSrcState {\n>  \tint queueRequest();\n>  \tvoid requestCompleted(Request *request);\n>  \tint processRequest();\n> +\tvoid clearRequests();\n>  };\n>  \n>  struct _GstLibcameraSrc {\n> @@ -301,23 +301,39 @@ int GstLibcameraSrcState::processRequest()\n>  \t\t\t\t\t\t\tsrcpad, ret);\n>  \t}\n>  \n> -\tif (ret != GST_FLOW_OK) {\n> -\t\tif (ret == GST_FLOW_EOS) {\n> -\t\t\tg_autoptr(GstEvent) eos = gst_event_new_eos();\n> -\t\t\tguint32 seqnum = gst_util_seqnum_next();\n> -\t\t\tgst_event_set_seqnum(eos, seqnum);\n> -\t\t\tfor (GstPad *srcpad : srcpads_)\n> -\t\t\t\tgst_pad_push_event(srcpad, gst_event_ref(eos));\n> -\t\t} else if (ret != GST_FLOW_FLUSHING) {\n> -\t\t\tGST_ELEMENT_FLOW_ERROR(src_, ret);\n> -\t\t}\n> +\tswitch (ret) {\n> +\tcase GST_FLOW_OK:\n> +\tcase GST_FLOW_NOT_NEGOTIATED:\n> +\t\tbreak;\n> +\tcase GST_FLOW_EOS: {\n> +\t\tg_autoptr(GstEvent) eos = gst_event_new_eos();\n> +\t\tguint32 seqnum = gst_util_seqnum_next();\n> +\t\tgst_event_set_seqnum(eos, seqnum);\n> +\t\tfor (GstPad *srcpad : srcpads_)\n> +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(eos));\n> +\n> +\t\terr = -EPIPE;\n> +\t\tbreak;\n> +\t}\n> +\tcase GST_FLOW_FLUSHING:\n> +\t\terr = -EPIPE;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tGST_ELEMENT_FLOW_ERROR(src_, ret);\n>  \n> -\t\treturn -EPIPE;\n> +\t\terr = -EPIPE;\n> +\t\tbreak;\n>  \t}\n>  \n>  \treturn err;\n>  }\n>  \n> +void GstLibcameraSrcState::clearRequests()\n> +{\n> +\tGLibLocker locker(&lock_);\n> +\tcompletedRequests_ = {};\n> +}\n> +\n>  static bool\n>  gst_libcamera_src_open(GstLibcameraSrc *self)\n>  {\n> @@ -488,6 +504,31 @@ gst_libcamera_src_task_run(gpointer user_data)\n>  \t\treturn;\n>  \t}\n>  \n> +\t// check if a srcpad requested a renegotiation\n> +\tgboolean reconfigure = FALSE;\n> +\tfor (GstPad *srcpad : state->srcpads_) {\n> +\t\tif (gst_pad_check_reconfigure(srcpad)) {\n> +\t\t\t// check whether the caps even need changing\n> +\t\t\tg_autoptr(GstCaps) caps = gst_pad_get_current_caps(srcpad);\n> +\t\t\tg_autoptr(GstCaps) intersection = gst_pad_peer_query_caps(srcpad, caps);\n\nJust wondering, but maybe gst_pad_peer_query_accept_caps() would do the job,\nwith a slightly improved performance ? This is only possible since\ngst_pad_get_current_caps() is fixed. At this point, it should cannot be null\neither.\n\n> +\n> +\t\t\tif (gst_caps_is_empty(intersection))\n> +\t\t\t\treconfigure = TRUE;\n> +\t\t}\n> +\t}\n> +\n> +\tif (reconfigure) {\n> +\t\tstate->cam_->stop();\n> +\t\tstate->clearRequests();\n> +\n> +\t\tif (!gst_libcamera_src_negotiate(self)) {\n> +\t\t\tGST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);\n> +\t\t\tgst_task_stop(self->task);\n> +\t\t}\n> +\n> +\t\tstate->cam_->start(&state->initControls_);\n> +\t}\n> +\n>  \t/*\n>  \t * Create and queue one request. If no buffers are available the\n>  \t * function returns -ENOBUFS, which we ignore here as that's not a\n> @@ -594,6 +635,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>  \t\tgst_segment_init(&segment, GST_FORMAT_TIME);\n>  \t\tgst_pad_push_event(srcpad, gst_event_new_segment(&segment));\n>  \n> +\t\tgst_pad_check_reconfigure(srcpad); // clear reconfigure flag\n>  \t}\n>  \n>  \tif (self->auto_focus_mode != controls::AfModeManual) {\n> @@ -629,11 +671,7 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,\n>  \tGST_DEBUG_OBJECT(self, \"Streaming thread is about to stop\");\n>  \n>  \tstate->cam_->stop();\n> -\n> -\t{\n> -\t\tGLibLocker locker(&state->lock_);\n> -\t\tstate->completedRequests_ = {};\n> -\t}\n> +\tstate->clearRequests();\n>  \n>  \t{\n>  \t\tGLibRecLocker locker(&self->stream_lock);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3B747BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 22 Nov 2023 15:25:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECB59629BC;\n\tWed, 22 Nov 2023 16:25:32 +0100 (CET)","from mail-qk1-x72a.google.com (mail-qk1-x72a.google.com\n\t[IPv6:2607:f8b0:4864:20::72a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2185C61DAD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Nov 2023 16:25:32 +0100 (CET)","by mail-qk1-x72a.google.com with SMTP id\n\taf79cd13be357-77d645c0e06so50694185a.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Nov 2023 07:25:32 -0800 (PST)","from nicolas-tpx395.localdomain ([2606:6d00:10:3ac8::7a9])\n\tby smtp.gmail.com with ESMTPSA id\n\tbk4-20020a05620a1a0400b007776c520488sm4495800qkb.9.2023.11.22.07.25.30\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 22 Nov 2023 07:25:30 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700666733;\n\tbh=ViUgRxm0QjLnfFgO8Sx/us1aRFablTdHvq6WLBR2a2M=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=p69reHzcMSekUIZauiDAhKjYM7xcniZX5G9Cv+OjKdH1/DwhRH3K7WJVxcNr/+K0E\n\tu8p6w6Qpw+bRK2aLzMt1MeiNicZSSHUsntwjFGD1VSKAwuxBbvIp4UPmyhBCPrH51T\n\tcuSKD1Evytbj06Sn2nwbjdw63/Vemt23Yd5rGBQopj7dqNzl7YCLRKfq20WenazTL5\n\t4i2rSnlQ4O9HXQs4j4/NSffCpDv5jaPPrvDUBhb+8YG+3LAlIvQ0zFB5cUsfZLi6Vf\n\tZ7H/ZRLVMVsHu1qG5h6yXvJShjWM7hC9LXQMVRJszOomFSTvtA9mDdUqPTiwT1B6o/\n\tSlJRNIkZxlyfQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1700666731;\n\tx=1701271531; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=uRBV9CHGC6j1hPBFPrE7S/C/L4R8DMm5jpSgT+4sd+w=;\n\tb=dv+FiL6uM+Kdpua1yN5i32JPmD8vGVL0EYcaeICjmTNv7dwBUjyuHBpC0+LuT3FjR2\n\tiqtljtlhrwyGk4NdAvJMNiREND4tST0Hh2q/uJ7Ss3ulpxd+aw/IC67ZnQP2HC/zK13q\n\tYWebQdFh1rxq+gZfScY7Exiw1jn/Z/RLvfosHqJTdvlDPB5dnXlH33gS/ceUwqEyQySo\n\tZRrPRVGwzOtmqM9beZwK2ca9ZOb1fv57McqvDHgTzUXv1KbtxQBdydlzl4e66Cu+MT35\n\txk6HvF7YrURxG+hh6HR6V+Lvy2vyYOcy+TUbutqkkdtwZyn9AmXQ1vRJInLkGGxwJ/0l\n\tVRMw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com header.b=\"dv+FiL6u\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700666731; x=1701271531;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=uRBV9CHGC6j1hPBFPrE7S/C/L4R8DMm5jpSgT+4sd+w=;\n\tb=sBatxL0fKm0IQFQj0mLHdGZuYGc9mXHQh2LfaKmzvnTdf6DujnUh75l4M1hvbi5Tlg\n\tawD+HWO2dnOODFNvBlY6CTquE2g0FQhVKCpeozPcD+2Kc0CtjONW1r0kyiqL5jCsG+Ae\n\tfChJPDHHN4/eWRIUNddh270MhkfN6R1YfRoKMmYenCeuHk4k/lDUKRrGYj8gIwCzmfci\n\tiPf0xoSED/R+5wEoOJ5uZjBsLmoZ6SioNKzbGsUd9NLwfbIg6J1p4LgO8rBNt/3OHz5y\n\tNWDsM/nepCBue+XvwWc1JQocHJWP4gutE/XQA1kuf8vzVz/ZvZXDx3ylzKscbXmYf416\n\t708Q==","X-Gm-Message-State":"AOJu0YxCBmGKYPXpjymBCMoyvxRmd749zEVoXI3BpvvCqT/5gULIQOv8\n\twZIf7p/bTBi9+Pv07zB97wCux5rh9RhIUGBoEy8=","X-Google-Smtp-Source":"AGHT+IGJoszmTRRbQyhMnCvdx5pGFIRL6MxWnZ0OC/uOatfiMJ8/wM4BRpwQszp+FRGVtwLARIdurg==","X-Received":"by 2002:a05:620a:2491:b0:77d:65d3:51aa with SMTP id\n\ti17-20020a05620a249100b0077d65d351aamr2490279qkn.33.1700666731020; \n\tWed, 22 Nov 2023 07:25:31 -0800 (PST)","Message-ID":"<a2d7c004997ba01661c21bdf3c29f93790aa586b.camel@ndufresne.ca>","To":"Jaslo Ziska <jaslo@ziska.de>, libcamera-devel@lists.libcamera.org","Date":"Wed, 22 Nov 2023 10:25:30 -0500","In-Reply-To":"<20231122135406.14921-3-jaslo@ziska.de>","References":"<20231122135406.14921-1-jaslo@ziska.de>\n\t<20231122135406.14921-3-jaslo@ziska.de>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.4 (3.48.4-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH 2/2] gstreamer: Implement renegotiation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Nicolas Dufresne via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28151,"web_url":"https://patchwork.libcamera.org/comment/28151/","msgid":"<87v89slna4.fsf@ziska.de>","date":"2023-11-23T11:00:15","subject":"Re: [libcamera-devel] [PATCH 2/2] gstreamer: Implement renegotiation","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi Nicolas,\n\nthanks for the reviews!\n\nNicolas Dufresne <nicolas@ndufresne.ca> writes:\n> Le mercredi 22 novembre 2023 à 14:43 +0100, Jaslo Ziska via \n> libcamera-devel a\n> écrit :\n>> This commit implements renegotiation of the camera \n>> configuration and\n>> source pad caps. A renegotiation can happen when a downstream \n>> element\n>> decides to change caps or the pipeline is dynamically changed.\n>>\n>> To handle the renegotiation the GST_FLOW_NOT_NEGOTIATED return \n>> value has\n>> to be handled in GstLibcameraSrcState::processRequest. \n>> Otherwise the\n>> default would be to print an error and stop streaming.\n>> To archive this in a clean way the if-statement is altered into \n>> a switch\n>> statement which now also has a case for \n>> GST_FLOW_NOT_NEGOTIATED. Just\n>> like the case for GST_FLOW_OK, the function will return without \n>> an error\n>> because the renegotiation will happen later in the running \n>> task.\n>\n> That's an interesting comment, I don't think we expect \n> GST_FLOW_NOT_NEGOTIATED\n> in the renegotiation process. Downstream should keep handling \n> the current\n> format, emit an upstream reconfigure event and wait (may drop) \n> but returning\n> that flow is not really expected. How did you get that ?\n\nYou are right, the downstream element does send the \nGST_EVENT_RECONFIGURE event. And I think we could handle this just \nlike with the EOS event by setting an atomic and handling it in \nthe running task. The way that I solved it now is similar to how \nGstBaseSrc does it, they also check for reconfigure with \ngst_pad_check_reconfigure.\n\nAs to why we receive GST_FLOW_NOT_NEGOTIATED: I'm not really sure \nwhat is supposed to happen in this situation as the documentation \nis a little scarce around this. I test this by dynamically \nswitching capsfilter elements with different caps so I think it \nmakes sense that the new capsfilter does not accept the buffers \nand instead returns GST_FLOW_NOT_NEGOTIATED. Maybe other elements \ncan keep processing the buffers with the old caps and only send \nthe event?\nBut GstBaseSrc also has a case where they check for \nGST_FLOW_NOT_NEGOTIATED (although they then also check if it \nreally is a reconfiguration with gst_pad_needs_reconfigure, maybe \nI should add that) so it might be that this is supposed to happen.\n\n>>\n>> In gst_libcamera_src_task_run all the source pads are checked \n>> for the\n>> reconfigure flag by calling gst_pad_check_reconfigure. It is \n>> important\n>> to iterate all source pads and not break after one pad \n>> requested a\n>> reconfiguration because gst_pad_check_reconfigure clears the \n>> reconfigure\n>> flag and if two pads request a reconfiguration at the same time \n>> the\n>> renegotiation would happen twice.\n>\n> nit: We may want to break on first, and then just systematically \n> clear all the\n> reconfigure flags in the negotiation() function. This would then \n> become handy if\n> we need to renegotiation due to some \"signal\" behind sent by \n> libcamera itself.\n\nThat's a good idea, I will add that.\n\n>> If a source pad requested a reconfiguration it is first checked \n>> whether\n>> the old caps are still sufficient. If they are not the \n>> renegotiation\n>> will happen.\n>> If any pad requested a reconfiguration the following will \n>> happen:\n>> 1. The camera is stopped because changing the configuration may \n>> not\n>>    happen while running.\n>> 2. The completedRequests queue will be cleared because the \n>> completed\n>>    buffers have the wrong configuration (see below).\n>> 3. The new caps are negotiated by calling \n>> gst_libcamera_src_negotiate.\n>>    When the negotiation fails streaming will stop.\n>> 4. The camera is started again.\n>>\n>> Clearing the completedRequests queue is archived with a new \n>> method in\n>> GstLibcameraSrcState called clearRequests. This function is now \n>> also\n>> used in gst_libcamera_src_task_leave as the code there did the \n>> same\n>> thing.\n>\n> Might want to slim down the patch by doing this refactoring in \n> its own patch ?\n\nOk, I will do that.\n\n>>\n>> In gst_libcamera_src_task_enter, after the initial negotiation, \n>> a call\n>> to gst_pad_check_reconfigure was added to clear the reconfigure \n>> flag to\n>> avoid triggering a renegotiation when running the task for the \n>> first\n>> time.\n>>\n>> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n>> ---\n>>  src/gstreamer/gstlibcamerasrc.cpp | 72 \n>>  +++++++++++++++++++++++--------\n>>  1 file changed, 55 insertions(+), 17 deletions(-)\n>>\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n>> b/src/gstreamer/gstlibcamerasrc.cpp\n>> index e7a49fef..868fa20a 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -11,7 +11,6 @@\n>>   *  - Implement GstElement::send_event\n>>   *    + Allowing application to use FLUSH/FLUSH_STOP\n>>   *    + Prevent the main thread from accessing streaming \n>>   thread\n>> - *  - Implement renegotiation (even if slow)\n>>   *  - Implement GstElement::request-new-pad (multi stream)\n>>   *    + Evaluate if a single streaming thread is fine\n>>   *  - Add application driven request (snapshot)\n>> @@ -133,6 +132,7 @@ struct GstLibcameraSrcState {\n>>  \tint queueRequest();\n>>  \tvoid requestCompleted(Request *request);\n>>  \tint processRequest();\n>> +\tvoid clearRequests();\n>>  };\n>>\n>>  struct _GstLibcameraSrc {\n>> @@ -301,23 +301,39 @@ int \n>> GstLibcameraSrcState::processRequest()\n>>  \t\t\t\t\t\t\tsrcpad, ret);\n>>  \t}\n>>\n>> -\tif (ret != GST_FLOW_OK) {\n>> -\t\tif (ret == GST_FLOW_EOS) {\n>> -\t\t\tg_autoptr(GstEvent) eos = gst_event_new_eos();\n>> -\t\t\tguint32 seqnum = gst_util_seqnum_next();\n>> -\t\t\tgst_event_set_seqnum(eos, seqnum);\n>> -\t\t\tfor (GstPad *srcpad : srcpads_)\n>> -\t\t\t\tgst_pad_push_event(srcpad, \n>> gst_event_ref(eos));\n>> -\t\t} else if (ret != GST_FLOW_FLUSHING) {\n>> -\t\t\tGST_ELEMENT_FLOW_ERROR(src_, ret);\n>> -\t\t}\n>> +\tswitch (ret) {\n>> +\tcase GST_FLOW_OK:\n>> +\tcase GST_FLOW_NOT_NEGOTIATED:\n>> +\t\tbreak;\n>> +\tcase GST_FLOW_EOS: {\n>> +\t\tg_autoptr(GstEvent) eos = gst_event_new_eos();\n>> +\t\tguint32 seqnum = gst_util_seqnum_next();\n>> +\t\tgst_event_set_seqnum(eos, seqnum);\n>> +\t\tfor (GstPad *srcpad : srcpads_)\n>> +\t\t\tgst_pad_push_event(srcpad, gst_event_ref(eos));\n>> +\n>> +\t\terr = -EPIPE;\n>> +\t\tbreak;\n>> +\t}\n>> +\tcase GST_FLOW_FLUSHING:\n>> +\t\terr = -EPIPE;\n>> +\t\tbreak;\n>> +\tdefault:\n>> +\t\tGST_ELEMENT_FLOW_ERROR(src_, ret);\n>>\n>> -\t\treturn -EPIPE;\n>> +\t\terr = -EPIPE;\n>> +\t\tbreak;\n>>  \t}\n>>\n>>  \treturn err;\n>>  }\n>>\n>> +void GstLibcameraSrcState::clearRequests()\n>> +{\n>> +\tGLibLocker locker(&lock_);\n>> +\tcompletedRequests_ = {};\n>> +}\n>> +\n>>  static bool\n>>  gst_libcamera_src_open(GstLibcameraSrc *self)\n>>  {\n>> @@ -488,6 +504,31 @@ gst_libcamera_src_task_run(gpointer \n>> user_data)\n>>  \t\treturn;\n>>  \t}\n>>\n>> +\t// check if a srcpad requested a renegotiation\n>> +\tgboolean reconfigure = FALSE;\n>> +\tfor (GstPad *srcpad : state->srcpads_) {\n>> +\t\tif (gst_pad_check_reconfigure(srcpad)) {\n>> +\t\t\t// check whether the caps even need changing\n>> +\t\t\tg_autoptr(GstCaps) caps = \n>> gst_pad_get_current_caps(srcpad);\n>> +\t\t\tg_autoptr(GstCaps) intersection = \n>> gst_pad_peer_query_caps(srcpad, caps);\n>\n> Just wondering, but maybe gst_pad_peer_query_accept_caps() would \n> do the job,\n> with a slightly improved performance ? This is only possible \n> since\n> gst_pad_get_current_caps() is fixed. At this point, it should \n> cannot be null\n> either.\n\nI will test if this works and if it does add it to v2.\n\nRegards,\n\nJaslo\n\n>> +\n>> +\t\t\tif (gst_caps_is_empty(intersection))\n>> +\t\t\t\treconfigure = TRUE;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\tif (reconfigure) {\n>> +\t\tstate->cam_->stop();\n>> +\t\tstate->clearRequests();\n>> +\n>> +\t\tif (!gst_libcamera_src_negotiate(self)) {\n>> +\t\t\tGST_ELEMENT_FLOW_ERROR(self, \n>> GST_FLOW_NOT_NEGOTIATED);\n>> +\t\t\tgst_task_stop(self->task);\n>> +\t\t}\n>> +\n>> +\t\tstate->cam_->start(&state->initControls_);\n>> +\t}\n>> +\n>>  \t/*\n>>  \t * Create and queue one request. If no buffers are \n>>  available the\n>>  \t * function returns -ENOBUFS, which we ignore here as \n>>  that's not a\n>> @@ -594,6 +635,7 @@ gst_libcamera_src_task_enter(GstTask *task, \n>> [[maybe_unused]] GThread *thread,\n>>  \t\tgst_segment_init(&segment, GST_FORMAT_TIME);\n>>  \t\tgst_pad_push_event(srcpad, \n>>  gst_event_new_segment(&segment));\n>>\n>> +\t\tgst_pad_check_reconfigure(srcpad); // clear \n>> reconfigure flag\n>>  \t}\n>>\n>>  \tif (self->auto_focus_mode != controls::AfModeManual) {\n>> @@ -629,11 +671,7 @@ \n>> gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,\n>>  \tGST_DEBUG_OBJECT(self, \"Streaming thread is about to \n>>  stop\");\n>>\n>>  \tstate->cam_->stop();\n>> -\n>> -\t{\n>> -\t\tGLibLocker locker(&state->lock_);\n>> -\t\tstate->completedRequests_ = {};\n>> -\t}\n>> +\tstate->clearRequests();\n>>\n>>  \t{\n>>  \t\tGLibRecLocker locker(&self->stream_lock);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 114F4BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 23 Nov 2023 11:30:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 367BE629AF;\n\tThu, 23 Nov 2023 12:30:54 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.217])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94ED961DAC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 Nov 2023 12:30:53 +0100 (CET)","from archlinux by smtp.strato.de (RZmta 49.9.1 AUTH)\n\twith ESMTPSA id j3f4eezANBUqsNG\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tThu, 23 Nov 2023 12:30:52 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700739054;\n\tbh=EBG/afvpZiZ02aYUzK6B4KtZxXdU5XUGCaESo2e8NJI=;\n\th=References:To:Date:In-reply-to:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=hILNJ5BqJFGY2uHA48thIfIa6ZOi2OF1rs1gcwkIDtPHdMufJ+WXq8Ye/3S0h5FxJ\n\tQ3I88LVlyJScYVF4hb85QbwXqNT1Fvd58qXYy+n31MLcv0kQ6NNnOQcYtn8jor2awl\n\t8zbGYz3TbWnFmMrn1XvSeuxMLsiJI1bJ/veNPFdAAyTQEZpEebVF9UiN0JKijpiOTG\n\tOGowtFFCKtpazZTyIoystGUKn7tZaYGkmxcQbjKRC4HoLjVa61l/8EJJ6BVRgm8oax\n\t+3ftax3SUScXNpdPlV0PVmjDKHJFSQkw9pt9HYE9LmXvIDJbfHGc8XcYCSVQ+9ix9c\n\tsc2Gdhrj0scuw==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1700739052;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=6ttj6njLrDMLTJhpGXUkr1mqBxdLXnj5+spLcRbbOOM=;\n\tb=qinz6u80R1a62VZzeQmH6gw2ScrochqtyOintqsSKnltYN0C/f0LPG1SmO227LD/VN\n\tX/kDiSKS90j++mCWqaoOYRjXp8SdkL120UDEIWbtITKPZat/W9961stW017bOZy51dbw\n\tgDfH9yofiwcNosBPRk1jM/jRUYO/nVNcltVNqPkU10UTEF+kpzPV955AoQxeNgo/e1/M\n\tRB5Xk+YlOJwqOQObPOLT1sPv3f7CU3SIkZkIC3qyIkAU7BR1xj9UmNOxpQG8+eBO+jkb\n\tFazgz1VToViwGPjFyiWtJRWytr+HtthIoMt1NeLFZ9Hxo04kZgDjpHqC3NM6zmzqBs6k\n\tavJw==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1700739052;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=6ttj6njLrDMLTJhpGXUkr1mqBxdLXnj5+spLcRbbOOM=;\n\tb=RQppvfy/CINM35ai8wF1TnlHOmlY9s9plGyx5k9GOmrrcuUVXHIcty4Wco8m+dOVrp\n\tOauak5mI0bwoy/ti1eBA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"qinz6u80\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"RQppvfy/\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1700739052; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=m7XS357e2kfQMlkmNkiX1rPBBsbmZ/rRU6RkKZltvyygP+Wz0yVgBtuGenH06I2UiZ\n\tH4omW45mzqWmuVnMNvfDi6m71Y9O4HMhRdd6mHZdHCz4Gee00yshNbD+mx4xaIxWML5l\n\tGK9+X64V5wisjDRiAqmd/cAb9qTMR9gxyl9QgFQW38EF+F/a/0mcLUzro0FOq9JEPBtr\n\tKNzsSYHqYWqMJGMTovJ9e+np9A5L8sHKXSw9U+3VWHVQkdhZEtmu+2i5hVu4D4PgqFO6\n\tvSUk4ZAWv93SEXJi4VGEEP79eHScTPhOKcy+mFfwyWHoSaR6mR703iT1j+Fa0EN/i0aL\n\tRvIg==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1700739052;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=6ttj6njLrDMLTJhpGXUkr1mqBxdLXnj5+spLcRbbOOM=;\n\tb=Uth6DJvLGPWdARO+FYeTA6f6euBh56LChFOlBt5E7ZgBJcYSgwN7E50fBy83mEMRe8\n\tRmLNHaC/GiXgEVY7HAEGypQBo9XfVdJE7pE9aKaR7b0HZuEbhfgPlV06zMwWlbuhJSYR\n\tgv7VUFXdsl7vWjypybEDTRn0KOLZe/PwKhVtokKzCSS2TwIlOMv8awQTMNqYBLQZrW/R\n\tbE7NduJ/p/dDN6lWBiSMOxI653iMPRjDmPbiblxRqC8reZS/0UgbmzYH9VLQHTg3hn/+\n\tfJIZhu7bNJI/g+zN/PYCB9qUWxpNVnGO80lWaBbmRD81YmRIkxzIAOWWmuzAy203uxPn\n\trbYQ==","ARC-Authentication-Results":"i=1; strato.com;\n    arc=none;\n    dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusQvJpnsql/jCNNVfDSgNWRnQi1f6u5xqcJh9iidb8nasMtp1Qzgslmi0VOElk=\"","References":"<20231122135406.14921-1-jaslo@ziska.de>\n\t<20231122135406.14921-3-jaslo@ziska.de>\n\t<a2d7c004997ba01661c21bdf3c29f93790aa586b.camel@ndufresne.ca>","User-agent":"mu4e 1.10.8; emacs 29.1","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Date":"Thu, 23 Nov 2023 12:00:15 +0100","In-reply-to":"<a2d7c004997ba01661c21bdf3c29f93790aa586b.camel@ndufresne.ca>","Message-ID":"<87v89slna4.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH 2/2] gstreamer: Implement renegotiation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jaslo Ziska via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jaslo Ziska <jaslo@ziska.de>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]