That preference is intentional. I hope that you won't be surprised that this patch will be rejected.

1) The pro is simplicity. If you provide patch that way, please don't limit this to v4l2src. We have helper to add properties everywhere it make sense.

2) GstVideoFormat is not a macro, but a C enumeration. What you propose just don't fit. It would also not work with anything that exists.

3) These are called fields. I think such field would only be understood by V4L elements, which we normally try to avoid.

Few questions, are you going to work on this ? Can you explain better how your two drivers ended up in this situation ? I would have expected this to never be an issue for IP from the same SoC

Review of attachment 363457 [details] [review]:

Justification provided is specific to a single SoC and does not represent a solution.

Hi Nicolas,

Thanks for the update.
If we consider the scenario when one IP within SoC supports both (contiguous)NV12 and NV12M (non-contiguous) and another IP only supports one of them, in such cases any user level option to select particular format would help select the common format.

I am trying to bring up below usecase and hitting this issue :

CAPTURE Device (v4l2src) -> VCU Encode (omxh264enc) -> Filesink
CAPTURE Device (v4l2src) -> VCU Encode (omxh264enc) -> Decode->Display

Now the IP used in Capture device supports both V42_PIX_FMT_NV12 and V4L2_PIX_FMT_NV12M but VCU supports only V4L2_PIX_FMT_NV12,
so we have to someway tell v4l2src to select V4L2_PIX_FMT_NV12 instead.

In your opinion what's the best possible way to handle this scenario?

Kindly let me know if any queries.

Thanks,
Devarsh

I don't know any good solution. In GStreamer, NV12 is defined the way NV12M is defined in V4L2.

In general, from what I understood, the reasons one would implement NV12M is to allow more flexible importation of buffers. Can't you import the buffers from your encoder into you capture driver ? (See io-mode property).

The second reason I know, and the reason it exists, is that with multiple allocation you can, on some platform, optimize the memory bandwidth. In such case, the drivers only support NV12M and the encoder would support that too. Those two formats are not really different format, they reflect an API limitation in V4L that was hack away by duplication.

Another way would be to design some method so this can be negotiated. Does not fit much in GST caps, it would be importing a hack. Maybe in the allocation query, or inside the pool configuration ? It's all going to be complicated and difficult to support. Changing the drivers so they implement a common denominator seems much easier. If you look at DRM side, they also only expose the equivalent of NV12M for dmabuf importation, and there is no way to know if there is an internal driver limitations.

Are you going to work on some patches ? Seems pretty difficult and hardware specific, so it's unlikely anyone will do on it's spare time.

Thanks for writing.

"Changing the drivers so they implement a common denominator seems much easier"
We checked with driver owners and they don't see any memory bandwidth optimization on shifting to NV12. Also they see implementing NV12M in driver stack as being more complex.
Moreover, I guess the drivers here already have single denominator i.e V4L2_PIX_FMT_NV12 but it's just that v4l2src device supports both V4L2_PIX_FMT_NV12 and V4L2_PIX_FMT_NV12M and has a preference for the latter and no window for user to change it.

Currently we are making it work by forcing NV12 locally but if we plan to upstream then we might need to implement it in more generic way.

I understand you want a generic solution in which using some kind of negotiation between v4l2src and encoder the v4l2src shifts to V4L2_PIX_FMT_NV12 if the encoder supports only that but as of now I don't know what's the best way to do this in gstreamer, so currently not working on any patch.


Thanks,
Devarsh

I'll close this issue then as that sound it may happen in a year, or in 10 years from now. Please re-open (or create a new one) whenever you would like to do any upstream work on this.

Just to add more rationale to closing this bug. A generic solution would complexify the userspace, it's already complex. Adding property would confused users, users are already confused by io-mode and other properties.

The other aspect is that your source is not forced to do two allocation in NV12M. It could be a single allocation. The planar interface purpose is to handle two allocation when strictly required, and to expose memory offsets. Of course from user space it will look like two allocation, but you are exchanging this between two HW that can check the physical continuity of these two user address.

Finally, you didn't share any failure traces or any step to reproduce. Could this be reproduced with some test driver, like vivid, vim2m or vimc ? There is currently no way to validate. Even with access to your HW I would not know how to test this.

Thanks for the update and sorry for the long delay, please see response inline with tag {Devarsh}:

"I'll close this issue then as that sound it may happen in a year, or in 10 years from now. Please re-open (or create a new one) whenever you would like to do any upstream work on this."
{Devarsh} Sure,  am looking into this and will share observations within a month or two on what can be done to handle this. Yes I am a beginner and pretty new to this and working on some other things as well but still it should not take 1 (or 10 years). Anyways thanks for the motivation.

Finally, you didn't share any failure traces or any step to reproduce. Could this be reproduced with some test driver, like vivid, vim2m or vimc ? There is currently no way to validate. Even with access to your HW I would not know how to test this.
{Devarsh} AFAIK video test devices like vivid support only the multiplanar format and may require to update them to support both i.e. NV12 and NV12M. I am able to reproduce with my hardware so do you want me to send a build and steps to reproduce?

You can send me instructions through emails, be sure to target Zynq 106 rev B (the older one), specially if HDMI capture is involved.

I wonder if we could add fake meta that the encoder returns in the allocation query to instruct the source to switch to a single allocation?

(In reply to Olivier Crête from comment #11)
> I wonder if we could add fake meta that the encoder returns in the
> allocation query to instruct the source to switch to a single allocation?

Can you elaborate ?

For more context, the conflict is mainly Exynos vs Zynq.

  v4l2src ! v4l2h264enc output-io-mode=dmabuf-import ! ...
  v4l2src ! omxh264enc ! ...

I have to admit, the first pipeline don't work anymore on Exynos, due to driver issues, though to work it requires two allocation. While for the Zynq case, to work we need 1 allocation.

If we flip the allocation the other way around, we loose the ability to multiplex using tee. Though, DMABuf exportation is not implemented by the Zynq OMX component. All this is pretty much a dead-end without some sort of negotiation or trial and error.

My only idea here is to add something to the allocation query. For example, we could maybe add a "dummy" meta with a same like "GstPreferSingleAllocation" intructing the source to prefer a single allocation... Or something like that?

I'm trying to implement dmabuf export on Zynq gst-omx encoder input and I'm hitting this problem as well.

The proper fix would be negotiate the memory layout but there are two issues to sort out:

1) We don't really have any proper API for downstream to communicate its memory format preference. Olivier's suggestion could work, something like this in upstream's propose_allocation function:

  params = gst_structure_new ("memory-layout-meta",
      "prefer-single-allocation", G_TYPE_BOOLEAN, TRUE);
  gst_query_add_allocation_meta (query, GST_MEMORY_LAYOUT_META_API_TYPE, params);

Ideally gst_query_add_allocation_param() could be a better fit but this API isn't extendable. :(

2) v4l2src currently configure the format when gst_v4l2src_set_caps() is called. So if we need extra information from the allocation query to pick the right format we'll have to move this logic gst_v4l2src_decide_allocation(). Would that be a problem?

You can add parameters to the VideoMeta, noneed for a new meta.

b) dmabuf import in V4L GST code is incomplete, so that should be done first. We basically need to try format in setcaps and s_fmt near allocation.

-- 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/414.