hverkuil, I have v6 of the cedrus driver nearly ready to go out now
sorry it took a few days
hverkuil, also, I see that vim2m exposes MEDIA_ENT_F_PROC_VIDEO_SCALER in the media pipeline -- is that relevant for stateless VPU drivers as well?
I'd say it's not, but couldn't really find a better candidate in the list
paulk-leonov: you need this patch: https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=vicodec&id=9c84fdf20f6df3212203eb3d11538a1c63ea0a4c
It gives you proper ENT_F defines.
It's part of a pending pull request.
(vicodec)
oh great
hverkuil, I was also wondering whether we should use v4l2_m2m_register_media_controller in cedrus
hverkuil, do you want that in v6 or can it wait for later?
it seems that the way we initialize the media pipeline currently is sub-optimal
(in the cedrus driver)
Please add that to v6 of the cedrus driver.
will do!
I recommend that you add the vicodec patch for the new ENT_F defines to your patch series and use them in the v4l2_m2m_register_media_controller call.
excellent
mchehab: thanks! I'll update v4l-utils accordingly today/tomorrow.
anytime. I guess all pull requests were handled
hmm... not really
mchehab: Obrigado!
mchehab: There's one more. Just a small one this time. The rest can go to 4.20.
I'd think.
hverkuil: I'm reviewing right now the venus patchset...
the decoder does "multi-stream" decoding... not sure what it means by that, but that seems to violate the descriptions you added for codecs
you said there that an encoder/decoder should have just one input and one output pads
if I remember well
It's not what you think. It's all hidden internally. Let me try to find the mail thread were this was explained.
if it is hidden internally, it should be ok...
however, I'm wonder if forcing just one input/output pad is too restrict
From the commit log:
"The DPB buffers are used for decoder reference frames and those have to be in a specific format (UBWC). So one decoder output is used to fill those reference buffers while the other output is used to fill the userspace buffers with the user requested format."
I never looked deeply, but, at least on Digital TV (ISDB-T), it seems that some encoders generate two output streams
one "low res" stream and one "high res" diff stream
So the HW can produce two streams, but one is only ever used internally.
for HD, both streams are needed
for LD, only one is needed
as I said, I didn't look deeply to see how this is actually encoded - but it makes me wonder that it would be possible to have encoders/decoders with multiple input or output pads
That said, in general an encoder/decoder can in principle produce more than one output. Esp. a decoder.
"Must have one sink pad and one source pad." should become "Must have one sink pad and at least one source pad."
Actually, I think that is only true for decoders.
yeah, that makes more sense for me
It doesn't make sense for an encoder.
well, it should be true for encoders too, if the encoder supports hierarchy encoding
(I guess that's the name they use on DVB/ISDB for such encoders)
It still produces one bitstream I think. I'm no expert on that, though.
on ISDB, it produces 2 streams
that are sent to different carriers
I'll change it for both.
carriers on ISDB are grouped into 13 segments
one segment contains the LD frames
the other 12 contains the ones required for HD
(with are combined with the other one, as far as I understand)
posted patch.
looks OK
gah...
drivers/media/platform/qcom/venus/hfi.c:210 hfi_session_init() debug XXX: trouble parsing 0-808996949,808996951-809062485,809062487-826496576,826496578-826757196,826757198-843534412,843534414-859189831,859189833-875967047,875967049-877088844,877088846-1129727303,1129727305-1145656919,1145656921-1194410837,1194410839-1278296917,1278296919-u32m c = u32m
smatch bug?
I had the same with smatch.
the output of smatch has been useless those days... lots of stuff that looks like false positives
sailus: this new driver is missing a MAINTAINERS entry
WARNING: prefer 'help' over '---help---' for new help texts
#41: FILE: drivers/media/i2c/Kconfig:640:
+config VIDEO_OV2680
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
tfiga: finished review
ezequielg, ok, about the timestamp with explicit fence
ezequielg, my conclusion is that the driver must return a timestamp on QBUF in that case, predicting when the next timestamp will be produced, and dealing with compensating base on obseration
Actually, this might not work, you need to Q a buffer to start streaming, but need to start streaming to predict timestamp
unless we vomit the idea that DQBUF gives the timestamp, and then userspace need to implement a clock around that ...
that's makes pretty complex userspace, but would in theory work
mchehab: Indeed, you're right. I missed checking that for this one.
Thanks for pointing this out.
I think a MAINTAINERS change is needed for the other patch, too.
I'll fix those.
Or, in the case of the other patch, ask for a MAINTAINERS entry from the author.
mchehab: I changed the patchwork state for the request.
ok
yeah, the author should be ok on adding him/her at MAINTAINERS
hverkuil: Thanks! I'll go through your comments tomorrow :)
hverkuil: I'm about to apply some patches on media that should likely be reverted at media_build tree:
Subject: [1/7] media: netup_unidvb: don't check number of messages in the driver
1b0aabee1295 media: cx231xx: don't check number of messages in the driver
281676986d73 media: si4713: don't check number of messages in the driver
6358c599ccc3 media: em28xx: don't check number of messages in the driver
1d534d19cea0 media: hdpvr: don't check number of messages in the driver
5ce42011afb8 media: dvb-usb: don't check number of messages in the driver
b29103608371 media: tm6000: don't check number of messages in the driver
051bf10cad3a media: netup_unidvb: don't check number of messages in the driver
hverkuil, mhh I just noticed that my cedrus cover letter mentions you as "Hand Verkuil" -- I'm quite sorry about that.
hverkuil: paulk-leonov: I just posted the v2 for the .device_run defered run.
One thing that I noticed is that device_run called via ioctls is called with queue lock held, while device_run currently called via job_finish does not.
This is not a big deal right now, I think, because drivers usually just take a couple buffers (with a spinlock) and then write some registers to fire DMA.
but my series is amending that, so that now device_run is always called with the q_lock held, mostly for symmetry.
hverkuil: I have been thinking that the q_lock in m2m_ctx should be mandatory. IOW, drivers shouldn't be allowed to set different locks for the out/cap queues.