<!-- Some styling for better description lists --><style type='text/css'>dt { font-weight: bold;float: left;display:inline;margin-right: 1em} dd { display:block; margin-left: 2em}</style>

   ***: ndec|away has quit IRC (Ping timeout: 260 seconds)
   hverkuil: <u>tfiga</u>: if V4L2-FLAG-MEMORY-NON-CONSISTENT is indeed a problem, then we need to remove it from v5.9-rc1 asap until it is clean what it should be replaced with. Christoph is right about that in one of his posts that it was only introduced very recently so we can remove it before it becomes part of the uAPI.
   <br> <u>ezequielg</u>: ping
   ezequielg: <u>hverkuil</u>: \o
   hverkuil: Good morning!
   <br> One quick question first: did you verify the v3 h264 struct changes with pahole?
   <br> Both for 32 and 64 bits?
   ezequielg: I should have done that yes.
   <br> Is it looking wrong?
   hverkuil: Since you didn't check, I'll do that. It doesn't hurt if someone else checks it.
   ezequielg: nono, I meant -- I did check.
   <br> I should have added that to the cover letter.
   hverkuil: Ah, good.
   ezequielg: I'm checking now, but please check as well -- it doesn't hurt to have an extra pass.
   <br> looks good to me.
   hverkuil: <u>ezequielg</u>: struct v4l2_h264_reference has implicit padding at the end:
   <br> struct v4l2_h264_reference {
   <br>        enum v4l2_h264_field_reference fields;           /*     0     4 */
   <br>        __u8                       index;                /*     4     1 */
   <br>        /* size: 8, cachelines: 1, members: 2 */
   <br>        /* padding: 3 */
   <br>        /* last cacheline: 8 bytes */
   <br> };
   ezequielg: yes, I noticed that.
   dv_: <u>question</u>: if I see this correctly, if for example I queue 10 buffers into a v4l2 capture device, then v4l2 will keep capturing frames on its own until it runs out of queued not-yet-filled buffers, right?
   <br> so, if for example filling these 10 buffers would take 200ms, and the application for some reason is blocked for 100ms, then in the meantime, 5 buffers will have been filled, and the next 5 VIDIOC_DQBUF ioctls will finish immediately, right?
   ezequielg: but the struct is only used in v4l2_ctrl_h264_slice_params ?
   <br> and then the padding doesn't seem used in v4l2_ctrl_h264_slice_params, is it?
   <br> struct v4l2_h264_reference ref_pic_list0[32];    /*    20   256 */ ?
   hverkuil: <u>ezequielg</u>: yes, but it introduces holes of potentially uninitialized data in the ref_pic_list0/1 arrays.
   ezequielg: right.
   hverkuil: either make index a __u32 or add reserved bytes (which is a pain when used in an array, I'd go with __u32).
   ezequielg: no probs with that.
   hverkuil: <u>dv_</u>: yes
   ezequielg: however, i'm still confused, isn't v4l2_ctrl_h264_slice_params size: 536?
   <br> (therefore, no padding)
   hverkuil: struct v4l2_h264_reference ref_pic_list0[32];    /*    20   256 */
   <br> So each element has a size of 8 bytes and the last three bytes are padding of the element.
   ezequielg: oh
   <br> i was reading this wrong.
   <br> right, index needs to be a u32.
   <br> :P how clumsy of me.
   hverkuil: <u>ezequielg</u>: also, I see inconsistent field naming: enum v4l2_h264_field_reference is used twice: once it is called 'fields' and the second time it is called 'reference'. It would be nice to make that consistent.
   ezequielg: yes, that's right.
   <br> probably reference on both.
   hverkuil: BTW, perhaps enum v4l2_h264_field_reference should be replaced with defines. Then you can use __u8 in struct v4l2_h264_reference and avoid the padding issue completely. And safe quite a lot of memory (64 * 6 bytes).
   <br> yeah, definitely do that. Saving 384 bytes is nothing to sneeze at :-)
   ezequielg: yes, that makes sense.
   sailus: <u>mchehab</u>: Bom dia!
   ezequielg: <u>sailus</u>: practicing for some future holidays in the beach?
   sailus: <u>ezequielg</u>: Interesting question.
   <br> There are beaches around here, but the holidays are gone for now. :-o
   <br> How do you do?
   ezequielg: Fully enjoying my staycation :)
   sailus: I enjoy working from home. :-)
   ezequielg: nice.
   sailus: Back in a bit...
   mchehab: <u>sailus</u>: moikka!
   ezequielg: <u>hverkuil</u>: would it be too much to ask to reply the patches with your review comments? i can get a v4 early next week (which we'll include the small patch v4 I sent yesterday).
   hverkuil: <u>ezequielg</u>: I can do that.
   ezequielg: thanks a lot.
   hverkuil: <u>ezequielg</u>: done. Otherwise this series looks good.
   <br> <u>ezequielg</u>: after this series, what is still on the TODO list, if anything?
   ezequielg: move to public following the plan you proposed on my rfc.
   <br> then vp8, then mpeg-2.
   <br> then of course, hevc and vp9 also pending.
   jernej: oh, I guess I should do v2 for Cedrus VP8
   ezequielg: well, vp9 is not even in staging :)
   hverkuil: <u>ezequielg</u>: are there still areas in h264 that have not been tested? (MVC? Others?)
   <br> <u>ezequielg</u>: interlaced has now been fully tested and verified, as I understand it, right?
   ezequielg: yes, interlaced (field) decoding is verified. my understanding is that was tested on rkvdec, although it's not fully implemented yet.
   <br> but the uapi is there.
   <br> as for MVC, I am not sure we've tested that.
   hverkuil: <u>pinchartl</u>: do you want to handle this patch yourself? https://patchwork.linuxtv.org/project/linux-media/patch/20200818133608.462514-7-gregkh@linuxfoundation.org/
   <br> <u>ezequielg</u>: if MVC can be supported by later adding new controls, then that would be fine. My main concern would be if changes would be needed for the existing H264 controls in order to support MVC.
   <br> <u>dafna2</u>: ping
   ezequielg: <u>hverkuil</u>: let me check about mvc with ndufresne
   pinchartl: <u>hverkuil</u>: not necessarily, but I'd like a reply from Greg :-)
   sailus: <u>mchehab</u>: I was quickly going to ask about the fixes branch.
   <br> There's a compile dependency fix that would be nice to get to 5.9 (and Linus's tree).
   mchehab: ok. feel free to send for 5.9
   <br> don't forget to use "GIT FIXES" at the subject
   <br> of the PR
   <br> (actually [GIT FIXES])
   sailus: But the fixes branch is currently for 5.8.
   mchehab: np
   <br> just base your work on the top of 5.9-rc1
   <br> usually, I only pull back from Linus there when I need
   <br> (e. g. when someone sends me something to be applied against 5.x-rc?)
   sailus: Ack.
   hverkuil: <u>pinchartl</u>: OK, I'll wait for a reply and if you're OK with it, then I'll merge that patch together with the others in that series.
   ***: elfGamal has quit IRC (Max SendQ exceeded)
   tfiga: <u>hverkuil</u>: I'll leave it up to you then
   <br> I think the UAPI itself is fine
   <br> and so is most of the implementation
   <br> only the vb2-dc code isn't right
   hverkuil: <u>ezequielg</u>: since you'll post a new version of the h264 cleanup series, I'll mark the current ones as 'Changes Requested'.
   ndufresne: <u>ezequielg</u>: hverkuil: perhaps have a read of https://en.wikipedia.org/wiki/Multiview_Video_Coding#Open_source_support_mostly_missing
   <br> What the article is not saying is that MVC was a failure, and adoption wasn't so big
   <br> In GStreamer we do support stereo movie, but with files that in fact does not use MVC (2D + delta encoding)
   <br> this is achieve by having 1 larger image, with the views side by side, or other similar simple layout
   <br> does not compress as much, but works with any existing decoders including ffmpeg
   ezequielg: <u>ndufresne</u>: thanks.
   ndufresne: I can send you an mvc file we'd use a lot to test the 2D decode fallback
   <br> I know this fallback works in gstreamer-vaapi
   ***: dlynes has left
   <br> varodek has quit IRC (Read error: Connection reset by peer)
   paulk-leonov: I'm not sure there is much practical interest in supporting MVC anyway
   <br> same for MVC-3D and others
   <br> but it would be good that the API does not close the door to implementing it
   jernej: paulk-leonov: what exactly is needed for mvc? is it possible that current controls are enough for mvc too?
   <br> afaik all H264 specific settings are now used in Cedrus
   paulk-leonov: <u>jernej</u>: IIRC and from a quick look at the spec, it needs lots of extra stuff
   <br> I doubt that cedrus or any other hw decoder implements it
   <br> grep mvc_extension in the spec
   <br> SVC would definitely be more useful
   <br> and maybe some hw decoders support that
   <br> for that you can just check if SI/SP frames are supported
   jernej: I'll just try 3D MVC H264 sample now...
   <br> hm... that was pretty severe crash, it will be hard to determine what's wrong
   <br> I guess I have different problem, all videos crash
   paulk-leonov: anyway I think MVC is not supported by ffmpeg sw decoding
   jernej: paulk-leonov: I tested 3D MVC H264 on H3 and it decodes correctly with ffmpeg/kodi
   <br> but I don't know if any shortcut is taken like with interlaced HEVC