[vlc-devel] [PATCH v2] Add support for reading EIA-608 closed captions via V4L2 VBI devices

[Devin Heitmueller]

On Mon, Oct 29, 2012 at 4:23 AM, Rémi Denis-Courmont <remi at remlab.net> wrote:
>> +    /* Now that everything is setup, start the reading thread */
>> +    if (vlc_clone (&sys->thread, entry, demux,
> VLC_THREAD_PRIORITY_INPUT))
>> +    {
>> +        if (sys->bufv != NULL)
>> +            StopMmap (sys->fd, sys->bufv, sys->bufc);
>> +        goto error;
>
> That leaks, I think.
> It might be simpler to call InitVBI() from InitVideo().

I've got no opinion one way or the other.  My only thinking was
keeping it one level higher kept better symmetry between the VBI and
Video initialization processes.

>> +    }
>> +
>>      sys->controls = ControlsInit (VLC_OBJECT(demux), fd);
>>      sys->start = mdate ();
>>      demux->pf_demux = NULL;
>> @@ -439,14 +470,33 @@ static int InitVideo (demux_t *demux, int fd,
>> uint32_t caps)
>>          return -1;
>>      }
>>
>> -    if (vlc_clone (&sys->thread, entry, demux,
> VLC_THREAD_PRIORITY_INPUT))
>> +    return 0;
>> +}
>> +
>> +#ifdef ZVBI_COMPILED
>> +static int InitVBI (demux_t *demux)
>> +{
>> +    demux_sys_t *sys = demux->p_sys;
>> +
>> +    for (int i = 0; i < VBI_NUM_CC_STREAMS; i++)
>>      {
>> -        if (sys->bufv != NULL)
>> -            StopMmap (sys->fd, sys->bufv, sys->bufc);
>> -        return -1;
>> +        es_format_t fmt;
>> +
>> +        es_format_Init( &fmt, SPU_ES, VLC_FOURCC('c','c',0x31 + i,' ')
> );
>
> Cosmetic but I'd use '1' rather than 0x31.

Ok.

>> +        if (asprintf(&fmt.psz_description, "CC%d", i + 1) >= 0)
>> +        {
>> +            msg_Dbg( demux, "new spu es %4.4s", (char*)&fmt.i_codec );
>> +            sys->p_es_subt[i] = es_out_Add( demux->out, &fmt );
>> +        }
>>      }
>> +
>> +    /* Do a single read and throw away the results so that ZVBI calls
>> +       the STREAMON ioctl() */
>> +    GrabVBI(demux, sys->vbi_cap, sys->p_es_subt, VBI_NUM_CC_STREAMS);
>> +
>>      return 0;
>>  }
>> +#endif
>>
>>  void DemuxClose( vlc_object_t *obj )
>>  {
>> @@ -459,6 +509,15 @@ void DemuxClose( vlc_object_t *obj )
>>          StopMmap (sys->fd, sys->bufv, sys->bufc);
>>      ControlsDeinit( obj, sys->controls );
>>      v4l2_close (sys->fd);
>> +
>> +#ifdef ZVBI_COMPILED
>> +    if( sys->vbi_cap )
>> +    {
>> +        close(vbi_capture_fd(sys->vbi_cap));
>
> Oh? zvbi does not close the descriptor in that case?

Correct.  If the file descriptor is provided to zvbi at
initialization, the expectation is that the caller takes
responsibility for closing it (the zvbi library internally has a flag
on its struct to indicate not to close the FD).

I'll roll those changes in and resubmit.

Thanks for the feedback,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com