| Message ID | 20210322104242.31107-4-m.cichy@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Le lundi 22 mars 2021 à 11:42 +0100, Marian Cichy a écrit : > Access the frame duration from the controlList of the stream > configuration and convert it to values fitting for GST_FRACTION. By > setting the frame rate to the caps configuration, the Gstreamer pipeline > has information about the camera frame rate, which can be potentially > used for more efficient memory allocation or for debug purposes. > > Signed-off-by: Marian Cichy <m.cichy@pengutronix.de> > --- > src/gstreamer/gstlibcamera-utils.cpp | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > index 5381dca5..fdd8b85c 100644 > --- a/src/gstreamer/gstlibcamera-utils.cpp > +++ b/src/gstreamer/gstlibcamera-utils.cpp > @@ -8,6 +8,7 @@ > > > > > #include "gstlibcamera-utils.h" > > > > +#include <libcamera/control_ids.h> > #include <libcamera/formats.h> > > using namespace libcamera; > @@ -128,10 +129,16 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg > { > GstCaps *caps = gst_caps_new_empty(); > GstStructure *s = bare_structure_from_format(stream_cfg.pixelFormat); > + int numerator; > + int denominator; > + double framerate = 1'000'000 / static_cast<double>(stream_cfg.controls.get( > + controls::FrameDurations)[0]); Perhaps that would be defensive, but I'm worried duration might be 0, which would crash here. Can you explain the array of duration, and why we know that the first item exist (to we don't overrun the array) and will be the selected one ? > > + gst_util_double_to_fraction(framerate, &numerator, &denominator); > gst_structure_set(s, > "width", G_TYPE_INT, stream_cfg.size.width, > "height", G_TYPE_INT, stream_cfg.size.height, > + "framerate", GST_TYPE_FRACTION, numerator, denominator, If duration is 0, this should be skipped, or set to 0/1 (which means variable framerate in GStreamer). > nullptr); > gst_caps_append_structure(caps, s); > > > > > > > >
On 3/22/21 8:31 PM, Nicolas Dufresne wrote: > Le lundi 22 mars 2021 à 11:42 +0100, Marian Cichy a écrit : >> Access the frame duration from the controlList of the stream >> configuration and convert it to values fitting for GST_FRACTION. By >> setting the frame rate to the caps configuration, the Gstreamer pipeline >> has information about the camera frame rate, which can be potentially >> used for more efficient memory allocation or for debug purposes. >> >> Signed-off-by: Marian Cichy <m.cichy@pengutronix.de> >> --- >> src/gstreamer/gstlibcamera-utils.cpp | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp >> index 5381dca5..fdd8b85c 100644 >> --- a/src/gstreamer/gstlibcamera-utils.cpp >> +++ b/src/gstreamer/gstlibcamera-utils.cpp >> @@ -8,6 +8,7 @@ >> >> >> >> >> #include "gstlibcamera-utils.h" >> >> +#include <libcamera/control_ids.h> >> #include <libcamera/formats.h> >> >> using namespace libcamera; >> @@ -128,10 +129,16 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg >> { >> GstCaps *caps = gst_caps_new_empty(); >> GstStructure *s = bare_structure_from_format(stream_cfg.pixelFormat); >> + int numerator; >> + int denominator; >> + double framerate = 1'000'000 / static_cast<double>(stream_cfg.controls.get( >> + controls::FrameDurations)[0]); > Perhaps that would be defensive, but I'm worried duration might be 0, which > would crash here. Noted > > Can you explain the array of duration, and why we know that the first item exist > (to we don't overrun the array) and will be the selected one ? By definition of the auto-generated controls, FrameDurations is a uint64_t[2] where [0] is the minFrameDuration and [1] the maxFrameDuration. I don't think this control can be generated in any other way, as libcamera auto-generates these controls. > >> + gst_util_double_to_fraction(framerate, &numerator, &denominator); >> gst_structure_set(s, >> "width", G_TYPE_INT, stream_cfg.size.width, >> "height", G_TYPE_INT, stream_cfg.size.height, >> + "framerate", GST_TYPE_FRACTION, numerator, denominator, > If duration is 0, this should be skipped, or set to 0/1 (which means variable > framerate in GStreamer). Skipping seems fine, as Gstreamer seems to default to 0/1, right? Marian > >> nullptr); >> gst_caps_append_structure(caps, s); >> >> >> >> >> >> >> >> > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Le lundi 22 mars 2021 à 20:37 +0100, Marian Cichy a écrit : > On 3/22/21 8:31 PM, Nicolas Dufresne wrote: > > Le lundi 22 mars 2021 à 11:42 +0100, Marian Cichy a écrit : > > > Access the frame duration from the controlList of the stream > > > configuration and convert it to values fitting for GST_FRACTION. By > > > setting the frame rate to the caps configuration, the Gstreamer pipeline > > > has information about the camera frame rate, which can be potentially > > > used for more efficient memory allocation or for debug purposes. > > > > > > Signed-off-by: Marian Cichy <m.cichy@pengutronix.de> > > > --- > > > src/gstreamer/gstlibcamera-utils.cpp | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > > index 5381dca5..fdd8b85c 100644 > > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > > @@ -8,6 +8,7 @@ > > > > > > > > > > > > > > > > > > > > > > > > #include "gstlibcamera-utils.h" > > > > > > > > > > > > > > > +#include <libcamera/control_ids.h> > > > #include <libcamera/formats.h> > > > > > > > > > > > > > > > using namespace libcamera; > > > @@ -128,10 +129,16 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg > > > { > > > GstCaps *caps = gst_caps_new_empty(); > > > GstStructure *s = bare_structure_from_format(stream_cfg.pixelFormat); > > > + int numerator; > > > + int denominator; > > > + double framerate = 1'000'000 / static_cast<double>(stream_cfg.controls.get( > > > + controls::FrameDurations)[0]); > > Perhaps that would be defensive, but I'm worried duration might be 0, which > > would crash here. > > > Noted > > > > > > Can you explain the array of duration, and why we know that the first item exist > > (to we don't overrun the array) and will be the selected one ? > > > By definition of the auto-generated controls, FrameDurations is a > uint64_t[2] where [0] is the minFrameDuration and [1] the > maxFrameDuration. I don't think this control can be generated in any > other way, as libcamera auto-generates these controls. Thanks, I'm not familiar with how libcamera controls have been implemented. This looks a bit strange considering that there is ranges type for width and height somewhere else, but now that I know it's a range, it helps. The minimum framerate will trigger the highest latency, so that is definitely the best value to expose in GStreamer. > > > > > > > + gst_util_double_to_fraction(framerate, &numerator, &denominator); > > > gst_structure_set(s, > > > "width", G_TYPE_INT, stream_cfg.size.width, > > > "height", G_TYPE_INT, stream_cfg.size.height, > > > + "framerate", GST_TYPE_FRACTION, numerator, denominator, > > If duration is 0, this should be skipped, or set to 0/1 (which means variable > > framerate in GStreamer). > > > Skipping seems fine, as Gstreamer seems to default to 0/1, right? Indeed, both ways do, I think I would set it to 0/1 as it's more explicit that we don't know and what to expect. Users can always force it with videorate afterward. > > > Marian > > > > > > > nullptr); > > > gst_caps_append_structure(caps, s); > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
Hello, On Mon, Mar 22, 2021 at 03:31:20PM -0400, Nicolas Dufresne wrote: > Le lundi 22 mars 2021 à 11:42 +0100, Marian Cichy a écrit : > > Access the frame duration from the controlList of the stream > > configuration and convert it to values fitting for GST_FRACTION. By > > setting the frame rate to the caps configuration, the Gstreamer pipeline > > has information about the camera frame rate, which can be potentially > > used for more efficient memory allocation or for debug purposes. > > > > Signed-off-by: Marian Cichy <m.cichy@pengutronix.de> > > --- > > src/gstreamer/gstlibcamera-utils.cpp | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > index 5381dca5..fdd8b85c 100644 > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > @@ -8,6 +8,7 @@ > > > > > > > > > > #include "gstlibcamera-utils.h" > > > > > > > +#include <libcamera/control_ids.h> > > #include <libcamera/formats.h> > > > > using namespace libcamera; > > @@ -128,10 +129,16 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg > > { > > GstCaps *caps = gst_caps_new_empty(); > > GstStructure *s = bare_structure_from_format(stream_cfg.pixelFormat); > > + int numerator; > > + int denominator; > > + double framerate = 1'000'000 / static_cast<double>(stream_cfg.controls.get( > > + controls::FrameDurations)[0]); > > Perhaps that would be defensive, but I'm worried duration might be 0, which > would crash here. Also there are no guarantee that FrameDurations is part of the stream's control list, so all of this i probably better wrapped in a if (stream_cfg.contains(controls::FrameDurations)) { } > > Can you explain the array of duration, and why we know that the first item exist > (to we don't overrun the array) and will be the selected one ? > > > > > + gst_util_double_to_fraction(framerate, &numerator, &denominator); > > gst_structure_set(s, > > "width", G_TYPE_INT, stream_cfg.size.width, > > "height", G_TYPE_INT, stream_cfg.size.height, > > + "framerate", GST_TYPE_FRACTION, numerator, denominator, > > If duration is 0, this should be skipped, or set to 0/1 (which means variable > framerate in GStreamer). > > > nullptr); > > gst_caps_append_structure(caps, s); > > > > > > > > > > > > > > > > > > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp index 5381dca5..fdd8b85c 100644 --- a/src/gstreamer/gstlibcamera-utils.cpp +++ b/src/gstreamer/gstlibcamera-utils.cpp @@ -8,6 +8,7 @@ #include "gstlibcamera-utils.h" +#include <libcamera/control_ids.h> #include <libcamera/formats.h> using namespace libcamera; @@ -128,10 +129,16 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg { GstCaps *caps = gst_caps_new_empty(); GstStructure *s = bare_structure_from_format(stream_cfg.pixelFormat); + int numerator; + int denominator; + double framerate = 1'000'000 / static_cast<double>(stream_cfg.controls.get( + controls::FrameDurations)[0]); + gst_util_double_to_fraction(framerate, &numerator, &denominator); gst_structure_set(s, "width", G_TYPE_INT, stream_cfg.size.width, "height", G_TYPE_INT, stream_cfg.size.height, + "framerate", GST_TYPE_FRACTION, numerator, denominator, nullptr); gst_caps_append_structure(caps, s);
Access the frame duration from the controlList of the stream configuration and convert it to values fitting for GST_FRACTION. By setting the frame rate to the caps configuration, the Gstreamer pipeline has information about the camera frame rate, which can be potentially used for more efficient memory allocation or for debug purposes. Signed-off-by: Marian Cichy <m.cichy@pengutronix.de> --- src/gstreamer/gstlibcamera-utils.cpp | 7 +++++++ 1 file changed, 7 insertions(+)