GNOME Bugzilla – Bug 748105
videotestsrc: add a 'max-duration' property
Last modified: 2018-11-03 11:36:57 UTC
Created attachment 301900 [details] [review] videotestsrc: add a 'max-duration' property This patch adds a new property to the `videotestsrc` element that allows you to set the maximum duration. After the specified time (in nano seconds) the `videotestsrc` will push EOS onto it's src pad. There are other ways to do this of course, but I found this the most accurate method. So, I am not entirely sure this change is an acceptable one. Tested on Linux/Debian x64 (patch file was generated on Windows). Patch is against master.
There are some additional formatting changes in the patch. These were made by running gst-indent on the modified files. I can easily provide two separate patches/commits for this if necessary.
Comment on attachment 301900 [details] [review] videotestsrc: add a 'max-duration' property Thanks for the patch. I think this is a good idea and a useful addition. Please try to produce the smallest patch posssible, ideally without formatting changes. We don't run header files through gst-indent, only .c files. Thanks! I also expect that there will be some more discussion about the name of the property, perhaps it should just be "duration" instead? We'll want the same for audiotestsrc as well then. Couple of other comments: - the default value of the property should be -1 for "disabled", not 0. - not sure what all this G_MAXLONG business in the param spec is about :) - The duration should be reflected in the GstSegment pushed out, and hopefully the base class will take care of the EOS-ing automatically then. We may require some clipping in addition for the buffers we produce (so that the last buffer has a shorter duration to make sure the stream has exactly the duration we want).
Thanks for the feedback, it is much appriciated. I wasn't aware that only implementation files were run through gst-indent. I'll produce a new patch with the smallest amount of changes possible. However, when I ran the implementation file through gst-indent, not only did it fix some of my formatting mistakes, it also fixed some made by others. Shouldn't they be fixed as well? I could produce two separate patches, one for my changes, and one for formatting fixes, not related to the changes I made. Would that be alright? As for the name, I had my doubts about it as well. I first wanted to name it "duration", but "max-duration" seemed more descriptive. As for the other comments, I'll take care of them and produce a new fix. However, the last comment is where I might run into trouble as I am not that familiar with gstreamer, but I'll see how far I get. It shouldn't be too hard to port this to audiotestsrc as well, if I have something working that is "acceptable" then I'll port it to audiotestsrc as well.
Regarding the GstSegment, try this: in gst_video_test_src_start() add: basesrc->segment.stop = 5 * GST_SECOND; basesrc->segment.duration = 5 * GST_SECOND; The GstBaseSrc base class will basically stop automatically when that value is reached, so you don't need to add any code for that.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/181.