[08:07] <hverkuil> ezequielg: if you can post a v6 of the "media: hantro: Add support for H264 decoding" series, then I think it is ready to be merged. Ideally I can merge it on Friday together with part of my "Stateful/stateless codec core support" series.
[08:09] <hverkuil> paulk-leonov: regarding your "HEVC/H.265 stateless support for V4L2 and Cedrus" series:
[08:11] <hverkuil> I think this needs support for HEVC encoding mode and start code controls as well?
[08:12] <hverkuil> When ezequielg posts his v6 it is probably a good idea to rebase your series on top of that and add these controls for hevc.
[08:13] <hverkuil> It would be nice if we can merge the hevc code next week.
[12:20] <ezequielg> hverkuil: alright, will do.
[12:26] <ezequielg> hverkuil: i originally had START_CODE, but then saw usage of _STARTCODE for other controls, and decided to be consistent.
[13:20] <hverkuil> pH5: mem2mem driver looks good. If there are no other comments, then I plan to merge this next week.
[13:29] <jmondi> sailus: pinchartl: ping
[13:43] <sailus> jmondi: Pong.
[13:47] <pinchartl> sailus: Jacopo left
[13:47] <pinchartl> he wanted to know your opinion on a sensor orientation/location control
[13:47] <pinchartl> in particular, how you would like the control to be named
[13:47] <pinchartl> what control type would be best
[13:47] <pinchartl> and what values should be defined (and how to define them)
[13:48] <pinchartl> I'm thinking of a menu control
[13:48] <pinchartl> with "back" and "front" for now
[13:48] <pinchartl> more can be added later (top, bottom, left, right, ...)
[13:48] <pinchartl> I would also define those relatively to the nominal orientation of the device
[13:48] <pinchartl> and for each device type, define what the nominal orientation is
[13:48] <pinchartl> (e.g. for phones, front == user-facing)
[13:49] <pinchartl> any opinion ?
[13:49] <hverkuil> I thought we wanted to use MC properties for that?
[13:49] <pinchartl> hverkuil: I was wondering about that too
[13:50] <pinchartl> we have read-only controls for similar information
[13:50] <pinchartl> e.g. the sensor rotation
[13:50] <hverkuil> and such info should typically come from the device tree.
[13:50] <pinchartl> so I wonder where we should draw the boundary
[13:50] <pinchartl> yes, that should definitely come from DT
[13:50] <pinchartl> (or some other firmware)
[13:50] <pinchartl> Jacopo will propose a new standard DT property for this
[13:52] <hverkuil> I do have patches adding MC properties, but nobody seemed to be interested in this, so I stopped working on it.
[13:52] <pinchartl> I have an interest, but I didn't like your approach, and don't have time to write a different one :-)
[13:53] <pinchartl> so I'm wondering if a read-only control would make sense, as we already have a few similar controls
[13:53] <pinchartl> sailus: what do you think ?
[13:53] <hverkuil> I don't think there is anything wrong as such with implementing RO controls.
[13:55] <pinchartl> and regardless of whether it's a control or a property, we need to define its semantic
[13:55] <pinchartl> does the above sound sane to you ? (menu, "front" & "back", defined relatively to the device)
[13:57] <hverkuil> what if there are multiple back sensors? E.g. the Samsung Note 10+ has three back sensors ('regular', telelens, wide angle).
[13:57] <sailus> pinchartl: This is a static value.
[13:57] <hverkuil> 'back' is a bit too general, I think.
[13:57] <sailus> How about a string control?
[13:57] <pinchartl> I think we'll need another control/property to tell them apart
[13:58] <pinchartl> they all face back, but have different lens types for instance
[13:58] <hverkuil> That would work.
[13:58] <pinchartl> I don't think we should combine lens type and location in a single control
[13:58] <pinchartl> otherwise we'll have too many combintions
[13:58] <hverkuil> In any case, this needs some thought, so an RFC would be useful.
[13:58] <pinchartl> sailus: a string control would work too, but why would that be better than a menu control (which contains strings) ?
[13:59] <pinchartl> hverkuil: sure. Jacopo wanted to have initial feedback here first, to minimise the number of iterations
[13:59] <hverkuil> A string control won't work: applications can't reliably test against it.
[13:59] <hverkuil> menu control seems appropriate here.
[14:01] <sailus> Hmm.
[14:01] <sailus> That would limit the number of possible locations.
[14:02] <pinchartl> sailus: you can always add new menu entries to the spec
[14:02] <sailus> Until you reach the limit, that is.
[14:03] <sailus> Ah, menu.
[14:03] <sailus> Indeed.
[14:03] <sailus> But why an application would need to enumerate the entire menu to know the location?
[14:03] <pinchartl> do you want for than 4G possible values ? :-)
[14:03] <sailus> A string is much more simple in this case.
[14:03] <pinchartl> it wouldn't need to
[14:03] <sailus> I don'
[14:04] <sailus> t think we support more than 64 entries currently.
[14:04] <sailus> That's where the mask ends...
[14:04] <pinchartl> right
[14:04] <pinchartl> but now I get your point
[14:04] <pinchartl> a string is better than a menu as it's static data
[14:05] <pinchartl> or we could make it an integer
[14:05] <pinchartl> with #define's
[14:05] <pinchartl> the latter would be slightly more efficient, and would prevent drivers from adding new undocumented values
[14:05] <hverkuil> ^^^ I was thinking the same thing.
[14:06] <sailus> How about adding the DT bindings at the same time?
[14:06] <sailus> Isn't there something in this area already btw.?
[14:07] <pinchartl> yes, DT bindings would be added at the same time
[14:07] <pinchartl> sailus: is there ?
[14:07] <sailus> Perhaps not.
[14:07] <sailus> ACPI does. :-D
[14:08] <hverkuil> sailus: do you have a link to the acpi definition of that? It makes sense to follow that if possible.
[14:08] <hverkuil> (leaving, back later)
[14:08] <pinchartl> sailus: does it ?
[14:08] <sailus> Yes!
[14:08] <sailus> A moment.
[14:09] <sailus> _PLD
[14:10] <sailus> https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
[14:11] <pinchartl> there's also https://jmondi.org/android_metadata_tags/docs.html#static_android.lens.facing
[14:11] <sailus> It's somewhat desktop / laptop centric.
[14:11] <sailus> The other seems to assume a phone / a tablet.
[14:12] <sailus> It should probably be defined in conjunction of some kind of a form factor type.
[14:12] <pinchartl> we could also use pose rotation and translation
[14:12] <pinchartl> (actually we'll have to at some point)
[14:12] <sailus> The rotation property already exists.
[14:12] <pinchartl> no, it's a 3D rotation
[14:12] <sailus> Ah.
[14:12] <sailus> Some combinations could be somewhat ambiguous.
[14:13] <sailus> Such as how do you describe the rotation of a camera in the bottom of a laptop?
[14:14] <pinchartl> how do you mean ?
[14:15] <sailus> Well... what would you expect of the orientation of a camera in the bottom of a laptop?
[14:15] <sailus> Facing down, that is.
[14:15] <pinchartl> ah
[14:15] <sailus> It's technically entirely possible.
[14:15] <pinchartl> I don't think that would be very useful indeed :-)
[14:15] <sailus> I didn't ask that. :-)
[14:16] <sailus> I'm sure somewhere, somewhere, has that.
[14:16] <sailus> Someone
[14:17] <pinchartl> I really hope you're wrong :-)
[14:17] <pinchartl> but in any case that would need to be defined in the spec, if we ever have to support it
[14:18] <sailus> Hopefully not.
[14:19] <pinchartl> _PLD seems to be meant for connectors
[14:19] <pinchartl> have you seen it used for cameras ?
[14:19] <pinchartl> most of the fields would make no sense
[14:22] <sailus> Yeah, this comes from connectors as those are what PCs have typically had, not cameras.
[14:23] <sailus> It's a binary data structure. I don't know whether it's been used for cameras.
[14:24] <sailus> Actually I can. It is.
[14:26] <pinchartl> what fields are populated in that case ?
[14:29] <sailus> Panel is used.
[14:29] <sailus> At least .
[14:30] <pinchartl> where is it used ? do you have a public example ?
[14:30] <sailus> A phone call.
[14:31] <pinchartl> it's used in a phone call ?
[14:32] <sailus> I'd expect the Windows ACPI tables for IPU3 have that.
[14:32] <sailus> So I'm back.
[14:33] <pinchartl> yes, I see something along those lines
[14:33] <pinchartl> http://paste.debian.net/1095759/
[14:34] <pinchartl>                     PLD_Ejectable          = 0x1,
[14:34] <pinchartl> for a camera, that seems weird
[14:34] <pinchartl>                     PLD_VerticalPosition   = "CENTER",
[14:34] <pinchartl>                     PLD_HorizontalPosition = "RIGHT"
[14:34] <pinchartl> that seems weird too
[14:35] <pinchartl> interestingly enough the PMIC also has a PLD...
[14:36] <sailus> It's important to know its location, isn't it?
[14:36] <pinchartl> one more proof that you can never trust the ACPI tables
[14:37] <pinchartl> at least on chromebooks I can send a patch for the ACPI tables, and it will get released in firmware upgrades :-)
[14:37] <sailus> Where did you see that?
[14:37] <pinchartl> I wish Dell would publish the ACPI sources
[14:37] <pinchartl> Dell Latitude 7285
[14:37] <pinchartl> one of the models using the IPU3
[14:38] <sailus> At least it didn't say the camera was located in the bottom, did it?
[14:38] <pinchartl> I suppose that's a small victory :-)
[14:39] <sailus> For there is a value for devices located in the bottom, too.
[14:41] <hverkuil> back
[14:41] <hverkuil> FYI: downward facing cameras exist, they are typically known as 'document cameras' since that's what they are usually used for.
[14:42] <hverkuil> although not in the bottom of a laptop :-)
[14:43] <hverkuil> They are usually cameras that you can manually move from a forward facing position to a downward facing position.
[14:44] <pinchartl> hverkuil: yes, but not on laptops :-)
[16:10] <jmondi> sailus: hverkuil: thanks for the feedback, I had to drop off a few minutes after my ping
[16:10] <jmondi> pinchartl: thanks Laurent for explaning the issue :)
[16:11] <jmondi> a few points: 1) I went for an integer control with defines, and I was undecided between that and an integer menu control
[16:12] <jmondi> setting up menu controls is a bit more work for drivers, and in my mind mind the possible values for the control are fixed, so maybe a series of defines works better and it's simpler
[16:12] <jmondi> the control will be a read-only one, and it's value comes from DT/ACPI
[16:13] <jmondi> I plan to add standard DT property for that
[16:15] <jmondi> in regard to the property semantic, well, I understand it depends on the device nature.. for a development board without an LCD "front" and "back" are not meaningful as they are for a phone, let's say
[16:21] <jmondi> I think we could just define a small set of mounting location such as "front", "back", "up", "down" (or maybe "frontward" "backward" "upward" and "downward" if we want th property to describe the camera orientation more than the mounting location) whose precise semantic depends on the device form-factor for which the property is specified...
[16:21] <jmondi> I'll send an RFC with all of this so we can move the discussion to the list maybe
[16:25] <rob_gries> Does anyone have a moment to spare to talk about a weird thing I'm seeing in the qualcomm venus h264 encoder?
[16:51] <koike> Hi mani_s, when testing the isp driver with Rock960, did you tested with the patches from the list? Or from my v9 branch? I'm asking because I saw this error in my v9 branch and I already fixed it
[16:54] <koike> I didn't see anything obivious from the dts you sent
[17:22] <koike> Hi sailus, you mention in your review, in the set_fmt subdev function, that for sub-device nodes, the driver is itself responsible for serializing. But looking at subdev_do_ioctl_lock(), it seems that it serializes the ioctl calls for subdevs, no?
[17:28] <mani_s> koike, I just used your v9 branch
[17:29] <mani_s> when did you fix it? Should I just rebase?
[17:34] <koike> mani_s: I fixed yesterday afternoon. Yes, please, rebase
[18:10] <koike> mani_s: I'm sure commit f07832aadd4b026fdb97fa0d19ac844bc75ef013 is working, I'm still doing some prof of concepts and some tests (and I'm commiting there because it is wip and to save the work in case my computer dies), so the tip of the branch might be broken
[18:11] <mani_s> koike, okay. what tree/branch did you use for your v9 branch?
[18:12] <mani_s> I tried applying the patches to media 5.3-rcx branch and it resulted in conflicts. hence I just used your branch directly
[18:12] <mani_s> koike, may be I should rephrase. what is the base branch for your v8 patches?
[18:21] <koike> mani_s: I based on media/master (but I need to update it), current is 5.3-rc2, but let me re-base it
[18:48] <koike> mani_s: I just updated the branch on top of media/master (I didn't tested yet because I just messed up with my rootfs for tests by accident, anyway), but it should be working
[18:49] <mani_s> koike, thanks. will give it a shot!
[21:35] <pinchartl> I think jmondi: I'd describe the location and not the orientation
[21:35] <pinchartl> so front/back