On Mon, 03 Jun 2019 14:52:44 -0400 Nicolas Dufresne <nicolas@xxxxxxxxxxxx> wrote: > > > - Dropping the DPB concept in H.264/H.265 > > > > > > As far as I could understand, the decoded picture buffer (DPB) is a > > > concept that only makes sense relative to a decoder implementation. The > > > spec mentions how to manage it with the Hypothetical reference decoder > > > (Annex C), but that's about it. > > > > > > What's really in the bitstream is the list of modified short-term and > > > long-term references, which is enough for every decoder. > > > > > > For this reason, I strongly believe we should stop talking about DPB in > > > the controls and just pass these lists agremented with relevant > > > information for userspace. > > > > > > I think it should be up to the driver to maintain a DPB and we could > > > have helpers for common cases. For instance, the rockchip decoder needs > > > to keep unused entries around[2] and cedrus has the same requirement > > > for H.264. However for cedrus/H.265, we don't need to do any book- > > > keeping in particular and can manage with the lists from the bitstream > > > directly. > > > > There was a bit of concern regarding this. Given that DPB maintenance is > > purely a software construct, this doesn't really belong in the kernel. A > > DPB will be the same no matter what hardware operates on the bitstream. > > Depending on the hardware it may use the DPB differently (or maybe not > > at all), but that's beside the point, really. This is pretty much the > > same rationale as discussed above for meta data. > > > > Again, VAAPI and VDPAU don't require drivers to deal with this. Instead > > they just get the final list of reference pictures, ready to use. > > I think we need a bit of clarification from Boris, as what I read here > is a bit contradictory (or at least I am a bit confused). When I first > read this, I understood that this was just about renaming the dpb as > being the references list and only require the active references to be > there. It's really just about renaming the field, it would contain exactly the same data. > > So what I'm not clear is where exactly this "active reference list" > comes from. In VAAPI it is describe "per-frame" .... That's my understanding as well. > > > > > > - Using flags > > > > > > The current MPEG-2 controls have lots of u8 values that can be > > > represented as flags. Using flags also helps with padding. > > > It's unlikely that we'll get more than 64 flags, so using a u64 by > > > default for that sounds fine (we definitely do want to keep some room > > > available and I don't think using 32 bits as a default is good enough). > > > > > > I think H.264/HEVC per-control flags should also be moved to u64. > > > > There was also some concensus on this, that u64 should be good enough > > for anything out there, though we obviously don't know what the future > > will hold, so perhaps adding some way for possible extending this in the > > future might be good. I guess we'll get new controls for new codecs > > anyway, so we can punt on this until then. > > > > > - Clear split of controls and terminology > > > > > > Some codecs have explicit NAL units that are good fits to match as > > > controls: e.g. slice header, pps, sps. I think we should stick to the > > > bitstream element names for those. > > > > > > For H.264, that would suggest the following changes: > > > - renaming v4l2_ctrl_h264_decode_param to v4l2_ctrl_h264_slice_header; > > > - killing v4l2_ctrl_h264_decode_param and having the reference lists > > > where they belong, which seems to be slice_header; > > But now here it's being described per slice. When I look at the slice > header, I only see list of modifications and when I look at userspace, > That list is simply built from DPB, the modifications list found in the > slice header seems to be only used to craft the l0/l1 list. Yes, I think there was a misunderstanding which was then clarified (unfortunately it happened on IRC, so we don't have a trace of this discussion). The reference list should definitely be per-frame, and the L0/L1 slice reflists are referring to the per-frame reference list (it's just a sub-set of the per-frame reflist re-ordered differently). > > There is one thing that come up though, if we enable per-frame decoding > on top of per-slice decoder (like Cedrus), won't we force userspace to > always compute l0/l1 even though the HW might be handling that ? That's true, the question is, what's the cost of this extra re-ordering? > Shall > we instead pass the modification list and implement the non-parsing > bits of applying the modifications in the kernel ? I'd be fine with that option too.
