| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
| Who | What | When |
|---|---|---|
| hverkuil1 | mchehab: what are the plans for a media summit in Europe? The Monday before the ELCE would probably be best for most people.
I'd love to have a session about the Codec API/Request API since this pops up more and more often, and we'll need to address this. | [08:41] |
| .... (idle for 16mn) | ||
| pinchartl | hverkuil1: Sakari and I will be there, so even if there's no mini-summit as such, we can work together on the request API | [08:58] |
| hverkuil1 | That would be good. I'm in.
tfiga: do you attend the ELCE? Are you still based on Poland? I think your input w.r.t. Codec/Request API would be very useful. I haven't seen much of posciak lately :-( | [09:00] |
| pinchartl | I think Tomasz is based in Japan now
so is Pawel ndufresne might be able to join us, although I think he will be busy with the gstreamer conference on Monday speaking of gstreamer anyway experienced in debugging gstreamer pipelines ? I get a very unwelcoming error ERROR: from element /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: Internal data flow error. Additional debug info: gstbasesrc.c(2948): gst_base_src_loop (): /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: streaming task paused, reason not-negotiated (-4) and I'm trying to figure out where it comes from (that's at stream start) | [09:09] |
| hverkuil1 | could this be a problem with colorspace negotiation? (FDP1 driver?) | [09:13] |
| pinchartl | possibly :-)
that's what I'm trying to figure out here's the pipeline (give me a second) | [09:14] |
| hverkuil1 | From some of the gstreamer conversations I've seen here gstreamer can be (overly?) picky about that. | [09:19] |
| pinchartl | https://paste.kde.org/po0esigee
I can get the full debugging output from gstreamer, but that's lots of information and nothing struck me as particularly wrong | [09:21] |
| hverkuil1 | pinchartl: sorry, I can't help any further. Other than the comments I read here on irc I have no gstreamer experience. | [09:30] |
| pinchartl | no worries | [09:31] |
| ................ (idle for 1h15mn) | ||
| sailus | hverkuil1: Heippa! | [10:46] |
| hverkuil1 | sailus: Goedemiddag! | [10:47] |
| sailus | I think we wanted to discuss the selection flags today.
I btw. sent an update to the bayer format patches. They've been converted to ReST now, with some other improvements. | [10:51] |
| hverkuil1 | Nice. Thanks for the patches.
Regarding selection: would 30 minutes from now work for you? | [10:56] |
| sailus | Sure.
I actually have a meeting right now. Not that it requires my full attention anyway... | [10:59] |
| hverkuil1 | sailus: suggest a time. I need about 30 minutes to finish something, and then I have time to discuss it until 5 pm (4 hours from now). | [11:01] |
| sailus | My meeting only takes 15 minutes (roughly). So in half an hour works well. | [11:02] |
| ...... (idle for 26mn) | ||
| hverkuil1 | Sorry, I need a bit more time. I'll ping you. | [11:28] |
| sailus | Ack. | [11:29] |
| ....... (idle for 33mn) | ||
| hverkuil1 | sailus: ping | [12:02] |
| sailus | hverkuil1: Pong. | [12:11] |
| javier__ | pinchartl: instead of --gst-debug-no-color, I usually use the GST_DEBUG=*:5 GST_DEBUG_NO_COLOR=1 env vars that gives more control over what's output | [12:12] |
| hverkuil1 | sailus: OK, so the question is what should VIDIOC_S_SELECTION do when the _LE and/or _GE flags are set and those constraints cannot be satisfied. | [12:12] |
| javier__ | pinchartl: i.e: GST_DEBUG=*videotestsrc*:5 or GST_DEBUG=*CAPS*:5 in your case
since I've seen that internal data flow error due gst failing to do the caps negotiation | [12:12] |
| sailus | hverkuil1: Yes. | [12:13] |
| hverkuil1 | Currently my interpretation of the spec (and that of the current driver implementations) is that it should return -ERANGE.
(Note: I'm only talking about VIDIOC_S_SELECTION, not VIDIOC_SUBDEV_S_SELECTION) | [12:13] |
| sailus | In pipeline configuration, providing something that works is a lot better than giving an error without any suggestion what's the problem.
In respect of rectangle position / size, I mean. I'd be inclined to think it's the same for video nodes: hardware limitations can be hard to figure out otherwise. | [12:13] |
| hverkuil1 | And as I understand the spec, SUBDEV_S_SELECTION doesn't have to return ERANGE. I don't think it is mentioned there. | [12:15] |
| pinchartl | javier__: I use GST°DEBUG too :-) | [12:15] |
| sailus | Your "intepretation" is supported by the VIDIOC_[GS]_SELECTION in the spec. | [12:15] |
| javier__ | pinchartl: Ok, I mentioned just in case :) | [12:15] |
| sailus | Elsewhere the spec says that the rectangle is adjusted.
Both in common selection flag documentation and sub-device selection documentation. This is an internal conflict in the spec. | [12:15] |
| pinchartl | javier__: thank you, it's appreciated. I narrowed it down to v4l2video0convert0 (my deinterlacer) returning interlaced caps on its source pad, which is quite obviously wrong. I don't know yet why that happens though | [12:16] |
| sailus | At least what comes to common flag documentation and VIDIOC_[GS]_SELECTION documentation. | [12:16] |
| hverkuil1 | Well, you would typically keep flags 0 if you just want the best fit. You use LE/GR if you need the rectangle to stay inside some other rectangle. | [12:16] |
| sailus | The behaviour of finding a best fit is different from finding a rectangle which is larger or smaller than the given one.
If you believe that the functionality supporting the error behaviour is really valuable, I think we should add flags to support both. Or maintain the different behaviour on video nodes and sub-devices. | [12:17] |
| hverkuil1 | Certainly, but in an application you can retry with flags = 0 if e.g. flags = _LE gave an ERANGE error. | [12:18] |
| javier__ | pinchartl: I see, since you are setting the rest to progressive then it makes sense that gst is not able to do the caps negotiation | [12:18] |
| hverkuil1 | When trying to make a subdev pipeline work, then I can imagine that you want more leeway for drivers to adjust it. | [12:18] |
| sailus | hverkuil1: What's the use case for giving an error? | [12:19] |
| hverkuil1 | Although I still think it is dubious that you can specify _LE and end up with a larger than requested rectangle. | [12:19] |
| sailus | The same use case is there for video nodes: you just have a memory buffer instead of the data flowing on a bus.
It could well be used by another hardware device after that --- and often is. | [12:19] |
| hverkuil1 | I assume that the rectangle has to fit inside an already allocated frame buffer of some kind. If it doesn't fit, then you need to backtrack and e.g. reallocate such a buffer. | [12:20] |
| sailus | That's how VIDIOC_S_FMT works, too: it gives a closest matching size rather than an error.
The buffer allocation typically takes place after format negotiation. | [12:20] |
| hverkuil1 | If it just adjusts the rectangle, then I don't know that it suddenly won't fit my requirement anymore. | [12:21] |
| sailus | It's not necessarily a frame buffer. Post-processing by another hardware device or a JPEG or video encoder is common.
The user should check the rectangle, as with S_FMT. Albeit if the implementation has been like this for quite some time, I'm not sure if we could just change the behaviour on video nodes. The same applies to sub-devices. | [12:21] |
| hverkuil1 | The reality is that I don't see how we can change it (regardless of whether we want to). All vidioc_s_selection implementations that care about the flags do this already, and that's also what is in the documentation. | [12:23] |
| sailus | When I say "I'm not sure", I way what I mean. But the use case for adjusting it is definitely there. | [12:23] |
| pinchartl | if I may chime in for a minute, havgin _LE | _GE return an error depart significantly from the current model where format and selection ioctls never fail but return the closest match instead. I wonder if we really want to take that route | [12:23] |
| hverkuil1 | pinchartl: it's there already. That's the problem. | [12:24] |
| pinchartl | do userspace rely on that ?
I assume the answer is the usual "we can't know for sure" | [12:24] |
| sailus | One additional option would be to deprecate the existing flags and create new ones with well defined semantics while still supporting what was there. | [12:25] |
| hverkuil1 | How about adding a flag: V4L2_SEL_FLAG_ADJUST: if set, then if the constraint cannot be accomodated, the driver just ignores the flags and behaves as if flags was 0.
pinchartl: exactly. sailus: I'm not going there. | [12:25] |
| sailus | How should the flag behave on sub-devices? | [12:26] |
| pinchartl | hverkuil1: and pretending we're not aware we modify the behaviour isn't an option either I assume :-) | [12:26] |
| sailus | The user space expects the adjustment to happen even without the presence of the flag.
Should it could only apply to video nodes. ? I'm fine with that idea btw. | [12:26] |
| hverkuil1 | I'm OK with applying that only for video nodes.
https://hverkuil.home.xs4all.nl/spec/uapi/v4l/v4l2-selection-flags.html#v4l2-selection-flags | [12:27] |
| sailus | That'd extend the functionality on video nodes by making it possible to adjust the size / position. | [12:27] |
| hverkuil1 | That table already differentiates between v4l2 and v4l2 subdev. | [12:27] |
| sailus | Yes.
I agree, this looks like the way to go to me. Who shall write the patches? :-) | [12:27] |
| hverkuil1 | Theoretically there could be a flag V4L2_SEL_FLAG_DONT_ADJUST for v4l2 subdev. | [12:28] |
| pinchartl | hverkuil1: that table tells that the driver can return a size that doesn't honour the flag | [12:28] |
| sailus | hverkuil1: I wouldn't add that flag.
The user can already check this by just looking at the result. | [12:28] |
| hverkuil1 | pinchartl: yes, I know the spec is inconsistent. But the current driver implementations aren't. They all return ERANGE. | [12:29] |
| sailus | Which is precisely as it works on other V4L2 IOCTLs. | [12:29] |
| hverkuil1 | sailus: I wouldn't add it either, it was just a possible suggestions for the future. | [12:29] |
| pinchartl | vivid doesn't return -ERANGE as far as I can see | [12:29] |
| sailus | I wouldn't do that even in the future. :-) | [12:29] |
| hverkuil1 | pinchartl: it only does that if both _LE and _GE are set. | [12:30] |
| pinchartl | hverkuil1: yes, I've checked
and nobody has complained about vivid's behaviour as not working with their apps, right ? :-) | [12:30] |
| hverkuil1 | I think I was too lazy to handle the other combinations.
pinchartl: you never know what userspace do. | [12:31] |
| pinchartl | hverkuil1: that's right, but it works both ways
how can you be sure that returning -ERANGE from drivers that don't currently do it won't break userspace ? especially given that most drivers don't support the _LE|_GE flags | [12:32] |
| hverkuil1 | All do AFAIK. Well, except vivid :-) | [12:33] |
| pinchartl | when they get support for those flags, they will suddenly break
that's why the flags are hints they get ignored by drivers that don't implement them yet but when the feature gets implemented in a driver, it shouldn't start returning -ERANGE I'd also argue that applications already have to handle _GE|_LE not being honoured, as they have to deal with the majority of drivers that don't support those flags so they need to check the returned rectangle anyway | [12:33] |
| hverkuil1 | My main concern is applications for SoCs that check these flags that rely on ERANGE. If it would suddenly no longer return ERANGE, what would break? | [12:35] |
| sailus | What kind of an application relies on an error in its functionality? | [12:35] |
| pinchartl | I have no exact answer to that question, but way less than what would break if all drivers started returning -ERANGE | [12:35] |
| sailus | I can imagine the other way of being a problem but not getting an error is a really far-fetched one.
Applications generally do have special error handling code for such situations that end what was going on. s/end/ends/ | [12:36] |
| hverkuil1 | Let's see: am437x checks both LE and GE flags and return ERANGE.
Ditto exynos-gsc | [12:36] |
| pinchartl | exynos4-is
s5p-tv and sti/bdisp that's it | [12:37] |
| hverkuil1 | s5p-tv returns ERANGE, and so does sti/bdisp.
In other words: ALL drivers that implement this (except vivid) follow the spec. I.e. if the resulting rectangle doesn't comply to the constraint flags, then it returns -ERANGE. Conclusion: don't change this and fix vivid to match that behavior. | [12:39] |
| pinchartl | no
ALL drivers that don't implement it would start returning an error when they did not imagine the breakage that this will introduce it's much larger | [12:40] |
| hverkuil1 | I'm only looking at drivers that actually check the LE/GE flags, that's true.
So drivers that don't check for this are: ivtv, rcar-vin, s5p-jpeg, sh_vou, soc_camera, ti-vpe. I don't care about sh_vou and soc_camera. | [12:41] |
| pinchartl | is that all the drivers we have ? | [12:42] |
| hverkuil1 | All the drivers that implement vidioc_s_selection but do not look at the constraint flags. | [12:43] |
| pinchartl | don't forget the ones that implement the crop ioctls
they expose the selection API too, through the core compat code | [12:43] |
| hverkuil1 | No. If they don't implement s_selection, then the ioctl will return ENOTTY.
s_crop is implemented on top of s_selection, not the other way around. | [12:45] |
| pinchartl | ah, that's a good start
we've put ourselves in the unfortunate but real situation where different drivers exhibit different behaviours *if* we want to standardize on a single behaviour, there will be a risk of breakage so in my opinion there are two questions - is it worth it standardizing on a single behaviour and thus incurring the risk of breakage ? - if the answer to the previous question is yes, what's the behaviour that will lower the risk of breakage ? | [12:45] |
| hverkuil1 | 1) New drivers should follow the spec. | [12:49] |
| pinchartl | regarding that, we first need to clarify the spec | [12:50] |
| hverkuil1 | 2) For existing non-compliant drivers it is case-by-case. IMHO adding this to ivtv is safe (it's quite old now and nobody uses the selection API with this. I happily take the responsibility for that)
I would recommend it for rcar-vin, especially since it is a new driver. | [12:50] |
| pinchartl | I'd say that it's the exynos and am437x drivers that are not compliant :-) | [12:51] |
| hverkuil1 | I'm not sure about ti-vpe and s5p-jpeg. | [12:51] |
| pinchartl | rcar-vin is technically new, but the soc-camera based version has been there for a long time
so there's definitely lots of userspace code using that driver | [12:51] |
| hverkuil1 | do you know if cropping worked with the rcar-vin soc-camera driver? | [12:53] |
| pinchartl | I haven't checked that personally. Niklas might know | [12:53] |
| sailus | Applications typically work on multiple devices.
We shouldn't assume otherwise. So the behaviour should be unified. | [12:54] |
| hverkuil1 | There are 13 drivers (excluding vivid) that implement s_selection, of those 13 there are 6 that check the constraint flags and, if it doesn't fit, they return ERANGE. The remaining 7 never check the flags. | [12:56] |
| pinchartl | exynos4-is and s5p-tv received selection API support after the initial version was merged upstream
so chances are that product-specific userspace code for them still support the crop API that lowers a bit the probability that it would rely on the flags returning -ERANGE (for what it's worth, gstreamer doesn't use those flags) | [12:57] |
| hverkuil1 | What a mess. | [13:01] |
| pinchartl | (checking Android AOSP now)
no hit in Android either searching on google, the only relevant hit I've found is https://marc.info/?l=gstreamer-devel&m=142678042717771&w=2 and you can see that any -ERANGE error will just log a warning but be otherwise ignored | [13:01] |
| hverkuil1 | it's not really clear whether they rely on the resulting rectangle (i.e. if S_SELECTION returns 0) is LE from the requested rectangle. | [13:08] |
| sailus | My presumption is that these flags have very few users in general. | [13:09] |
| pinchartl | and that seems to be an unmerged patch
sailus: I wonder if they're used at all | [13:09] |
| hverkuil1 | I agree with that.
pinchartl: there is no way of knowing that. | [13:09] |
| sailus | On mobile devices, the user space is typically hardware dependent and the pipeline configuration is hardware specific. And it is programmed exactly as intended, i.e. there is no reliance on these flags. | [13:10] |
| hverkuil1 | I would think that if it is used with drivers that do not check the flags, then it is unlikely that someone would specify those flags. | [13:10] |
| pinchartl | hverkuil1: that's why I said that I wonder, not that I'm sure :-) | [13:10] |
| sailus | As we do not have generic and automatic pipeline configuration library as of yet. | [13:10] |
| hverkuil1 | And if the driver *does* support those flags, then custom apps might well rely on them. | [13:11] |
| sailus | I'd be extremely surprised if unifying the behaviour either way would break something.
Still, adjusting the rectangle has a fewer chances of breaking something as no error code is returned. | [13:11] |
| pinchartl | sailus: I agree with you | [13:12] |
| hverkuil1 | Still, returning an error has fewer chances of breaking something then it is likely that those constraint flags are only used with drivers that support them. | [13:12] |
| sailus | hverkuil1: Yes, *might*. But custom applications tend to do what's necessary and no more than that. And using these flags is not necessary in any common use case for a custom application (that already uses the exact size and does not rely on it being adjusted). | [13:13] |
| hverkuil1 | PoV
IF an application sets these flags, then I assume it has been tested against a driver that actually honors them. | [13:13] |
| pinchartl | how about asking Samsung, as we're talking about three of their drivers ? | [13:14] |
| hverkuil1 | There normally is no need to set these flags.
mszyprow_: ping! | [13:14] |
| pinchartl | and Pawel/Tomasz, to see if they have used those flags in ChromeOS ?
posciak: ping tfiga: ping | [13:15] |
| mszyprow_ | hverkuil1: pon
hverkuil1: pong | [13:15] |
| pinchartl | wow, that was fast :-) | [13:15] |
| mszyprow_ | :) | [13:15] |
| hverkuil1 | mszyprow_: did you follow this discussion? (Probably not) | [13:15] |
| pinchartl | quick summary: we're having an issue with the V4L2_SEL_FLAG_LE and V4L2_SEL_FLAG_GE selection flags | [13:15] |
| sailus | I need to leave at any moment. I'll check the log later on. | [13:16] |
| mszyprow_ | not yet | [13:16] |
| pinchartl | the V4L2 spec is ambiguous on their subject
it's not clear whether a driver should return -ERANGE if the flags can't be honoured, or if it should return an adjusted rectangle that doesn't honour the flags 5 drivers currently return -ERANGE, including 3 Samsung drivers | [13:16] |
| sailus | ^On video devices, that is. | [13:17] |
| pinchartl | (exyons4-is, s5p-tv, exynos-gsc)
yes, on video devices, not on subdevs on subdevs the spec is clear, -ERANGE is not an option, the ioctl must succeed with an adjusted rectangle so the question is, if we want to standardize the behaviour for all drivers, which ones should we modify ? regardless of the path we chose, there's a theoretical risk of breakage and we were thus wondering if you're aware of userspace code, possibly Samsung-specific, that would use those flags | [13:17] |
| mszyprow_ | mszyprow_ asked snawrocki for help ;)
you can definitely ignore s5p-tv driver, it is no longer used by us | [13:19] |
| pinchartl | snawrocki: https://paste.kde.org/phgivoz4s | [13:20] |
| hverkuil1 | mszyprow_: should s5p-tv perhaps be removed? (I love removing drivers!) | [13:20] |
| snawrocki | pinchartl: thanks | [13:21] |
| pinchartl | hverkuil1: I'd say that as long as it doesn't impede development we can keep it
snawrocki: you're welcome | [13:21] |
| mszyprow_ | hverkuil1: imho yes, there is no point keeping dead code
hverkuil1: although the driver is used by some obscure old vendor code (like 3.10 reference kernel for odroids) | [13:21] |
| sailus | Are there no other users for that driver? :-) | [13:22] |
| hverkuil1 | Ha! Nobody sets VIDEO_SAMSUNG_S5P_TV anymore, it's not even in the Kconfig. | [13:23] |
| mszyprow_ | s5p-gsc can be also ignored as most userspace use now the drm version of that driver
the only important is exynos4-is and Sylwester is the one who should be asked about it | [13:24] |
| hverkuil1 | posted patch removing s5p-tv :-) | [13:27] |
| snawrocki | I would only worry about any use of those V4L2_SEL_FLAG_{LE,GE} flags in the upstream gstreamer code | [13:27] |
| pinchartl | upstream gstreamer doesn't use them | [13:28] |
| hverkuil1 | never mind, I made a mistake. | [13:28] |
| snawrocki | ok, let me check one more place
no, I can't find any user of these flags here as well | [13:29] |
| hverkuil1 | mszyprow_: what replaced s5p-tv?
drm driver? | [13:31] |
| mszyprow_ | hverkuil1: yes, exynos drm | [13:31] |
| snawrocki | so the plan is to modify drivers to not return ERANGE and just adjust adjust passed rectangle as it is done for subdevs? | [13:36] |
| hverkuil1 | It looks that way. | [13:36] |
| pinchartl | snawrocki: that's at least what Sakari and I have been advocating
any opinion ? | [13:37] |
| snawrocki | I'm just wondering what driver should do in situation when it is not possible to adjust the rectangle according to the passed flags from user space | [13:40] |
| pinchartl | it should then return the closest possible rectangle, as if the flag wasn't specified
that's what subdevs do the rationale, from my point of view, is that drivers don't have to implement the LE/GE flags, so applications already have to cope with the flags not being honoured and need to check the returned rectangle anyway the flags are thus a hint | [13:41] |
| hverkuil1 | This change would also mean that specifying both LE and GE flags is pointless and would be equal to 0. | [13:42] |
| pinchartl | correct | [13:42] |
| snawrocki | ok, I see now the flags specification is quite liberal, so the flags are treated as hints
pinchartl: I can't disagree, it sounds reasonable what you're saying | [13:42] |
| hverkuil1 | OK, let's do this. But I hate it that this crops up years after the API was designed :-( | [13:45] |
| pinchartl | hverkuil1: I agree :-/
I think this really shows that our APIs are undertested at design time that's something we should fix | [13:46] |
| hverkuil1 | I think I am going to insist that new APIs must have v4l2-compliance tests to accompany them.
It's in my experience the best method of ensuring that you covered all corner cases. | [13:46] |
| pinchartl | that would be a good start | [13:47] |
| hverkuil1 | (or hopefully covered them all!) | [13:47] |
| pinchartl | but I'm wondering whether we shouldn't at some point also require that new APIs get a real userspace implementation
test tools are nice, but they don't put the API in use in real world use cases | [13:47] |
| hverkuil1 | Adding it to v4l2-ctl, v4l2-compliance and qv4l2 (if possible) would go a long way. But requiring 'real world' use cases is, I think, very hard to enforce. | [13:48] |
| pinchartl | it is
because we have no such thing as a standard userspace DRM/KMS enforces that requirement | [13:49] |
| hverkuil1 | How do they do that? | [13:49] |
| pinchartl | they require new APIs to be implemented in either Weston, X11 or Android hwcomposer
and the patches to be acked by the upstream maintainers of those projects | [13:49] |
| hverkuil1 | Ah, makes sense there. Yeah, we don't have that luxury. | [13:50] |
| pinchartl | (not applied of course, as you can't apply a patch before the kernel API gets merged)
it's quite a harsh requirement but maybe we should think about what we could do in that direction | [13:50] |
| hverkuil1 | Anyway, I'll see what I can do about creating patches for this issue. Not sure when I'll have time. | [13:51] |
| pinchartl | thank you
I'll review them :-) now I'll go back to gstreamer debugging... | [13:52] |
| *** | groleo has quit IRC (Remote host closed the connection) | [13:55] |
| pinchartl | pinchartl wonders how Kieran managed to test the deinterlacer with gstreamer
the answer is simple with exactly the same change as the one I've just made https://github.com/kbingham/gst-plugins-good/commit/e28b6aeae384497de57be77b51dcea7684fa3c6f *sigh* :-) | [13:58] |
| ........ (idle for 38mn) | ||
| *** | hverkuil1 has left | [14:39] |
| ......... (idle for 43mn) | ||
| svarbanov has left | [15:22] | |
| ........................................ (idle for 3h18mn) | ||
| gsivori | hi everyone. is anyone experienced compiling the driver for an easycap device? :) | [18:40] |
| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |