<ndufresne>
S_FMT should chain to internal of TRY_FMT
<ndufresne>
so the FMT is always valid
<ndufresne>
buffer size is "bytesused" not MIN_BUFFERS ...
<javier__>
ah, Ok
<javier__>
ndufresne: I see that the S_FMT handler calls to vidioc_try_fmt(file, priv, f);
<javier__>
that's what you meant right?
<ndufresne>
yes, which should ensure that all value are valid to start with
<ndufresne>
not sure what is the rest of this function
<ndufresne>
let me read the original code
<javier__>
ndufresne: Ok
<javier__>
hmm, there are many things set to 0 there... but the struct v4l2_format *f passed is not touched after the call to vidioc_try_fmt()
<ndufresne>
looks like bugs to me ;-P
<javier__>
pix_mp->{height,width} and pix_mp->plane_fmt[0].bytesperline for OUTPUT
<ndufresne>
well, except for the encoded side
<ndufresne>
it's bytestream, so you don't need to set width/height/stirde
<javier__>
ah, Ok
<ndufresne>
but you should suggest a buffer size, but in our case, it's the CAPTURE, which is RAW frames
<ndufresne>
so all width/height/size need to be set, along with buffer/plane size (bytesused) (it's all mplane right ?)
* ndufresne
quickly hacked away the colorspace in FIMC, and the scaler/colorconverter works nicely
<ndufresne>
I wonder if I should probably implement the colorimetry, or just keep ignoring it properly ...
<ndufresne>
(Hans would certainly favour option 1)
<ndufresne>
Do we have colorimetry information given back by MFC ?
<javier__>
ndufresne: I've no idea tbh
<ndufresne>
javier__: a thing I'd like to see done, is a port of MFC driver to v4l2_m2m framework, like the IMX/Coda one, it would reduce a lot of code
<ndufresne>
javier__: side question, does the XU4 have USB 3 ?
<javier__>
ndufresne: you mean using struct v4l2_m2m_ctx and such rigth? yes, I think that would be nice indeed
<javier__>
ndufresne: yes, it has 2 USB 3.0 and 1 USB 2.0 port
<ndufresne>
a nice, when I switch to that boards, I'll probably set it up with a SSD connected on that port (for the rootfs)
<javier__>
ndufresne: so my problem is that there is information missing in the struct s5p_mfc_fmt formats[] array or that {S,TRY}_FMT handlers are missing to fill some fields for struct v4l2_format *f ?
<ndufresne>
something like that
<ndufresne>
javier__: seriously, checking capture/output is stupid, just make two functions, see the ops
<ndufresne>
ok, so formats array has nothing in it that relates to v4l2, so there must be somethign else to fill the v4l2 side
<javier__>
ndufresne: well, this will change once we move to v4l2_m2m since you can do vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type);
<ndufresne>
those try/s_fmt are just doing validation, they don't actually set the format
<javier__>
ndufresne: yeah, I notice that are just looking if the format exist in that array
<ndufresne>
it's in g_fmt that they actually do something
<ndufresne>
I suspect the framework will call that
<ndufresne>
to "initialize" things
<ndufresne>
hmm, aparently not, how does that work ..
<javier__>
ndufresne: so, how does it work when the capture format is not negotiated? (i.e: with the current -EBUSY in S_FMT)
<ndufresne>
you mean in gst?
<javier__>
yes
<ndufresne>
ok, so basically it setup the OUTPUT, and start streaming on OUTPUT (pass the header)
<ndufresne>
Right now gst expect the header stuff to be synchronous (not sure we'll keep that though)
<ndufresne>
when that is send (queued), we call G_FMT
<ndufresne>
You'll see s5p_mfc_wait_for_done_ctx() in there
<ndufresne>
this is to wait for the header to be processed
<ndufresne>
otherwise, as it's asynchronous, G_FMT would initially fail
<ndufresne>
this gives us the decoder "native format", it's best choice
<ndufresne>
On IMX Coda, the driver is capable of producing multiple pixel format
<ndufresne>
So the next step, is negotiate with the display the format, as a final step, we S_FMT the format
<ndufresne>
Most of the time the format is different, but certain decoder require that we call S_FMT to actually commit that format
<ndufresne>
it's somehow a bug, but we try to be flexible
<ndufresne>
if the format in S_FMT is not acceptable (or can't be changed), the driver is expected to just fill the structure with the same as what G_FMT would return
<ndufresne>
in gst, we read that back, and keep going
<wwilly_>
does somebody master ina231 nodes? I don't understand in0_input and in1_input
<ndufresne>
we just force downstream to whatever the driver gives us
<wwilly_>
I gues that in1_input is the volatage, but in0_input, to what it correspond?
<javier__>
ndufresne: got it, so S_FMT is correctly setting the src and dst formats (i.e: ctx->dst_fmt = find_format(f, MFC_FMT_RAW) )
<javier__>
but the problem is that struct v4l2_pix_format_mplane *pix_mp is not reported back to user-space?
<javier__>
so if S_FMT fails (like is the case now) you just use the px format plane that was returned by G_FMT (the native / default /whatever)
<javier__>
ndufresne: I'll give a try then filling in S_FMT as well
* ndufresne
surprised that the TRY_FMT stuff works ...
<ndufresne>
which we use for probing, it should implement enum_frame... tbf
<ndufresne>
javier__: what I don't get is where try_fmt() fill the fmt
<javier__>
ndufresne: it doesn't AFAICT
<javier__>
just checks if the format is one of the supported ones
<ndufresne>
wow, it's kind of badly implemented ...
<javier__>
ndufresne: so TRY_FMT should check that the format is correct *and* fille the format right?
<ndufresne>
it should fill everything yes
<ndufresne>
specially the strides
<javier__>
so {G,S}_FMT will just call find_format(f,...) and not do the filling
<ndufresne>
note that if you implement ENUM_ stuff, you can return EINVAL if the format is not in the list
<ndufresne>
but for width/height/stride you need to find the nearest
<javier__>
ndufresne: I see, right now only -EINVAL is returned
<javier__>
and is filled just if the format is found
<ndufresne>
I think einval if the format does not exist is fine
<ndufresne>
ah yeah, it does vidioc_enum_fmt()
<ndufresne>
make sense
<javier__>
ndufresne: I meant that doesn't fill the nearest
<ndufresne>
for the width/height no
<ndufresne>
the old way of probing, which get triggered here, is to S_FMT() with 1/1, and then MAX/MAX
<ndufresne>
to find the supported range, clearly it return as if 1/1 and MAX/MAX was supported
<ndufresne>
which I strongly doubt
<javier__>
ndufresne: Ok, step by step. I'll first move the fmt fill from G_FMT to TRY_FMT
<javier__>
that way S_FMT will also return valid values
<ndufresne>
++
<ndufresne>
javier__: well, not exactly, G_FMT returns what is configured, why TRY_FMT should return what is valid to configure
<ndufresne>
fixing whatever is not right (Excep the format, which need to be in the list)
<ndufresne>
so you actually need a filling function
<ndufresne>
javier__: see coda_try_fmt() they seem to do a little better
<ndufresne>
coda_get_max_dimensions() and v4l_bound_align_image
<javier__>
ndufresne: Ok, I'll look. Thanks for your help!
<ndufresne>
v4l_bound_align_image is the thing I need to look at, as it round middle, I need to be able to choose the direction (middle, up, down)
<ndufresne>
for all OUTPUT, you want to round up really
<ndufresne>
for G_SELECTION, you want to round down