GNOME Bugzilla – Bug 581011
Move DVB plugins to -good
Last modified: 2018-11-03 14:39:13 UTC
Otherwise we can't ship gnome-dvb-daemon in Fedora.
You can, it just takes some extra effort on your side. There are other things in -bad which you want as well, like input-selector for playbin2 and rtpmanager.
Comments on dvbsrc. Some of them may be uninformed. dvbsrc should have a device= property, like v4l2src. tuning needs to be asynchronous. Holding a lock and not returning from g_object_set() until tuning is finished is not ok. - What is the most appropriate thread to do tuning? In PLAYING, probably streaming thread. In READY, probably special thread. Restore timeout property from 5616efb0. polarity property should be an enum (or boolean). Or autoprobed. Needs documentation of dvb-adapter structure. Needs documentation of dvb-frontend-stats structure. Perhaps should have a URI handler Move gst_dvbsrc_plugin_init() somewhere else. In read_dvb_device(), why are there two timeout paths? In read_dvb_device(), it would be better to configure the device not to wake up until there are N packets to read, and/or some delay has passed. This avoids unnecessary read()/poll() syscalls. Is this possible? Buffers should be timestamped by the hardware. What is the purpose of gst_dvbsrc_start_stop_filters()? Please document. gst_dvbsrc_frontend_status() has a usleep(). This should be removed. Move diseqc to separate source file?
Oops, this is about fmilo's branch, but I'm mostly ready to push it.
> dvbsrc should have a device= property, like v4l2src. how would be different from the adapter and frontend properties ? one adapter(ie. device) could potentially have multiple frontend. > tuning needs to be asynchronous. Agreed. > Holding a lock and not returning from g_object_set() > until tuning is finished is not ok. Agreed. > - What is the most appropriate thread to do tuning? In PLAYING, > probably streaming thread. In READY, probably special thread. > Restore timeout property from 5616efb0. Yes there are many timeouts inside the logic, one for the poll, one for acquiring the signal, one for retrying. and are all "timeout" but I think the main timeout sense is how long should wait before giving up in tuning. right ? > polarity property should be an enum (or boolean). Or autoprobed. > Needs documentation of dvb-adapter structure. > Needs documentation of dvb-frontend-stats structure. ok. > Perhaps should have a URI handler Are you talking about : dvb_base_bin_uri_set_uri inside dvbbasebin.c ? > Move gst_dvbsrc_plugin_init() somewhere else. why ? > In read_dvb_device(), why are there two timeout paths? to differentiate from a single poll timeout and a total timeout (i.e when the signal drops). Requires a bit more of logic and I think needs to be considered with bug 641204 > In read_dvb_device(), it would be better to configure the device not > to wake up until there are N packets to read, and/or some delay has > passed. This avoids unnecessary read()/poll() syscalls. Is this > possible? I am not sure. But I agree on the strategy. > Buffers should be timestamped by the hardware. Not sure how this can be achieved. The mpegts packet contain timestamps usually but are not handled at this level. > What is the purpose of gst_dvbsrc_start_stop_filters()? Please document. I guess was for be able to add or remove at runtime PIDS to be monitored by the hardware. > gst_dvbsrc_frontend_status() has a usleep(). This should be removed. Agreed > Move diseqc to separate source file? Yes seems does not belong to the dvbsrc class too much, they are a separate set of commands. Should we remove also those usleep from the logic ?
(In reply to comment #4) > > dvbsrc should have a device= property, like v4l2src. > how would be different from the adapter and frontend properties ? > one adapter(ie. device) could potentially have multiple frontend. Other plugins usually use a char * property to indicate the device. > > Restore timeout property from 5616efb0. > Yes there are many timeouts inside the logic, one for the poll, one for > acquiring the signal, one for retrying. and are all "timeout" but > I think the main timeout sense is how long should wait before giving > up in tuning. right ? Yeah, the timeout property would only be for "how long to wait until the application is told that data is not flowing", which would apply to tuning as well as signal loss. We could have separate timeouts for both of those, but I don't think there's much value in having two. I think this is the only thing that really needs to be fixed before merging your branch into bad. (Which I suppose we could talk about on that bug report.) > > Move gst_dvbsrc_plugin_init() somewhere else. > why ? Style preference. I didn't mean for you to have to fix all these things yourself before moving to -good, but a checklist of things that need to be done. This is something that I would do.
The other issue is that it depends on mpegtsparse... so those would need to be validated in order for the move to happen.
-- 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-good/issues/12.