Hi Kieran,
On Wednesday, 7 November 2018 16:30:46 EET Kieran Bingham wrote:
> On 06/11/2018 23:13, Laurent Pinchart wrote:
> > On Tuesday, 6 November 2018 23:27:19 EET Kieran Bingham wrote:
> >> From: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> >>
> >> We have both uvc_init_video() and uvc_video_init() calls which can be
> >> quite confusing to determine the process for each. Now that video
> >> uvc_video_enable() has been renamed to uvc_video_start_streaming(),
> >> adapt these calls to suit the new flow.
> >>
> >> Rename uvc_init_video() to uvc_video_start() and uvc_uninit_video() to
> >> uvc_video_stop().
> >
> > I agree that these functions are badly named and should be renamed. We are
> > however entering the nitpicking territory :-) The two functions do more
> > that starting and stopping, they also allocate and free URBs and the
> > associated buffers. It could also be argued that they don't actually
> > start and stop anything, as beyond URB management, they just queue the
> > URBs initially and kill them. I thus wonder if we could come up with
> > better names.
>
> Well the act of killing (poisoning now) the URBs will certainly stop the
> stream, but I guess submitting the URBs isn't necessarily the key act to
> starting the stream.
>
> I believe that needs the interface to be set correctly, and the buffers
> to be available?
>
> Although - I've just double-checked uvc_{video_start,init_video}() and
> that is indeed what it does?
>
> - start stats
> - Initialise endpoints
> - Perform allocations
> - Submit URBs
>
> Am I missing something? Is there another step that is pivotal to
> starting the USB packet/urb stream flow after this point ?
>
>
> Is it not true that the USB stack will start processing data at
> submitting URB completion callbacks after the end of uvc_video_start();
> and will no longer process data at the end of uvc_video_stop() (and thus
> no more completion callbacks)?
>
> (That's a real question to verify my interpretation)
>
> To me - these functions feel like the real 'start' and 'stop' components
> of the data stream - hence my choice in naming.
The other part of the start operation is committing the streaming parameters
(see uvc_video_start_streaming()). For the stop operation it's issuing a
SET_INTERFACE or CLEAR_FEATURE(HALT) request (see uvc_video_stop_streaming()).
> Is your concern that you would like the functions to be more descriptive
> over their other actions such as? :
>
> uvc_video_initialise_start()
> uvc_video_allocate_init_start()
>
> Or something else? (I don't think those two are good names though)
Probably something else :-) A possibly equally bad proposal would be
uvc_video_start_transfer() and uvc_video_stop_transfer().
--
Regards,
Laurent Pinchart
