No audio support in struct v4l2_subdev_format

No audio support in struct v4l2_subdev_format

[Divneil Wadhawan]

Hello,


There's an HDMIRx subdev I have implemented and I would like the application to read properties of incoming audio.

For the moment, there's no support of it.

Can you share if some work is already done on this but not complete or we do it from scratch?


Regards,

Divneil 		 	   		  

Re: No audio support in struct v4l2_subdev_format

[Hans Verkuil]

On 07/04/2014 08:49 AM, Divneil Wadhawan wrote:
> Hello,
> 
> 
> There's an HDMIRx subdev I have implemented and I would like the application to read properties of incoming audio.
> 
> For the moment, there's no support of it.
> 
> Can you share if some work is already done on this but not complete or we do it from scratch?

To my knowledge nobody has done much if any work on this. Usually the
audio part is handled by alsa, but it is not clear if support is also
needed from the V4L2 API. If such support is needed it will most likely
be in the form of a bunch of new controls.

Regards,

	Hans

RE: No audio support in struct v4l2_subdev_format

[Divneil Wadhawan]

Hi Hans,


> To my knowledge nobody has done much if any work on this. Usually the
> audio part is handled by alsa, but it is not clear if support is also
> needed from the V4L2 API.

Actually, the application needs to know when to ask the capture device to start capturing.

Let's say, the cable is already plugged in/or plugged out.

So, any events will be missed as the driver state machine starts during boot up and app is not started.

App starts later, registers for (V4L2_EVENT_SOURCE_CHANGE back ported to 3.10) and listens, but will not receive any as they are already generated.


So, the application is in a blind spot whether to start capture or not.

If we get the same interface as video it's good. I mean G_FMT with a union for audio as well.

Otherwise, I can go with a proprietary control/ioctl indicating whether audio is valid or not. 

ioctl seems to be an easy choice, because this subdev is not exposing any controls, so, registration with ctrl framework for a single one seems a bit of overload.


Regards,

Divneil 		 	   		  

Re: No audio support in struct v4l2_subdev_format

[Hans Verkuil]

Hi Divneil,

On 07/04/2014 11:30 AM, Divneil Wadhawan wrote:
> Hi Hans,
> 
> 
>> To my knowledge nobody has done much if any work on this. Usually
>> the audio part is handled by alsa, but it is not clear if support
>> is also needed from the V4L2 API.
> 
> Actually, the application needs to know when to ask the capture
> device to start capturing.
> 
> Let's say, the cable is already plugged in/or plugged out.
> 
> So, any events will be missed as the driver state machine starts
> during boot up and app is not started.
> 
> App starts later, registers for (V4L2_EVENT_SOURCE_CHANGE back
> ported to 3.10) and listens, but will not receive any as they are
> already generated.

I don't understand the problem. When the application starts the first thing it should
do is to call VIDIOC_QUERY_DV_TIMINGS: that will tell it if any signal is present.

An alternative might be to extend v4l2_src_change_event_subscribe() and have it honor
the V4L2_EVENT_SUB_FL_SEND_INITIAL flag: if set, it should issue this event at once.
That might actually be a nice improvement.

> 
> 
> So, the application is in a blind spot whether to start capture or
> not.

But what has that to do with audio?

> If we get the same interface as video it's good. I mean G_FMT with a
> union for audio as well.
> 
> Otherwise, I can go with a proprietary control/ioctl indicating
> whether audio is valid or not.
> 
> ioctl seems to be an easy choice, because this subdev is not
> exposing any controls, so, registration with ctrl framework for a
> single one seems a bit of overload.

It's perfectly fine to have a single control. Nothing wrong with that.

Still not sure what you actually need in V4L2 w.r.t. the audio information.


Regards,

	Hans

RE: No audio support in struct v4l2_subdev_format

[Divneil Wadhawan]

Hi Hans,


> I don't understand the problem. When the application starts the first thing it should
> do is to call VIDIOC_QUERY_DV_TIMINGS: that will tell it if any signal is present.

I guess dv timings will be valid for DVI too. If yes, then there is no audio.

There's no video device for this, just subdev.



> An alternative might be to extend v4l2_src_change_event_subscribe() and have it honor
> the V4L2_EVENT_SUB_FL_SEND_INITIAL flag: if set, it should issue this event at once.
> That might actually be a nice improvement.

Okay, if it gives the already fired event, then it's good.


There is one concern for this v4l2_src_change_event_subscribe().

I was using v4l2_subscribed_event_ops for storing the "struct v4l2_subscribed_event sev."


Later take out from list so, that I can call v4l2_event_queue() from the async event handler.


I didn't had to worry for addition/deletion of "sev" from my list as this was managed by event framework calling these ops.

So, now with v4l2_src_change_event_subscribe(), I cannot use the ops, and I have to manage "fh" using list.

This addition deletion must now be handled by me, and cannot be taken care by v4l2_event_subdev_unsubscribe().

I hope I have not missed any trivial stuff ;)



> But what has that to do with audio?

For video, we have VIDIOC_SUBDEV_G_FMT, for audio, there's nothing.


Regards,

Divneil 		 	   		  

Re: No audio support in struct v4l2_subdev_format

[Hans Verkuil]

Hi Divneil,

On 07/04/2014 12:24 PM, Divneil Wadhawan wrote:
> 
> 
> Hi Hans,
> 
> 
>> I don't understand the problem. When the application starts the first thing it should
>> do is to call VIDIOC_QUERY_DV_TIMINGS: that will tell it if any signal is present.
> 
> I guess dv timings will be valid for DVI too. If yes, then there is no audio.
> 
> There's no video device for this, just subdev.
> 
> 
> 
>> An alternative might be to extend v4l2_src_change_event_subscribe() and have it honor
>> the V4L2_EVENT_SUB_FL_SEND_INITIAL flag: if set, it should issue this event at once.
>> That might actually be a nice improvement.
> 
> Okay, if it gives the already fired event, then it's good.

It should generate an initial SOURCE_CHANGE event with 'changes' set to
V4L2_EVENT_SRC_CH_RESOLUTION. That way the application that just subscribed to this
event with V4L2_EVENT_SUB_FL_SEND_INITIAL will get an initial event.

> 
> 
> There is one concern for this v4l2_src_change_event_subscribe().
> 
> I was using v4l2_subscribed_event_ops for storing the "struct v4l2_subscribed_event sev."
> 
> 
> Later take out from list so, that I can call v4l2_event_queue() from the async event handler.
> 
> 
> I didn't had to worry for addition/deletion of "sev" from my list as this was managed by event framework calling these ops.
> 
> So, now with v4l2_src_change_event_subscribe(), I cannot use the ops, and I have to manage "fh" using list.
> 
> This addition deletion must now be handled by me, and cannot be taken care by v4l2_event_subdev_unsubscribe().

I hate to say this, but I have no idea what you mean. Can you show some code?

> 
> I hope I have not missed any trivial stuff ;)
> 
> 
> 
>> But what has that to do with audio?
> 
> For video, we have VIDIOC_SUBDEV_G_FMT, for audio, there's nothing.

But shouldn't that be handled by an alsa driver? That's what someone has to
figure out: what goes in alsa and what still has to be provided by V4L2. For HDMI
output in e.g. an nvidia card the audio is fully handled by alsa AFAIK.

Obviously the hdmi driver has to work together with the alsa driver since it has
to exchange the audio infoframe information, but that's internal to the kernel.

To my knowledge nobody has really worked on this.

Regards,

	Hans

RE: No audio support in struct v4l2_subdev_format

[Divneil Wadhawan]

Hi Hans,



> It should generate an initial SOURCE_CHANGE event with 'changes' set to
> V4L2_EVENT_SRC_CH_RESOLUTION. That way the application that just subscribed to this
> event with V4L2_EVENT_SUB_FL_SEND_INITIAL will get an initial event.

Just checked in 3.10 which I am using, this is for the control events.

So, I will check in detail and in case fits our case, will reuse the part in my code.


> I hate to say this, but I have no idea what you mean. Can you show some code?


static int hdmirx_evt_add(struct v4l2_subscribed_event *sev, unsigned elems)
{
        struct v4l2_fh *fh = sev->fh;
        struct v4l2_subdev *sd = vdev_to_v4l2_subdev(fh->vdev);
        struct hdmirx_ctx_s *hdmirx_ctx = v4l2_get_subdevdata(sd);


        .......


        list_add_tail(&sev->node, &hdmirx_ctx->subs_list);


        ........


        return 0;
}


static void hdmirx_evt_del(struct v4l2_subscribed_event *sev)
{
        struct v4l2_fh *fh = sev->fh;
        struct v4l2_subdev *sd = vdev_to_v4l2_subdev(fh->vdev);
        struct hdmirx_ctx_s *hdmirx_ctx = v4l2_get_subdevdata(sd);



        .............


        list_del(&sev->node);


        ..........

}



struct v4l2_subscribed_event_ops hdmirx_event_ops = {
        .add = hdmirx_status_evt_add,
        .del = hdmirx_status_evt_del,
        .replace = v4l2_ctrl_replace,
        .merge = v4l2_ctrl_merge,
};



hdmi_core_subscribe_event()

{



 ret = v4l2_event_subscribe(fh, sub,
                     HDMIRX_EVENT_QUEUE_DEPTH, &hdmirx_event_ops);


}


static const struct v4l2_subdev_core_ops hdmirx_subdev_core_ops = {
        .ioctl =  xxx
        .subscribe_event = hdmirx_core_subscribe_event,

...

}


v4l2_event_subscribe() takes the 4th arg as the event_ops. ctrl and source_change events are not allowing 

to override ops. It can be like:

int v4l2_ctrl_subdev_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
                                     struct v4l2_event_subscription *sub, xxx )

{

    if (!xxx)

         default ops as open source


}



> But shouldn't that be handled by an alsa driver? That's what someone has to
> figure out: what goes in alsa and what still has to be provided by V4L2. For HDMI
> output in e.g. an nvidia card the audio is fully handled by alsa AFAIK.

Okay, I will come back on this later.

Let's see if I can do something better with the event handling.


Regards,

Divneil 		 	   		  

Re: No audio support in struct v4l2_subdev_format

[Hans Verkuil]

On 07/04/2014 01:38 PM, Divneil Wadhawan wrote:
> Hi Hans,
> 
> 
> 
>> It should generate an initial SOURCE_CHANGE event with 'changes' set to
>> V4L2_EVENT_SRC_CH_RESOLUTION. That way the application that just subscribed to this
>> event with V4L2_EVENT_SUB_FL_SEND_INITIAL will get an initial event.
> 
> Just checked in 3.10 which I am using, this is for the control events.
> 
> So, I will check in detail and in case fits our case, will reuse the part in my code.
> 
> 
>> I hate to say this, but I have no idea what you mean. Can you show some code?
> 
> 
> static int hdmirx_evt_add(struct v4l2_subscribed_event *sev, unsigned elems)
> {
>         struct v4l2_fh *fh = sev->fh;
>         struct v4l2_subdev *sd = vdev_to_v4l2_subdev(fh->vdev);
>         struct hdmirx_ctx_s *hdmirx_ctx = v4l2_get_subdevdata(sd);
> 
> 
>         .......
> 
> 
>         list_add_tail(&sev->node, &hdmirx_ctx->subs_list);
> 
> 
>         ........
> 
> 
>         return 0;
> }
> 
> 
> static void hdmirx_evt_del(struct v4l2_subscribed_event *sev)
> {
>         struct v4l2_fh *fh = sev->fh;
>         struct v4l2_subdev *sd = vdev_to_v4l2_subdev(fh->vdev);
>         struct hdmirx_ctx_s *hdmirx_ctx = v4l2_get_subdevdata(sd);
> 
> 
> 
>         .............
> 
> 
>         list_del(&sev->node);
> 
> 
>         ..........
> 
> }
> 
> 
> 
> struct v4l2_subscribed_event_ops hdmirx_event_ops = {
>         .add = hdmirx_status_evt_add,
>         .del = hdmirx_status_evt_del,
>         .replace = v4l2_ctrl_replace,
>         .merge = v4l2_ctrl_merge,
> };
> 
> 
> 
> hdmi_core_subscribe_event()
> 
> {
> 
> 
> 
>  ret = v4l2_event_subscribe(fh, sub,
>                      HDMIRX_EVENT_QUEUE_DEPTH, &hdmirx_event_ops);
> 
> 
> }
> 
> 
> static const struct v4l2_subdev_core_ops hdmirx_subdev_core_ops = {
>         .ioctl =  xxx
>         .subscribe_event = hdmirx_core_subscribe_event,
> 
> ...
> 
> }

Very weird code. If you want to support events all you have to do is this:

int my_subdev_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
                                     struct v4l2_event_subscription *sub)
{
	switch (sub->type) {
	case V4L2_EVENT_CTRL:
		return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
	case V4L2_EVENT_SOURCE_CHANGE:
		return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
	...
	default:
		return -EINVAL;
	}
}

Since your QUEUE_DEPTH event is custom and I have no idea what it does, I also
can't say if you really need event_ops, but it seems very peculiar. It is almost
certainly wrong what you are doing.

Regards,

	Hans

> 
> 
> v4l2_event_subscribe() takes the 4th arg as the event_ops. ctrl and source_change events are not allowing 
> 
> to override ops. It can be like:
> 
> int v4l2_ctrl_subdev_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
>                                      struct v4l2_event_subscription *sub, xxx )
> 
> {
> 
>     if (!xxx)
> 
>          default ops as open source
> 
> 
> }
> 
> 
> 
>> But shouldn't that be handled by an alsa driver? That's what someone has to
>> figure out: what goes in alsa and what still has to be provided by V4L2. For HDMI
>> output in e.g. an nvidia card the audio is fully handled by alsa AFAIK.
> 
> Okay, I will come back on this later.
> 
> Let's see if I can do something better with the event handling.
> 
> 
> Regards,
> 
> Divneil 		 	   		  --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: No audio support in struct v4l2_subdev_format

[Divneil Wadhawan]

Hi Hans,


> int my_subdev_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
> struct v4l2_event_subscription *sub)
> {
> switch (sub->type) {
> case V4L2_EVENT_CTRL:
> return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
> case V4L2_EVENT_SOURCE_CHANGE:
> return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
> ...
> default:
> return -EINVAL;
> }
The state machine is already ON, whether the user does a subdev open or not.

So, the events are generated anyways.

I guess it's not too weird as the v4l2_event_queue_fh() requires fh, and it seems okay to get hold off fh by using these ops.


Regards,

Divneil 		 	   		  

RE: No audio support in struct v4l2_subdev_format

[Divneil Wadhawan]

Hi Hans,


I agree that it was not a good implementation of using event.

(Please discard the exact code, as it is erroneous in managing ctrl events replace/merge and other ones)


I restart with the concern.

Here, I have a v4l2 subdev, which can generate events from the time we load it.

We later found some use cases, where we would like the application to get the events too.


v4l2_event_queue_fh() requires fh. 

I think, there's no way of gaining the access to this fh, except the SUBSCRIBE_EVENT or any calls landing on subdev before this.

The adding and deleting of fh in the list, is well managed by the event ops.

However, adding fh to the list is the tricky part, as I don't want to fill in the link list with the same fh over and over.


If you know of any other way, please suggest.

I hope I clarified the point this time.


Regards,

Divneil 		 	   		  

Re: No audio support in struct v4l2_subdev_format

[Hans Verkuil]

On 07/09/2014 12:55 PM, Divneil Wadhawan wrote:
> Hi Hans,
> 
> 
> I agree that it was not a good implementation of using event.
> 
> (Please discard the exact code, as it is erroneous in managing ctrl events replace/merge and other ones)
> 
> 
> I restart with the concern.
> 
> Here, I have a v4l2 subdev, which can generate events from the time we load it.
> 
> We later found some use cases, where we would like the application to get the events too.
> 
> 
> v4l2_event_queue_fh() requires fh. 
> 
> I think, there's no way of gaining the access to this fh, except the SUBSCRIBE_EVENT or any calls landing on subdev before this.
> 
> The adding and deleting of fh in the list, is well managed by the event ops.
> 
> However, adding fh to the list is the tricky part, as I don't want to fill in the link list with the same fh over and over.

I still don't understand your problem. So the application wants to subscribe to an event,
it calls VIDIOC_SUBSCRIBE_EVENT and from that point onwards it will receive those events.

All the driver does is to call v4l2_event_subscribe (possibly through helper functions
like v4l2_src_change_event_subscribe).

You never have to touch filehandles yourself, that's all done in v4l2-event.c.

When your driver needs to raise the event it will typically call v4l2_event_queue() and
in rare cases v4l2_event_queue_fh() to send an event to a specific filehandle (primarily
used by m2m devices which have a per-filehandle state).

If you would like to have an initial event that is issued as soon as a filehandle subscribes
to an event, then the application has to set the V4L2_EVENT_SUB_FL_SEND_INITIAL flag and
that event also has to support that flag. It would make sense that v4l2_src_change_event_subscribe()
is extended to support that flag.

The bottom line is that you never have to touch filehandles or keep track of them.

Are you perhaps trying to receive events from a sub-device in a platform driver?
If that's the case, then let me know since that is not supported and it should
really be improved (I have some ideas about that). The only communication between
a subdev and the bridge driver is via the notify callback in v4l2_device.

Regards,

	Hans