<div dir="ltr"><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Regarding the subject, you should have mentioend [RFC] as you mentioned<br>you are still working on it and could've mention the missing pieces below.</blockquote><div>Noted. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Great results, happy to see. Also you could've mentioned the platform on<br>which you tested.<br>In this case RPi 4 + OV5647<br></blockquote><div>Noted. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-im" style="color:rgb(80,0,80)">> +void<br>> +gst_libcamera_configure_controls_from_caps(ControlList &controls, [[maybe_unused]] GstCaps *caps)<br>> +{<br>> +     /* read framerate from caps - convert to integer and set to frame_time. */<br>> +     GstStructure *s = gst_caps_get_structure(caps, 0);<br>> +     gint fps_n = -1, fps_d = -1;<br>> +     if (gst_structure_has_field(s, "framerate"))<br>> +             gst_structure_get_fraction(s, "framerate", &fps_n, &fps_d);<br><br></span>I think if the user input is malformed, get_fraction() will return a<br>FALSE. We should check that for invalid input and log errors.<br></blockquote><div>Okay.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-im" style="color:rgb(80,0,80)">> +<br>> +     if (fps_n < 0 || fps_d < 0)<br>> +             return;<br><br></span>Can it happen that only the numerator is mentioned? Have you tested<br>something like : "framerate=30"  instead of "framerate=30/1"<br></blockquote><div>I tested for the case "framerate=30". We cannot accept (int)30 for the framerate field that</div><div>is supposed to be a fraction. Passing 30 fails the negotiation as gst_pad_peer_query_caps() </div><div>returns empty caps here.</div><div>g_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);</div><div><br>              if (gst_caps_is_empty(caps)) {<br>                        flow_ret = GST_FLOW_NOT_NEGOTIATED;<br>                   break;<br>                }<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Since frame_duration is int64_t, and fps_d and fps_n are both integers,<br>can we do:<br><br>             int64_t frame_duration = (fps_d * 1000000) / fps_n; ?<span class="gmail-im" style="color:rgb(80,0,80)"><br>> +     controls.set(controls::FrameDurationLimits,<br>> +                  Span<const int64_t, 2>({ static_cast<int64_t>(frame_duration), static_cast<int64_t>(frame_duration) }));<br><br></span>you could then drop the cast from here  I believe<br></blockquote><div>Right.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">nit: To be pedantic, we are only parsing controls needed at<br>camera::StarT(); so maybe<br>         gst_libcamera_get_init_ctrls_from_caps(...);<br>?</blockquote><div><span id="gmail-docs-internal-guid-3ccd58d1-7fff-a3d1-7162-a99134b86f9f"><span style="font-size:11pt;font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;vertical-align:baseline;white-space:pre-wrap">Yes, this will make more sense. </span></span></div></div></div>